Opened 6 months ago

Closed 4 months ago

#13196 closed defect (fixed)

SCUMM: Invalid phrase with GIVE crashes the game (Demo)

Reported by: antoniou79 Owned by: bluegr
Priority: normal Component: Engine: SCUMM
Version: Keywords: crash
Cc: Game: Monkey Island 1

Description

Tested on Windows 10 x64 with the Monkey Island EGA demo from the ScummVM Demos page: "The Secret of Monkey Island (DOS EGA Demo)" (English)

This was with ScummVM local build 2.6.0git from today's master.

Steps to reproduce:

  • Go into the Scumm Bar
  • Pick up a breath mint from the bowl on the table
  • Go into the kitchen
  • Pick up the pot from under the table
  • Form the phrase "Give pot to breath mint" and submit it

Crash Error:
ERROR: (57:13:0x1F): Invalid actor 303 in o5_faceActor!

I have not checked with the full version of the game.

Attachments (1)

opcodes.txt (72.8 KB ) - added by r41k0u 4 months ago.

Download all attachments as: .zip

Change History (8)

comment:1 by AndywinXp, 6 months ago

Does this happen when running the game with the original exe on e.g. DosBox?

in reply to:  1 comment:2 by antoniou79, 6 months ago

Replying to AndywinXp:

Does this happen when running the game with the original exe on e.g. DosBox?

I just checked with a quick DosBox setup (borrowed from GOG's installation of LBA) and while I don't have any sound (probably due to me not fiddling with DOSBOX configuration for sound), the crash does not happen.

The game takes a second, but then just ignores the command and reverts to "Walk to" in the verb line. So: "GIVE pot to breath mint" results in being ignored and "Walk to breath mint" is shown in the verb line at the end. No line from Guybrush. "GIVE breath mint to pot" is similarly ignored in DOSBOX ("Walk to pot" is shown at the verb line in the end).

Note: this all is again for the DEMO downloaded from the ScummVM site. I cannot test for the full game as of yet.

I don't know if it's important but both "Give pot to hunk o' meat" and "Give hunk o' meat to pot" will also produce a line from Guybrush "That does not seem to work" (this happens both in DOSBOX and ScummVM, with no crash on ScummVm). This line does not play for the problematic pot and breath mint command combination.

Also note, ScummVM crashes whether the command is "Give pot to breath mint" or "Give breath mint to pot".

Last edited 6 months ago by antoniou79 (previous) (diff)

comment:3 by tag2015, 6 months ago

I did some quick testing. The crash occurs with any inventory item the breath mint is "given" to, for example "give breath mint to hunk of meat" causes a:
ERROR: (51:13:0x1F): Invalid actor 308 in o5_faceActor!
while "give breath mint to red herring" causes
ERROR: (51:13:0x1F): Invalid actor 306 in o5_faceActor!
and so on, where the actor number is the inventory item id, i suppose.

Also, the issue occurs identically in the DOS version, in the DOS german version ("gib pfefferminz an topf") and in the amiga version. It does not happen in the "passport to adventure" version or in the full game (in both cases, Guybrush just says the usual "that doesn't seem to work" phrase).
As you correctly stated, the problem is exclusive to scummvm and does not happen when using dosbox.

in reply to:  3 comment:4 by antoniou79, 6 months ago

Replying to tag2015:

The crash occurs with any inventory item the breath mint is "given" to, for example "give breath mint to hunk of meat" causes a:
ERROR: (51:13:0x1F): Invalid actor 308 in o5_faceActor!
while "give breath mint to red herring" causes
ERROR: (51:13:0x1F): Invalid actor 306 in o5_faceActor!
and so on, where the actor number is the inventory item id, i suppose.

The error occurs most of the time when the verb command is "reversed" too, as I mentioned above, which means it may occur when something is "given" to the breath mint.
So eg.

"GIVE red herring to breath mint" still produces:
ERROR: (51:13:0x1F): Invalid actor 306 in o5_faceActor!
(same error as "GIVE breath mint to red herring")

"GIVE pot to breath mint" still produces:
ERROR: (51:13:0x1F): Invalid actor 303 in o5_faceActor!
(same error as "GIVE breath mint to pot")

However, oddly enough:

"GIVE hunk o' meat to breath mint" does not crash the game. It results in a "that doesn't seem to work" comment. (but :"GIVE breath mint to hunk o' meat" does indeed crash the game as noted in tag2015's comment.)

comment:5 by r41k0u, 4 months ago

https://github.com/scummvm/scummvm/pull/3731
This issue is being addressed by this PR.

by r41k0u, 4 months ago

Attachment: opcodes.txt added

comment:6 by r41k0u, 4 months ago

Why this bug occurs:
breathe mint is meant to be given to actors. In a playthrough of the EGA Demo, I found out that this is given to a prisoner in jail. before getting the mint the prisoner paces up and down in the cell. But when the give sentence is passed, both guybrush and the prisoner face each other.

During debugging I found that giving breath mint to something calls o5_faceActor twice: Once to make guybrush face the receiving object, then again to make the receiving object face guybrush.

But as in the reported bug's case; pot, red herring and hunk o' meat are not actors, so this results in an error.

Now this error also occurs when pot or red herring is given to breath mint, as they reverse the sentence in their scripts (they call o5_doSentence again) with objectA and objectB reversed. This behaviour results in the sentence "give breath mint to pot/red herring" being actually executed, which results in the error.

Giving hunk o' meat to breath mint doesn't trigger this error becuase it doesn't reverse the sentence.

Attaching the "opcodes" logs I made during debugging.

comment:7 by bluegr, 4 months ago

Owner: set to bluegr
Resolution: fixed
Status: newclosed

The PR has been merged, so this can be closed now. Thanks to r41k0u for his work on this!

Note: See TracTickets for help on using tickets.