Opened 21 years ago
Closed 19 months 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)
Change History (12)
comment:1 by , 21 years ago
| Summary: | Invalid actor → INDY4: Invalid actor |
|---|
by , 21 years ago
| Attachment: | atlantis.s11 added |
|---|
comment:2 by , 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 , 21 years ago
| Owner: | set to |
|---|
comment:5 by , 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 , 21 years ago
| Keywords: | script added |
|---|---|
| Owner: | removed |
| Summary: | INDY4: Invalid actor → FOA: Invalid actor |
comment:8 by , 21 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:9 by , 21 months ago
| Resolution: | fixed |
|---|---|
| Status: | closed → new |
| Summary: | FOA: Invalid actor → FOA: 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 , 19 months ago
| Owner: | changed from to |
|---|---|
| Resolution: | → fixed |
| Status: | new → pending |
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 , 19 months ago
| Status: | pending → closed |
|---|
Savegame just before the crash