Opened 21 years ago

Closed 2 years ago

#832 closed defect (fixed)

FOA: Invalid actor XXX in o5_getActorRoom()

Reported by: SF/grandepuffo Owned by: AndywinXp
Priority: normal Component: Engine: SCUMM
Version: Keywords: script
Cc: Game: Indiana Jones 4

Description

In room 222 (crashed door, robot and chain), crossing the door causes scummvm to entering console, showing the following log

Loading room 222 loadResource(Script,23) loadResource(Room,94) loadResource(Script,33) loadResource(Script,19) loadResource(Script,45) loadResource(Script,10) loadResource(Script,131) (94:206:0x15FFA): Invalid actor 205 in o5_getActorRoom(94:206:0x15FFA): Invalid actor 205 in o5_getActorRoom!

i'm using today (05/30/2003) cvs snapshot (0.4.2cvs)

Ticket imported from: #746349. Ticket imported from: bugs/832.

Attachments (1)

atlantis.s11 (84.9 KB ) - added by SF/grandepuffo 21 years ago.
Savegame just before the crash

Download all attachments as: .zip

Change History (12)

comment:1 by SF/grandepuffo, 21 years ago

Summary: Invalid actorINDY4: Invalid actor

by SF/grandepuffo, 21 years ago

Attachment: atlantis.s11 added

Savegame just before the crash

comment:2 by fingolfin, 21 years ago

I can't even load this savegame. Was it made with a non-english version of FOA by chance (your name sounds italian :-)

comment:3 by fingolfin, 21 years ago

Owner: set to fingolfin

comment:4 by fingolfin, 21 years ago

Found my own savegame for that place to reproduce the problem.

comment:5 by fingolfin, 21 years ago

This seems to be either a bug in the FOA scripts, or we have a bug in our script engine. Script 206 (in room 94) is run from script 200, like this:

... [0019] (10) Var[442] = getObjectOwner(586) [001E] (2A) startScript(201,[Var[442]]) [0024] (6A) startScript(206,[Var[442]]) ...

The problem is that script 201 gets to run first, and it changes the value of Var[442], so by the time script 206 is invoked, it gets a bad value as param.

So, either: 1) A script bug, and we should just "ignore" it 2) An engine bug, several things possible: 2a) Maybe high vars should be "buffered", kind of like local vars? That seems odd... but then again it seems odd to use a global var like 442 for something which is clearly a local business 2b) Maybe we shouldn't be running script 201/206 immediately but rather just queue them for running. But that seems very unlikely.

Var[442] is really used a lot for temporary results, in many scripts. I wonder why they didn't use a localvar instead. Maybe this was kind of a "TEMP_REGISTER" variable or so.

comment:6 by fingolfin, 21 years ago

Keywords: script added
Owner: fingolfin removed
Summary: INDY4: Invalid actorFOA: Invalid actor

comment:7 by fingolfin, 21 years ago

Put a workaround into CVS; but I don't like this a bit, hm...

comment:8 by fingolfin, 21 years ago

Owner: set to fingolfin
Resolution: fixed
Status: newclosed

comment:9 by dwatteau, 2 years ago

Resolution: fixed
Status: closednew
Summary: FOA: Invalid actorFOA: Invalid actor XXX in o5_getActorRoom()

Reopening this very old ticket, as suggested by AndywinXp ("This way I will eventually remember to check for the correct behavior in my v5 disasm and properly fix it") after this commit of mine:
https://github.com/scummvm/scummvm/commit/9c78d324a6a674d022d725cc05aee57de35fd216

It appears that we still need this workaround as of 2022. A quick test for this is the 5234 boot param.

If I test the original interpreter with DREAMM, I don't see any crash, while ScummVM will issue a fatal error about this. I can't say if this is because we're more strict in our getActorRoom() implementation for v5, or if we don't handle these startScript() and Var calls exactly as the original does.

comment:10 by AndywinXp, 2 years ago

Owner: changed from fingolfin to AndywinXp
Resolution: fixed
Status: newpending

Apparently the original relies on undefined behavior for these (well, actually this is currently the only one which has shown up in 20 years...) edge cases, so the fix seems to be correct. But I have extended this to all the games using this piece of code.

comment:11 by AndywinXp, 2 years ago

Status: pendingclosed
Note: See TracTickets for help on using tickets.