Opened 2 months ago

Closed 3 weeks ago

#11082 closed defect (fixed)

QFG4: exporting the character requires deleting the "a:"

Reported by: lwcorp Owned by: sluicebox
Priority: normal Component: Engine: SCI
Keywords: export Cc:
Game: Quest for Glory 4

Description

When the game asks you to export your character it suggests a:shadows.sav.
It then saved it as qfg4-cd-a with 0 bytes.
When deleting the a: it finally worked and it saved it as qfg4-cd-shadows.sav

I suggest you just ignore the [letter]: part.
Also, it's a bit confusing you add qfg4-cd- prefix without asking.

Attachments (1)

version.png (1.7 KB ) - added by lwcorp 2 months ago.

Download all attachments as: .zip

Change History (11)

by lwcorp, 2 months ago

Attachment: version.png added

comment:1 by digitall, 6 weeks ago

Component: Port: Win64Engine: SCI

comment:2 by m-kiewitz, 6 weeks ago

The qfg4-cd- prefix is the prefix of the game itself and that's added to all the game files, otherwise conflicts may happen.
Afaik we also added informational text boxes to the other games when the export happens, so the user knows what's going on.

No one managed to complete QfG4 before, we already removed the "a:" from the other games.
Congrats.

Can you please provide a saved game right before the export happens?
I will look into it.

comment:3 by m-kiewitz, 6 weeks ago

Back then the games were meant to save to a floppy drive, that's why "a:" is the default. That won't work in ScummVM, because ScummVM ignores drive letters for saving game files.
I should also look into why the game file is 0 bytes when the drive letter is specified.

comment:4 by sluicebox, 5 weeks ago

This is really exciting that someone made it to the end.

I have a save near the end but it's easier to test this by just warping to room 52 to kick off the export sequence from anywhere. Poor John Rhys-Davies, they had him record audio for all the export I/O messages! What a professional.

The "a:" is removed from the other QFG games in kDrawControl but that's only for SCI16. For this one we could just trim "a:shadows.sav" with a message workaround and keep the kernel code clean.

The reason "qfg4-cd-a" appears to have zero bytes is that the export has been written to an NTFS alternate data stream. ScummvM asks Windows to open "qfg4-cd-a:shadows.sav" and Windows dutifully creates the file "qfg4-cd-a" with a stream named "shadows.sav" and then successfully writes the save to it. If lwcorp had a way to view that stream they'd see the full export there.

So good news, exporting works, and the zero-byte file isn't a QFG4 bug. We should still filter colons and similar characters in the kernel, but that's a issue for any game that takes arbitrary filename input. Removing "a:" from the default text is all that's needed to bring QFG4's export in line with the other games.

m-kiewitz, does that sound right to you? I can add that workaround if it does.

comment:5 by m-kiewitz, 5 weeks ago

@sluicebox
That sounds fine.
Yes, I didn't even think about adding kernel hacks for this one.

I also wanted to add a popup explaining where the saved character is located like the ones that are shown for the other QfG games.

As you said ScummVM-system should probably either deny file names that include the ":" character. Or maybe filter it out, I'm not sure what's the better approach for this, at least for the Windows platform. Maybe even for all platforms, idk.

And what, audio even exists for error messages? Wow, just wow. QfG 4 is one of Sierra's best works.

comment:6 by m-kiewitz, 5 weeks ago

I wonder if there are potential overflows in QfG4 character files as well.
If I remember correctly, I fixed QfG 3 in that regard.

...
It seems it actually does have these too:
https://github.com/Blazingstix/QFGImporter/blob/master/QFGImporter/FileSpecs/QFG4.txt

So that will require a script patch.

comment:7 by sluicebox, 5 weeks ago

Great, I will add the workaround to remove the "a:" from the default. I verified it's the same string in the German version.

That popup is helpful on the other games. Someone asked me how importing worked and I didn't know and that message explained it to me.

It's soothing to hear Rhys-Davies gently break the bad news that "There are too many files in that directory."

I haven't worked with the character export files yet, but I do know that QFG4 has a checkered history with them. 1.0 floppy had a script bug that corrupted them. I forget if that was on import or export. There is a separate import script patch that they distributed after the final big "1.1a" floppy for what looked like a different bug. It's extra confusing because the German version, which came afterwards, contains some patches but excludes others.

I don't know if everything was fixed by the end of the final CD version.

I have run out of major QFG4 issues so I will go through the versions and patches in the next few days and try to figure out the timeline for what problems got fixed. Hopefully we can backport those to earlier versions. That QFGImporter link will be a big help, I'm glad so many fans have already done that work. I'm excited to learn how this part works.

comment:8 by sluicebox <22204938+sluicebox@…>, 5 weeks ago

In 00548e15:

SCI32: Remove QFG4 "a:" prefix from export filename

Trac #11082

comment:9 by sluicebox, 4 weeks ago

I took a look.

Export is the same in all versions. The bug I'd noticed in 1.0's File:writeString didn't affect this game. (Passing multiple parameters was broken)

There are three versions of the Import script:

  • Floppy (all versions)
  • Import Patch
  • CD

The Import Patch is a pretty funny "i give up!" script, it prompts the user for the raw values to use for the attributes! It was released around the same time as the big 1.1a patch set, the patch notes mention that you should get it from the BBS if you have import problems.

The CD version of Import makes three changes to a function but I don't know what the story is there.

So it seems like that leaves:

  • Patching export overflow so that QFG5 can import by capping values to the format's maximum
  • Filtering out ":" and other such characters from kernel IO name input. I don't think this is high priority now that "a: is gone but it would be nice.

I'm thinking about making tickets for those two and closing this one down now that "a:" is removed.

comment:10 by sluicebox, 3 weeks ago

Owner: set to sluicebox
Resolution: fixed
Status: newclosed

On closer inspection, the overflow problem is with the encryption format, and I don't think can be fixed. I thought the spec description was saying that individual game attributes, like experience, were exceeding 10,000, which the format can't represent. If that were the case then we could patch the script to prevent that before export. Instead, it's the encryption scheme where each short is calculated from all previous shorts, and the values accumulate until they overflow. No individual attribute is too high or invalid, it's just a bad format that can't represent normal game state.

Closing this one since the "a:" has been removed.

Note: See TracTickets for help on using tickets.