ALL: The new platform options seems to break the parsing of other options

[eriktorbjorn]

This appears to be a regression from ​https://github.com/scummvm/scummvm/pull/6773

As I understand it, GUI options are stored in two ways. There is the human readable one in scummvm.ini, and the binary one produced by parseGameGUIOptions(), which produces a string which we then usually calls contains() in to see if it has a particular option.

The options are defined in gui_options.h, with values from GUIO_NONE ("\x00") through GUIO_NOLANG ("\x33") for global options, and the game options GUIO_GAMEOPTIONS1 ("\xe0") through GUIO_GAMEOPTIONS32 ("\xff").

And then there is GUIO_PLATFORM_PREFIX ("\x40"), followed by a platform code from kPlatformApple2GS_VAL ("\x01") through kPlatformNintendoSwitch_VAL ("\x27"). So now a single byte can mean more than one thing?

At the very least, this breaks the music device setting when adding a new SCI game. E.g. if I add Space Quest III, it only offers me "<default>", "No music", and "AdLib emulator". That's because it does this to determine if the game explicitly specifies MIDI options:

	const Common::String allFlags = MidiDriver::musicType2GUIO((uint32)-1);
	bool hasMidiDefined = (strpbrk(_guioptions.c_str(), allFlags.c_str()) != nullptr);

But I could easily see it break any code that calls parseGameGUIOptions() and then uses contains() on it. I just haven't tried to verify it.

[eriktorbjorn]

Some more notes along the way.

There is only one place where parseGameGUIOptionsPlatforms() is called, and that's from parseGameGUIOptions(). So we only need to worry about the latter, and fortunately that's not called in that many places either.

Beneath a Steel Sky uses it to test for GUIO_NOSPEECH ("\x03"). This would potentially clash with kPlatform3DO_VAL, but by pure luck I don't think that's an issue.

The SCUMM engine uses it to test for several things:

• GAMEOPTION_ORIGINALGUI (GUIO_GAMEOPTIONS4)
• GAMEOPTION_COPY_PROTECTION (GUIO_GAMEOPTIONS7)
• GAMEOPTION_ENHANCEMENTS (GUIO_GAMEOPTIONS2)
• GAMEOPTION_LOWLATENCYAUDIO (GUIO_GAMEOPTIONS5)
• GAMEOPTION_AUDIO_OVERRIDE (GUIO_GAMEOPTIONS3)
• GAMEOPTION_REMASTERED_AUDIO (GUIO_GAMEOPTIONS8)
• GAMEOPTION_TTS (GUIO_GAMEOPTIONS9)
• GAMEOPTION_TRIM_FMTOWNS_TO_200_PIXELS (GUIO_GAMEOPTIONS1)

None of these can clash with the platform options.

The SCI engine does something with it, but only to append the language settings?

So the main use is probably the options dialog, and I'll put that in a separate comment.

[eriktorbjorn]

The options dialog checks for single flags:

• GUIO_NOASPECT ("\x13"), possible clash with kPlatformNES_VAL
• GUIO_LINKMUSICTOSFX ("\x31")
• GUIO_LINKSPEECHTOSFX ("\x30")
• GUIO_MIDIADLIB ("\x0a"), possible clash with kPlatformDOS_VAL
• GUIO_NOMIDI ("\x05"), possible clash with kPlatformAmiga_VAL
• GUIO_NOMUSIC ("\x02"), possible clash with kPlatformApple2_VAL
• GUIO_NOSFX ("\x04"), possible clash with kPlatformAcorn_VAL
• GUIO_NOSPEECH ("\x03"), possible clash with kPlatform3DO_VAL
• GUIO_NOSPEECHVOLUME ("\x32")
• GUIO_NOSUBTITLES ("\x01"), possible clash with kPlatformApple2GS_VAL

It also checks for groups of flags:

All MIDI flags:

• GUIO_MIDIPCSPK ("\x07"), possible clash with kPlatformAtariST_VAL
• GUIO_MIDICMS ("\x08"), possible clash with kPlatformC64_VAL
• GUIO_MIDIPCJR ("\x09"), possible clash with kPlatformAmstradCPC_VAL
• GUIO_MIDIADLIB ("\x0a"), possible clash with kPlatformDOS_VAL (possibly the source of the bug I initially saw)
• GUIO_MIDIC64 ("\x0b"), possible clash with kPlatformPC98_VAL
• GUIO_MIDIAMIGA ("\x0c"), possible clash with kPlatformWii_VAL
• GUIO_MIDIAPPLEIIGS ("\x0d"), possible clash with kPlatformCoCo_VAL
• GUIO_MIDITOWNS ("\x0e"), possible clash with kPlatformCoCo3_VAL
• GUIO_MIDIPC98 ("\x0f"), possible clash with kPlatformFMTowns_VAL
• GUIO_MIDISEGACD ("\x10"), possible clash with kPlatformLinux_VAL
• GUIO_MIDIGM ("\x12"), possible clash with kPlatformPCEngine_VAL
• GUIO_MIDIMT32´ ("\x11"), possible clash with kPlatformMacintosh_VAL`
• GUIO_MIDIMAC ("\x17"), possible clash with kPlatformPS2_VAL

Aside note: I would have thought that this would mean that a Macintosh SCI game would only offer MT-32 music, but that's not the case.

All render modes:

• GUIO_RENDERHERCGREEN ("\x18"), possible clash with kPlatformPS3_VAL
• GUIO_RENDERHERCAMBER ("\x19"), possible clash with kPlatformXbox_VAL
• GUIO_RENDERCGA ("\x1a"), possible clash with kPlatformCDi_VAL
• GUIO_RENDEREGA ("\x1b"), possible clash with kPlatformIOS_VAL
• GUIO_RENDERVGA ("\x1c"), possible clash with kPlatformAndroid_VAL
• GUIO_RENDERAMIGA ("\x1d"), possible clash with kPlatformOS2_VAL
• GUIO_RENDERFMTOWNS ("\x1e"), possible clash with kPlatformBeOS_VAL
• GUIO_RENDERPC98_256C ("\x1f"), possible clash with kPlatformPocketPC_VAL
• GUIO_RENDERPC98_16C ("\x20"), possible clash with kPlatformMegaDrive_VAL
• GUIO_RENDERAPPLE2GS ("\x21"), possible clash with kPlatformSaturn_VAL
• GUIO_RENDERATARIST ("\x22"), possible clash with kPlatformPippin_VAL
• GUIO_RENDERMACINTOSH ("\x23"), possible clash with kPlatformMacintoshII_VAL
• GUIO_RENDERMACINTOSHBW ("\x24"), possible clash with kPlatformShockwave_VAL
• GUIO_RENDERCGACOMP ("\x25"), possible clash with kPlatformZX_VAL
• GUIO_RENDERCGABW ("\x26"), possible clash with kPlatformTI994_VAL
• GUIO_RENDERPC ("\x27"), possible clash with kPlatformNintendoSwitch_VAL
• GUIO_RENDERZX ("\x28")
• GUIO_RENDERC64 ("\x29")
• GUIO_RENDERVGAGREY ("\x2A")
• GUIO_RENDERPC98_8C("\x2B")
• GUIO_RENDERWIN_16C ("\x2D")
• GUIO_RENDERWIN_256C ("\x2C")

Aside note: The kRenderWin256c and kRenderWin16c entries seem wrong in s_renderGUIOMapping[]?

I do have one game for OS/2. I guess I should check if it incorrectly offers me Amiga rendering. Can't do it at the moment, though.

[eriktorbjorn]

It's a bit more complicated than I first thought for Macintosh games. The Mac version of Castle of Dr. Brain offers me the following music drivers:

• <default>
• No music
• Sndio
• FluidSynth
• MT-32 Emulator
• Embedded Audio Synthesis
• TiMidity

In 2.9.1 it offered the full (?) list, probably because the SCI engine doesn't specify which ones it wants. That, at least, is probably caused by it mistaking the Macintosh platform flag for the MT-32 Emulator music flag.

[eriktorbjorn]

In some cases, we use Common::checkGameGUIOption() or Common::checkGameGUIOptionLanguage(). Haven't checked if they cause any problem.

[eriktorbjorn]

I incorrectly stated that the platform options aren't automatically updated when a game was launched. I was wrong. I just started a SCI game, and the platform options were updated alongside others.

It still keeps the old music_driver setting in scummvm.ini, so it doesn't break your sound. But it may very well be one that you're no longer allowed to set.

[leny100]

Hi eriktorbjorn, thank you for this detailed report. This issue should be fixed with ​https://github.com/scummvm/scummvm/pull/6944