Opened 12 years ago

Closed 6 years ago

#5958 closed defect (outdated)

TINSEL: DW1 Crash When Loading Savegame

Reported by: (none) Owned by: csnover
Priority: blocker Component: Engine: Tinsel
Version: Keywords: has-save-game has-backtrace
Cc: Game: Discworld

Description

As explained here: http://forums.scummvm.org/viewtopic.php?t=10850&sid=e6fea1eff4a9c5923b2e45d06025b6bf I saved my game on my Android HTC Desire on the night version of the map screen and then next time I tried to load it the game loaded for about a second and then the emulator bombed out. I tried updating to the latest daily build but had the same problem. I also tried loading the save game with the Windows version of ScummVM but the same thing happens.

Game save attached.

Ticket imported from: #3482728. Ticket imported from: bugs/5958.

Attachments (1)

dw-cd.001 (3.2 KB ) - added by SF/*anonymous 12 years ago.
Save file that causes the problem

Download all attachments as: .zip

Change History (20)

by SF/*anonymous, 12 years ago

Attachment: dw-cd.001 added

Save file that causes the problem

comment:1 by digitall, 12 years ago

Summary: Crash when reloading game saveTINSEL: DW1 Crash When Loading Savegame

comment:2 by digitall, 12 years ago

For any other developers looking at this: Replicated with attached savegame on Linux x86_32. The savegame has a corrupt game state, which is causing the following error: "GetInvObject(0): Trying to manipulate undefined inventory icon!"

Tracing this to the second call of GetInvObject in Tinsel::HoldItem (int item, bool bKeepFilm). The value of item here is 0, which results in the error... Running a load from a normal savegame gives -1... I think this value is the item "held" on the cursor by the Tinsel Engine at the point of save.

By adding an assert in engine/tinsel/dialogs.cpp at HoldItem:, the following backtrace is shown: #4 0x0808b390 in Tinsel::HoldItem (item=0, bKeepFilm=false) at engines/tinsel/dialogs.cpp:1872 #5 0x08061995 in Tinsel::DoSync (s=...) at engines/tinsel/saveload.cpp:429 #6 0x08061b35 in Tinsel::DoRestore () at engines/tinsel/saveload.cpp:471 #7 0x080620ac in Tinsel::ProcessSRQueue () at engines/tinsel/saveload.cpp:579 #8 0x0807bb1a in Tinsel::TinselEngine::run (this=0x83dacc8) at engines/tinsel/tinsel.cpp:983

comment:3 by digitall, 12 years ago

A further note: If you strip off the savegame zlib compression i.e. rename with .gz suffix then run them through gunzip, then the attached "bad" savegame is 64191 bytes long, compared to my "good" savegames which are 64243 bytes... i.e. 52 bytes shorter, but this is not a simple truncation as the 0xFACEFEED termination field is present... Looks like one of the internal records is omitted or corrupted?

I'd normally write this off, but it could indicate an endian or similar issue?

The heldItem value is likely due to the static being globally initialised and not initialised on restart (which can be done in game by the F1 menu)... and this should be refactored anyway.

comment:4 by wjp, 12 years ago

I think the different size may just mean you have a different DW version. (Specifically with a different number of global variables.)

Since the savegame format doesn't seem to check that (which we may want to change?), this means this backtrace is unfortunately probably not of the reported crash.

comment:5 by digitall, 12 years ago

Hmm... Quite possible.

Crundy: Could you make file MD5sums for your DW1 datafiles so we can confirm your version? A tool such as http://md5summer.org/ will allow you to do this on Win32, and the list below shows you the main 5 files we are interested in...

MD5sums for my DW1 versions relevant datafiles: 9fd32b3ac7a52a4e2d46d62e99691166 *english.idx dba5cd0be3343dd17b7ce59b71eec881 *english.smp 3fd92a00ed36d12585421f1f0ca9c50e *english.txt 46f249ee4b7f51295701efcfaf81d256 *midi.dat ae28af0f89ab1afc964cbcf85cc8097d *index

comment:6 by (none), 12 years ago

(Sorry, SF didn't save my comment, trying again)

OK my MD5s are:

d4b11031e59bc85b187ba5245d9eb943 *ENGLISH.IDX b9973da5c04aa7407f0efe621406d296 *ENGLISH.SMP 1cda31864620af353974a18203c55393 *ENGLISH.TXT 1f7b2dbeeb595d01e8211bf20842e0b6 *INDEX 3375b0c85658efd8a511608b0aaa009f *MIDI.DAT

Weird thing: I did note that the required name of the savegame changed. When I was initially playing it ages ago when the error first happened the save was called dw.001. When I came back to trying it again and reinstalled ScummVM it wouldn't recognise the save. I created a dummy save which came up as dw-cd.001, so I renamed my save to that, the engine recognised it, but the crash still occurs.

comment:7 by digitall, 12 years ago

Grundy: Thanks. Could you also confirm if you have .SCN or .GRA files for each scene, and confirm your version from this gallery?: http://www.mobygames.com/game/discworld/cover-art

I have the DOS UK Limited Edition CD version with .SCN files.

comment:8 by dreammaster, 12 years ago

I tried loading this, and it loads on my GRA version. It's just that there's a problem once it tries to starteexecuting the scripts. Briefly:

It seems to load the savegame properly. Then the RestoredProcess executes script, which does a call to the 'STAND' library routine, with a 'hFilm' of 4. Locking the memory for that handle, gives invalid film data with a numReels of several million. :P <dreammaster> It's been a while since I worked on Tinsel, so I don't know if the given script that runs is only run when you restore a savegame, and the hFilm is correct, or the memory it's attached to is valid or not. Ideally, you'd need to try from a different savegame entering the scene and seeing if the STAND library routine is called when you enter the scene versus restoring a savegame, to determine if the hFilm=4 is correct, and when the routine is called. And if it's called on scene enter without errors, you'd need to track down how the locked film data was corrupted.

comment:9 by (none), 12 years ago

Hmm, I don't recognise the case from any on the page you linked. It's actually my dad's (unwanted present) and so I took it off him but I may have given it back to him trying to convince him to play it. It was a jewel case though. I seem to have GRA files.

Thinking about it I may have downloaded this version of the files I have, as I lived quite a way off when I first played dw on my phone. If I get the disc back I should be able to redo the md5s on the files and find out. Would my savegame work with a different set of the files?

comment:10 by sev-, 12 years ago

What is the status of this item?

comment:11 by digitall, 12 years ago

Owner: set to dreammaster

comment:12 by digitall, 12 years ago

dreammaster: I assume the conclusion here is that we know how the savestate is broken, but not what went wrong to cause this corruption/incorrect state to be saved?

Also, could the cause of this corrupted savestate have been fixed by your recent Tinsel savegame fixes i.e. 2dee92a908b84ae870bfdbfd00318549485b7984 ?

comment:13 by dreammaster, 12 years ago

Sorry, I should have been clearer in the previous posting. What we really need was a nearby savegame so it could be tested whether creating a new savegame in the night map also causes a crash.

As for the recent fix, this was just about fixing 1.5 savegames; as per my previous message, the main savegame loaded correctly, it just that a script executed as part of the finalisation of the restore process had an invalid parameter, and I wasn't sure if it was caused by the savegame or a bug from saving/restoring in that scene generally.

comment:14 by SF/softwarespecial, 10 years ago

I also get this bug. The thing is I was using the Floppy disk version and never ever had this happen. Then I used the normal CD version for a little bit didn't happen but never got to far. Then I used the fixed CD version I think they called it the directors cut and it happened all the time.

comment:15 by dreammaster, 10 years ago

That makes it more concerning, although unfortunately, I don't have this "Director's cut" version. Hopefully, though, my previous notes may help in tracking down the problem: if it's still due to an invalid parameter to "Stand" (such as '4') in an invalid savegame when reloading the scene, try double-checking against entering the scene normally from a slightly earlier valid savegame.

The earlier backtrace error about 'HeldItem' is normally just due to someone trying to use a savegame on a different version of the game (GRA versus SCN, or single country versus international). So I'm more inlcuded to trust that if this is the same problem, it is in the call to STAND as my earlier report notes. So the question to track down is, why does the value of "4" get passed.. is it a valid value, or if nto, why is it stored in the savegame to begin with.

comment:16 by csnover, 6 years ago

Priority: normalblocker

Raising all identified crasher, hang, and memory violation bugs which I could not fully triage myself to blocker priority for the next release.

comment:17 by csnover, 6 years ago

Owner: dreammaster removed

Removing all owners from release blockers so they can be reclaimed during the release process. If you are the previous owner and would graciously fix this bug for the next release, please go ahead and re-add yourself as owner.

comment:18 by csnover, 6 years ago

Keywords: has-save-game has-backtrace added

comment:19 by csnover, 6 years ago

Owner: set to csnover
Resolution: outdated
Status: newclosed

So while it sounds like there may still be a problem with the engine generating save games with invalid data, since this has been open for several years without anyone actually being able to provide a pre-corruption save to isolate the problem, and without any new comments or reports of such a problem, I am going to close the ticket as outdated for now. If someone is able to reproduce this error condition and can provide the needed information to address the problem, please go ahead and do so and this ticket can be reopened in the future.

Note: See TracTickets for help on using tickets.