Opened 6 months ago

Closed 5 months ago

#10758 closed defect (fixed)

QFG4: Autosave SCI Script Deletes Manual Savegames...

Reported by: Vhati Owned by: bluegr
Priority: high Component: Engine: SCI
Keywords: SCI32 has-pull-request Cc:
Game: Quest for Glory 4

Description

ScummVM 2.1.0git3770-g15306581ab (Oct 18 2018 04:27:32)
Windows 7 64bit
QFG4 CD (English)

ScummVM is deleting my saved games!?

As I create saved games at ever greater indeces, earliest ones are getting deleted. It seems I can only have one screen's worth of save slots, and the menu view continuously creeps downward. If I scroll up, there's a vast range of empty slots.

I think, early on, I deleted slot 2 and left a small gap. Dunno if that started this, but I certainly didn't delete 40 of them.

File - 5kb MD5 - Full MD5
RESOURCE.000 - 263dce4aa34c49d3ad29bec889007b1c - 1364ba69e3c0abb68cc0170650a56692
RESOURCE.AUD - c39521bffb1d8b19a57394866184a0ca - 71098b9e97e20c8941c0e4812d5f906f
RESOURCE.MAP - aba367f2102e81782d961b14fbe3d630 - 801a04cc6aa5d437681a2dd0b6545248
RESOURCE.SFX - 3cf95e09dab8b11d675e0537e18b499a - 7c858d7253f86dab4cc6066013c5ecec

Change History (18)

comment:1 Changed 6 months ago by Vhati

Summary: ScummVM is deleting all my early saved games!?QFG4: ScummVM is deleting all my early saved games!?

comment:2 Changed 6 months ago by Vhati

Might be deleting the chronologically earliest, not the slots with least index. A slot I'd overwritten just survived while surrounding slots didn't.

comment:3 Changed 6 months ago by digitall

Hmm... The only code I can find in the SCI engine to remove save states is this:
https://github.com/scummvm/scummvm/blob/master/engines/sci/engine/kfile.cpp#L130

But it does not look right, it almost looks like a case statement has been deleted such that the GET_FILENAME case is falling through and removing them!

comment:4 Changed 6 months ago by digitall

You could try running with scummvm -d 3 and whether the relevant message is being triggered.

comment:5 Changed 6 months ago by digitall

Summary: QFG4: ScummVM is deleting all my early saved games!?QFG4: When Running QFG4, SCI Engine Deletes Older Savegames...

Have amended title as I don't want to have to deal with lots of panicked users on forums! ;-)

Seriously, this is limited to savestate files and the function is built into the SCI engine's kernel file functionality. Not sure why this is used though and what the scripts are doing, but QFG4 does not appear to have any workaround / code modifications in that file so may need to some work to neuter the original scripts. I wonder if the original forced you to have only 10 savegames (since only that many fitted on screen) or similar... but that is speculation and until a SCI developer looks at the relevant game scripts and code and tracks this down :/

Sorry about this, and I can only suggest copying savegame states to a subdirectory for now for reference.

comment:6 in reply to:  4 Changed 6 months ago by Vhati

Replying to digitall:

You could try running with scummvm -d 3 and whether the relevant message is being triggered.

I think the deletion coincides with autosave creation.

I tried spamming new saves, never leaving the room I was in. Nothing was lost. I started filling in low indeces. No loss. Tried restarting ScummVM a couple times. No loss.

I left an explorer window open to watch my saves folder, and wandered from town, through the forest, to the swamp wisps. Just then, simply by walking, my chronologically oldest high index save files disappeared.

The modified time of the autosave file matched that moment.

The autosave's embedded screenshot was of the forest north of the swamp.

" " "
Running Quest for Glory IV: Shadows of Darkness (CD/DOS/English)
ADLIB: Starting driver in SCI1 mode
kValidPath() -> 0
kValidPath() -> 0
kValidPath() -> 0
kValidPath() -> 0
kValidPath() -> 0
kValidPath() -> 0
kValidPath() -> 0
kValidPath() -> 0
" " "
Those repeated lines occur when I bring up the menu. No other message signaled the deletion.

Sidenote: I tried restoring that autosave. "That game was saved under a different game or interpreter version. It cannot be restored."

Last edited 6 months ago by Vhati (previous) (diff)

comment:7 Changed 6 months ago by digitall

Had another look at the code in engines/sci/engine/kfile.cpp. You need to pass "./scummvm -d 3" and then enable the file debug flag using the console i.e. CTRL-SHIFT-F5 in game, then type "debugflag_enable file" and hit return, then "exit" to return to game.

The text console will now show file I/O ... I think this is probably a call to kFileUnlink, but looking at the game, the autosave is under the game script control and something odd is going on there.

comment:8 Changed 6 months ago by Vhati

@digitall:

enable the file debug flag using the console i.e. CTRL-SHIFT-F5 in game

CTRL-SHIFT-F5 didn't do anything for me.

So I went with args: -d 3 --debugflags=file


" " "
Running Quest for Glory IV: Shadows of Darkness (CD/DOS/English)
ADLIB: Starting driver in SCI1 mode
kFileIO(fileExists) 18.scr -> 0
kFileIO(fileExists) 18.scr -> 0
kFileIO(fileExists) 18.scr -> 0
# Restore Menu
kValidPath() -> 0
# Walk out of the adventurers' guild.
kFileIO(fileExists) 18.scr -> 0
kFileIO(close): 32100
kFileIO(unlink): sci.030
kFileIO(close): 32100
kFileIO(unlink): sci.031
kFileIO(close): 32100
kFileIO(unlink): sci.032
kFileIO(close): 32100
kFileIO(unlink): sci.033
kFileIO(close): 32100
kFileIO(unlink): sci.034
kFileIO(close): 32100
kFileIO(unlink): sci.035
Game name Glory save 0 desc Automatic Save ver
" " "

comment:9 Changed 6 months ago by digitall

Hmm... Probably there is a latent bug in the autosave script. This can be disabled in the in-game settings panel so try doing that and see if your savegames persist.

comment:10 Changed 6 months ago by Vhati

Copied my files from a backup.

Restored, disabled autosave, walked out of the adventurers' guild. No deletion, no log chatter.

Restored again, kept autosave enabled, walked out of the adventurers' guild. Files disappeared noisily, as reported above.

comment:11 Changed 6 months ago by digitall

Summary: QFG4: When Running QFG4, SCI Engine Deletes Older Savegames...QFG4: Autosave SCI Script Deletes Manual Savegames...

So definitively an issue with the QFG4 autosave SCI script... or it's interaction with the rest of the manual save slots. Will need a SCI engine developer to take a look.

comment:12 Changed 6 months ago by Vhati

@digitall:

using the console i.e. CTRL-SHIFT-F5 in game

Ah ha! I found a shortcut that works: CTRL-SHIFT-d

comment:13 Changed 6 months ago by digitall

Sorry! Got confused between the key combination for triggering the ScummVM Global Main Menu (GMM) which is CTRL-F5 (unless the game uses that, when it is CTRL-Shift-F5), and the one for triggering the debugger / debug console (if implemented by the engine) which is usually CTRL-d, but again if the game uses that, it will be usually CTRL-Shift-d.

comment:14 Changed 6 months ago by Vhati

@digitall:

I wonder if the original forced you to have only 10 savegames

I think you are correct.

https://github.com/scummvm/scummvm/blob/master/engines/sci/engine/kfile.cpp
bp_kernel GetSaveFiles break
bp_kernel FileIOOpen break
bp_kernel FileIOClose break
bp_kernel FileIOUnlink break
bp_kernel MakeSaveFileName break


Script 0, Glory::save() calls kGetSaveFiles() and localproc_21c5() in a loop. The procedure is responsible for deleting everything. Looks like kCheckFreeSpace() is the loop's condition?

(procedure (localproc_21c5 param1 param2 param3 param4 &tmp temp0 temp1 newFile temp3 newStr)
	(= newStr (Str new:))
	(MakeSaveCatName (newStr data?) (global1 name?))
	# kFileIO(Open): 000f:08b8 ('fake.cat'), 2 = 32100
	((= newFile (File new:)) name: (newStr data?) open: 2)
	(= temp1 0)
	# Loop calls kFileIO(WriteRaw) to write every slot's title to the catalog.
	(while (< temp1 param3)
		(if (!= temp1 param1)
			(= temp3 (param4 at: temp1))
			(newStr
				at: 0 (& temp3 $00ff)
				at: 1 (& (>> temp3 $0008) $00ff)
				at: 2 0
			)
			(newFile write: (newStr data?) 2)
			(= temp0 (Str new: 36))
			(temp0 copyToFrom: 0 param2 (* temp1 36) 36)
			(newFile write: (temp0 data?) 36)
			(temp0 dispose:)
			(= temp0 0)
		)
		(++ temp1)
	)
	(newStr at: 0 255 at: 1 255)
	# kFileIO(Close): 32100 = 1
	(newFile write: (newStr data?) 2 close: dispose:)
	# kMakeSaveFileName: 000f:08b8 ('sci.033'), 0001:4c0d ('Glory'), 32 = 000f:08b8
	(MakeSaveFileName
		(newStr data?)
		(global1 name?)
		(param4 at: param1)
	)
	# kFileIO(Unlink): 000f:08b8 ('sci.033') = 1
	(FileIO fiUNLINK (newStr data?))
	(newStr dispose:)
)

Throughout the loop, my breakpoint reports "kCheckFreeSpace: 000f:0000 ('') = 1"

Save has a couple of loops. This might be it?

					(while
						(and
							(> temp9 0)
							(or
								(not (CheckFreeSpace (global29 data?)))
								(>= temp9 20)
							)
						)
						(localproc_21c5 (- temp9 1) newStr temp9 temp1)
						(= temp9
							(GetSaveFiles
								(global1 name?)
								(newStr data?)
								(temp1 data?)
							)
						)
					)

The loop has another condition "(>= temp9 20)". That looks suspicious. After all the deletions, I'm left with 20 files.


A 20 slot cap has been patched away in two other places.

https://github.com/scummvm/scummvm/blob/b749116cbe1f710caa6fa225eef49af33e769e79/engines/sci/engine/script_patches.cpp#L254

Last edited 6 months ago by Vhati (previous) (diff)

comment:15 Changed 6 months ago by Vhati

Occurs in the floppy edition under ScummVM, too.

Script 0, localproc_21c0() is the equivalent procedure.


ScummVM 2.1.0git3797-ge7d23d2cd9 (Oct 25 2018 04:17:12)
Windows 7 64bit
QFG4 Floppy 1.1a + note patch (English)

File - 5kb MD5 - Full MD5
RESOURCE.000 - f64fd6aa3977939a86ff30783dd677e1 - ff42260a665995a85aeb277ad80aac8a
RESOURCE.MAP - d10a4cc177d2091d744e2ad8c049b0ae - 3695b1b0a1d15f3d324ea9f0cc325245
RESOURCE.SFX - 3cf95e09dab8b11d675e0537e18b499a - 7c858d7253f86dab4cc6066013c5ecec

comment:16 Changed 5 months ago by Vhati

Pull Request: SCI32: Fix QFG4 autosave deleting manual saves

comment:17 Changed 5 months ago by digitall

Keywords: has-pull-request added

comment:18 Changed 5 months ago by bluegr

Owner: set to bluegr
Resolution: fixed
Status: newclosed

Great work! Many thanks for the effort you put on this.

The pull request has been merged, so this can be closed now.

Note: See TracTickets for help on using tickets.