Opened 5 years ago

Closed 3 years ago

#6751 closed defect (fixed)

SCI: Longbow - Invalid arithmetic operation

Reported by: raziel- Owned by: m-kiewitz
Priority: normal Component: Engine: SCI
Keywords: original script Cc:
Game: Conquests of the Longbow

Description

ScummVM 1.8.0git (Nov 6 2014 19:33:16)
Features compiled in: TAINTED Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC FreeType2 JPEG PNG

Wandering through Sherwood Forest (especially north of the camp) leads to an error entering a specific screen. (In my case i was wandering around south of the camp then continuing east of it and finally arriving in the shooting range screen where the error happened).
It is not reproducable though...i tried

I know the build is rather old already, i was pondering if i should report a non-reproducable error at all...

User picked target 'longbow' (gameid 'sci')...
Looking for a plugin supporting this gameid... SCI [SCI0, SCI01, SCI10, SCI11]
Starting 'Sierra SCI Game'
WARNING: Failed to locate base object for object at 0053:1A40; skipping!
WARNING: Failed to locate base object for object at 0053:13AE; skipping!
Invalid arithmetic operation (addition - params: 002a:035d and 0000:0003) from method ::export 2 (room 220, script 225, localCall ffffffff)!

Conquests of the Longbow: The Adventures of Robin Hood (DOS/English)

AmigaOS4 - SDL - PPC - BE
gcc (GCC) 4.2.4 (adtools build 20090118)

Ticket imported from: bugs/6751.

Attachments (2)

ScummVM #6571 - SCI Longbow - Console - bt.png (82.7 KB ) - added by raziel- 3 years ago.
longbow.040 (32.0 KB ) - added by raziel- 3 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 by m-kiewitz, 5 years ago

Hi,

If you get this error again, please copy&paste the details from the additional window as well, if there are any. And also please enter the ScummVM debugger and execute command "bt" (for a backtrace) and post the results in here as well.

Have you applied any patch files to this game?

comment:2 by raziel-, 5 years ago

Hi,

There was no additional window when it happened, i'm afraid.
Also, as said above, i wasn't able to reproduce it at all.

I did have patches installed (not sure if those have been fixed already by ScummVM, but i was told to install them anyway)
http://www.sierrahelp.com/Patches-Updates/Patches-Updates-Games/ConquestsUpdates.html#Longbow
Only the patches from Conquest of the Longbow (LBPAT) (ZIP file) [14.3 KB]

comment:3 by m-kiewitz, 4 years ago

bug 5820 reports the same issue

comment:4 by raziel-, 3 years ago

I managed to reproduce the bug.

If you want to try (not very time consuming, but i still don't know if it can always be triggered like that).
1) Start the game
2) Walk straight south of the camp
3) Proceed south until you can't go any further
4) Now turn east and proceed until you are stuck again
5) Turn north and proceed north and east (sometimes you can still walk east)
6) When you reach the farthest point north east, finally turn west
7) You can walk about two screens before the error pops, normally when you enter the shooting range

I've never seen this error pop up in another place, must be something speciific to the shooting range screen?

ScummVM 1.9.0git (Jul 3 2016 22:57:42)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC FreeType2 JPEG PNG

Invalid arithmetic operation (addition - params: 002a:0043 and 0000:0003) from method ::export 2 (room 220, script 225, localCall ffffffff)!

As per your request i did a "bt" in the console, unfortunately i can only provide a screenshot, as i can't copy&paste the console text, i hope this is sufficient

comment:5 by m-kiewitz, 3 years ago

Wow great. I tried around for an hour and couldn't get it. Will look at this asap.

by raziel-, 3 years ago

Attachment: longbow.040 added

comment:6 by raziel-, 3 years ago

Now that i know how to reproduce it, here is the save state :-)

Just walk to the left from this screen and the error pops

comment:7 by m-kiewitz, 3 years ago

Hmm the saved game crashes my ScummVM.

Did you apply any script patches?

comment:8 by m-kiewitz, 3 years ago

It seems you applied the official Sierra update. Works now for me.

It seems the scripts get a local and then try to add a random number to it. In this specific case the local points to the object "berryBush". It seems something really bad is going on.

Can you please do the same steps using the original interpreter?

comment:9 by m-kiewitz, 3 years ago

Hmm, local[110h] and local[111h] are both overwritten with berryBush.

The other locals after that were not changed at all and are set to the values originally set by the script resource data.

comment:10 by m-kiewitz, 3 years ago

the locals are set at least in the room, that you enter after going all south and then all east, north once and then you can go east into a dead-end. That's when the locals are changed for me from the default to berryBush.

I need to figure out what exacly is going wrong in there.

But as a little note - there is no way to fix this corruption after it occurred except using debug commands. So in case someone else gets this bug, you can fix it by entering
bpe 225 2
this sets a breakpoint, that should trigger as soon as you leave/enter a room in the forest.
When it triggers type
vv l 110h
This will show you the contents of local 110h. In case it's berryBush, then that's this bug right here.
Enter
vv l 110h 110h
vv l 111h 2bh

this will then change the values back the original ones and then this error shouldn't happen anymore.

I think it's some kind of overflow. There are many berryBushes in the locals before 110h.

comment:11 by m-kiewitz, 3 years ago

Added a script patch via commit ee41f64

I changed the arithmetic code, which means old saved games are not broken and do not need to get fixed via debug commands.

It is indeed an array overflow.

Please test this out and I also would be interested what the original interpreter did.

comment:12 by m-kiewitz, 3 years ago

Keywords: original script added
Owner: set to m-kiewitz
Resolution: fixed
Status: newpending

comment:13 by raziel-, 3 years ago

Hi Martin,

yes, the official Sierra update was applied (otherwise i get the nag screen about a script bug)

Tested with and without Sierra patch under ScummVM, no more crash :-)

Tested with and without Sierra patch under DosBox, nothing happens at all, ego will walk by just fine, no sign of an error or warning.
Maybe the original interpreter wasn't as sctrict or fixed such errors on the fly, or it simply ignored it?

In any way, awesome patch to ScummVM, thank you very much

comment:14 by m-kiewitz, 3 years ago

It seems the scripts are using that value plus a random number for the x-part of the coordinate of bushes, so I expect that the bushes are placed completely differently in the original interpreter - they may even be off screen (not shown at all).

And yes, the original interpreter ignored all sorts of things. It also didn't save pointers to objects as we do, but it simply used a straight 16 bit offset. And in case the scripts were buggy, it used that offset for calculations.

The original Sierra interpreter also didn't reset the stack, which caused for example temp variables to never get initialized. Sierra's compiler also doesn't seem to have checked for those things, that why there are few games, that never set certain temp variables, but use them and some games only work, when specific values are effectively set to those temp variables (they simply worked in Sierra's interpreter by accident).

We originally also didn't check for those issues, but then we found quite a few script bugs and then I implemented all sorts of detections. Official ScummVM releases have a few of those checks disabled.

Thanks for testing.

comment:15 by m-kiewitz, 3 years ago

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