Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#2466 closed defect (fixed)

FT: Invalid Actor when

Reported by: SF/jorpho Owned by: eriktorbjorn
Priority: normal Component: Engine: SCUMM
Keywords: script Cc:
Game: Full Throttle

Description

I have observed this error in both ScummVM 0.8.0 and
the daily CVS build (Win32 versions in both cases) from
January 15, 2006.

Go to the Junkyard with the gas can, lock pick, and
hose in your inventory. After trapping the dog in a
suspended car, go to the area where the dog is trapped.
The attached save game already has everything in
position.

Right-click to bring up the inventory, click on the
hose, and then click on the gas can. ScummVM will
crash with Error (28:103:0x12E): Invalid Actor 108 in
o6_isActorInBox.

I am not sure if this bug can only be triggered in this
location, but this is where I have observed it.

I am using an English version of Full Throttle
(specifically, the Creative Labs-issued CD with the 3-
level Special Edition of Dark Forces).

Ticket imported from: #1407789. Ticket imported from: bugs/2466.

Attachments (5)

ft.s04 (22.4 KB ) - added by SF/jorpho 14 years ago.
FT Save Game: Right click, click hose, click can to crash ScummVM
script-103.txt (2.3 KB ) - added by eriktorbjorn 14 years ago.
The crashing script
ft-workaround.diff (3.4 KB ) - added by eriktorbjorn 14 years ago.
Possible workaround
ft-PC-en-script-1.txt (13.5 KB ) - added by fingolfin 14 years ago.
Script 1, English, PC
ft-Mac-de-script-1.txt (14.4 KB ) - added by fingolfin 14 years ago.
Script 1, German, Mac

Download all attachments as: .zip

Change History (42)

by SF/jorpho, 14 years ago

Attachment: ft.s04 added

FT Save Game: Right click, click hose, click can to crash ScummVM

comment:1 by SF/jorpho, 14 years ago

Summary: Invalid Actor when "Combining" Two ItemsFT Invalid Actor when "Combining" Two Items

comment:2 by eriktorbjorn, 14 years ago

A few random observations:

* It seems to happen regardless of which inventory objects I
use on each other.

* It does not happen if I first leave the room.

* It appears to be the "sentence script" (in this case,
script 103) that's triggering the error message. The
parameters to the script are 'verb', 'objectA' and
'objectB'. In this case, 'objectB' is 108, so that's
probably where it gets the invalid actor id from.

It could be a script bug that happened to work with the
original interpreter, but that ScummVM considers to be a
fatal error. But I'm far from sure.

If it is a script bug, I guess the only thing we can do is
to add a special case to o6_isActorInBox(). That's easy, but
perhaps there's a more correct bugfix...?

comment:3 by bluegr, 14 years ago

I only get this error in that location, other places seem
to work fine

comment:4 by fingolfin, 14 years ago

Summary: FT Invalid Actor when "Combining" Two ItemsFT: Invalid Actor when "Combining" Two Items

comment:5 by fingolfin, 14 years ago

Wouldn't be the first script bug of that kind. I guess we'll have to disassemble
the relevant scripts and stare at the them for some time to figure out what to do
:-)

comment:6 by eriktorbjorn, 14 years ago

Well, I believe I have the correct script descummed already,
so I've attached it. But to be honest, I'm a bit uncertain
about what some of it means. It does this:

[00C3] (5D) if (getOwner(localvar1) == 1) {
[00CE] (43) localvar3 = localvar2
[00D4] (73) } else {
[00D7] (43) localvar3 = localvar1
[00DD] (**) }
[00DD] (0C) dup[1] = localvar3

Does that dup[1] line at the end mean that localvar3 is
being pushed onto the stack? Anyway, then it does

[00E1] (5D) if (dup[1] == 195) {
[00E8] (D5) jumpToScript(0,28,[localvar0,localvar1,localvar2])
[00FC] (5D) } else if (dup[1] == 0) {
[0107] (D5) jumpToScript(0,28,[localvar0,localvar1,localvar2])
[011B] (5D) } else if (!isActorInBox(21)) {

I know o6_isActorInBox() pops two parameters: The box
number, and the actor number. I take it 21 is the box
number, and according to the error message, 108 is the actor
number. That would correspond to localvar2, I believe, which
could be what localvar3 contains.

Either way, except for some special cases it's probably
assuming that either 'objectA' or 'objectB' is an actor. At
least, that's how I understand it.

by eriktorbjorn, 14 years ago

Attachment: script-103.txt added

The crashing script

comment:7 by cyxx, 14 years ago

This is definitely a script bug.

I just ran the original interpreter through a debugger and
played this scene. Like in scummvm, the sentence script (104
in my french version) is run with object numbers/ids as
parameters. Then o6_isActorInBox is called and silently read
data of out of the actor tables...

comment:8 by eriktorbjorn, 14 years ago

Hmm... Adding a workaround in just o6_isActorInBox() isn't
enough because other actor opcodes will be called with the
same bad actor. Setting one of the parameters to a valid
actor causes odd things to happen...

It looks like the usual sentence script is script 28.
Perhaps we could call that instead, it's about to call
script 103 with bad parameters. I've attached a possible patch.

by eriktorbjorn, 14 years ago

Attachment: ft-workaround.diff added

Possible workaround

comment:9 by fingolfin, 14 years ago

Well, since it's now confirmed as a script bug (thanks cyx), we shouldn't hesitate
to add a "WORKAROUND bug #XY" :-). Erik, you could try to implement your
trick and see if it does something sensible?

Alternatively, instead of calling the usual sentence script, we could simply do
nothing, no?

comment:10 by eriktorbjorn, 14 years ago

The advantage of calling the usual sentence script is that
then Ben will say "A hose won't help here." just as in any
other room. If you think it's a safe workaround, I'd be
happy to apply it.

Not calling any script at all will make nothing happen.

comment:11 by fingolfin, 14 years ago

Then I'd say, go for it :-)

comment:12 by fingolfin, 14 years ago

Owner: set to eriktorbjorn

comment:13 by eriktorbjorn, 14 years ago

Resolution: fixed
Status: newclosed

comment:14 by eriktorbjorn, 14 years ago

I've applied the fix to the trunk, but not the 0.8 branch.
Perhaps I should, but it is a pretty obscure bug...

comment:15 by cyxx, 14 years ago

Is there a way to differentiate FT versions (ie. a versus b)
? It seems your script numbers are all off-by one compared
to the ones in my french version.

room-28-2031

[034D] (5E) startScript(0,2024,[])
[0357] (43) VAR_SENTENCE_SCRIPT = 104
[035D] (5E) startScript(1,2030,[])

exit-91

[003D] (9D) actorOps.setShadowMode(1)
[0042] (43) VAR_SENTENCE_SCRIPT = 29
[0048] (7C) stopScript(2005)

Adding an extra check is always possible, but that doesn't
look like really a 'nice' solution to me....

comment:16 by eriktorbjorn, 14 years ago

Status: closednew

comment:17 by eriktorbjorn, 14 years ago

I don't know, but I guess I had better re-open the bug
report... Rats!

comment:18 by eriktorbjorn, 14 years ago

Resolution: fixed

comment:19 by eriktorbjorn, 14 years ago

I've made some changes in an attempt to deal with the
different Full Throttle versions. There's some guesswork
involved, so I don't know if it's working correctly.

comment:20 by cyxx, 14 years ago

That's working correctly with the french version.

comment:21 by fingolfin, 14 years ago

So can this item be closed now?

comment:22 by eriktorbjorn, 14 years ago

I'm not sure, but I hope so.

comment:23 by fingolfin, 14 years ago

I slightly modified your code in SVN: It now doesn't check the language, but
rather checks which value VAR_SENTENCE_SCRIPT is set to the very first time it is
set (by script 1).

Plus I confirmed that the scripts are correct for the German Mac version, too.

comment:24 by eriktorbjorn, 14 years ago

Is that really a safe thing to do? What happens if you use
the -x command-line option to load a saved game, thus (I
assume) bypassing script 1?

Mind you, I haven't been able to find a case where it breaks
yet...

comment:25 by fingolfin, 14 years ago

Arr, err... gee, saving and loading is for wuzzies only, I'll just disable it and we'll
be fine!

Gee, hm, embarassing but I totally forgot about that. So we either need to save
those script IDs, or revert, or come up with another method :-/

comment:26 by fingolfin, 14 years ago

OK here's another possibility: Use the value of res.num[rtScript]. For me it is 456
in the first case, and 469 for my German/Mac FT version.

Can you guys check which values you are seeing there?

comment:27 by eriktorbjorn, 14 years ago

My English PC version has res.num[rtScript] = 456, but
perhaps that's what you meant with the first case.

If this detection method works, we could remove the
variables keeping track of the script numbers. That'd be nice.

comment:28 by fingolfin, 14 years ago

Yes, that's what I meant. I have three versions of
FT/English (V1 PC, V2 PC, and Mac), all have 456. And my
German Mac FT has 469.

Now, if cyx says that his French FT also has 469, I say we
can simply modify the code accordingly. And maybe add a
warning() in the startup code if a different number is found
(or maybe initially let's use error() so that people notice
it and report it *g*).

comment:29 by cyxx, 14 years ago

...
allocResTypeData(Script/script,PRCS,467,1)
...
I get the same debug output for both of my FT Mac/PC french
versions.

Just to satisfy my curiosity, can someone with an english
version of FT attach here a dump of script-1 ? Thanks.

comment:30 by fingolfin, 14 years ago

OK, so the count doesn't match, but we still might be able
to do something like "if script count > 456" or "if script
count > 460" or so. It definitly would be good to find out
the script count for more versions of FT, we could ask
people in the forums and on IRC for some more testing.

I will attach a dump of script-1 once I am back home (unless
somebody is faster :). I already determined which script
they inserted (IIRC it was script 12 or 13 or so, need to
look it up at home), if that's what you are interested in.

by fingolfin, 14 years ago

Attachment: ft-PC-en-script-1.txt added

Script 1, English, PC

by fingolfin, 14 years ago

Attachment: ft-Mac-de-script-1.txt added

Script 1, German, Mac

comment:31 by fingolfin, 14 years ago

Here ya go, script for PC/English and Mac/German attached.

comment:32 by cyxx, 14 years ago

Thanks for uploading it ; and yes, that was what I was
interested in, looking at which scripts they may have added.

comment:33 by fingolfin, 14 years ago

It's script 12 that was inserted (for me one of the first things it does is to call
"printDebug.msg("start dialog scan")"). Compare this with script 13 (resp. the
script 12 in the "old" numbering, i.e. in the english version).

comment:34 by fingolfin, 14 years ago

Resolution: fixed

comment:35 by fingolfin, 14 years ago

I commited a new fix based on the script count. It would be nice to know
whether this works correctly for other FT versions, of course.

BTW, it's really simple to find out the second script: One just has to run
scummvm -b28 -u ft
to get a dump of the relevant scripts from room 28. In my case that it is
room-28-2031 which modifies VAR_SENTENCE_SCRIPT.

comment:36 by fingolfin, 14 years ago

Closing this for now, since nobody complained so far =)

comment:37 by fingolfin, 14 years ago

Status: newclosed
Summary: FT: Invalid Actor when "Combining" Two ItemsFT: Invalid Actor when
Note: See TracTickets for help on using tickets.