SCI: KQ7: Odd behavior for disabling subtitles

[antoniou79]

This was tested on ScummVM 2.8.1 and recent dev build of 2.9.0git on Windows 10.

Under some conditions subtitles for the game (and the early disclaimer warning about them when launching the game) are not disabled despite the Audio setting for the game (from the ScummVM launcher, Game Options) is set to "Speech".

This was brought up on the ScummVM forum here:
​https://forums.scummvm.org/viewtopic.php?p=100028#p100028

My steps to reproduce this (some steps may be redundant):

1. Have ScummVM's "Global Options" for "Audio" set to "Both"
2. Add the KQ7 game to ScummVM
3. Launch the game. It will start with the disclaimer and subtitles enabled, as expected.
4. Exit the game, go to "Game Options" specifically for the game, override the global settings, set Audio to "Speech", click ok.
5. Re-launch the game. It continues to show the warning and subtitles.

At this point it's expected that the game would *not* show the disclaimer warning nor the subtitles. Yet, even the entry in the scummvm.ini file is not updated to have a key-value for "subtitles" as it should.

The issue can be resolved, if the user goes (again) to the Game Options for the game, and sets the Audio to "Text", clicks Apply (at this point the scummvm.ini entry for the game gets a key-value for "subtitles" as "true"). Then, go to Game Options -> Audio again and set the Audio to "Speech", which now does update scummvm.ini and sets "subtitles" to "false".

[eriktorbjorn]

There seems to be something wrong with this part of OptionsDialog::apply():

if (subtitles != ConfMan.getBool("subtitles", _domain)) {
	ConfMan.setBool("subtitles", subtitles, _domain);
	_subToggleDesc->setFontColor(ThemeEngine::FontColor::kFontColorNormal);
}

You've changed the "subtitles" setting to false, but it's already false by default for this game so it doesn't see fit to write the setting to the file? Something like that, anyway?

[m-kiewitz]

Our settings within ScummVM are just trying to set subtitles + audio for games, but there are some games that reset these in game variables to the default on startup, and thus ignore what we set.

In these cases the only solution would be a script patch.

Would have to look into this specific game to be sure that it's this.

Which KQ7 do you use? 1.51 or 2.0?

[eriktorbjorn]

The version file of my copy of King's Quest VII says "2.00b". But I'm still leaning towards an error int he ScummVM options dialog. Let's try from a completely blank slate.

I started ScummVM and changed the global audio settings to "Both". This is now my scummvm.ini:

[scummvm]
gui_browser_native=true
mute=false
speech_volume=192
native_mt32=false
mt32_device=null
confirm_exit=false
gui_use_game_language=false
gui_scale=100
gui_return_to_launcher_at_exit=false
gui_disable_fixed_font_scaling=false
subtitles=true
gm_device=auto
sfx_volume=192
music_volume=192
autosave_period=300
music_driver=auto
opl_driver=auto
aspect_ratio=true
versioninfo=2.9.0git10472-gec1c846eaff
speech_mute=false

I add the game (selecting the "DOS" version), but change the Audio settings to override the global settings so that I get "Speech" instead without subtitles. This is the entry I get for the game:

[kq7]
description=King's Quest VII: The Princeless Bride (DOS/English)
gmm_save_enabled=false
extrapath=dists/engine-data/
path=/path-to/kq7
enable_video_upscale=true
engineid=sci
gameid=kq7
language=en
music_driver=auto
platform=pc
enable_hq_video=true
opl_driver=auto
speech_mute=false
guioptions=sndLinkSpeechToSfx sndLinkMusicToSfx noAspect gameOptionA gameOptionD gameOptionI lang_English

Note that there is no "subtitles" setting in the entry.

I think what happens is that when the options dialog changes the settings, it checks if there is a setting specifically for the game and gets the default value (false). This is the same as what you're trying to save, so it doesn't think it necessary to explicitly write the setting to the file. When the engine checks the setting, it sees that there is no game-specific setting, so you get the global setting (true).

If you change the game-specific setting to one where "subtitles" is true, the dialog does see that it has changed and writes it to the file. Then when you change it back to voices only, the setting has once again changed, so it writes it to the file. Now everything works as expected.

[m-kiewitz]

Can you please try an earlier ScummVM version?
Maybe this is a regression in the settings, but it's weird because I would expect such a problem to get reported basically immediately elsewhere.

I think back then I programmed it in a way, that the game itself changes the game-specific settings too, so if the game sets defaults or changes subtitles on/off, that change is saved to the game specific settings, at least that's what it was originally. And yes, when the global setting is the same as the game specific setting, the game specific setting is not saved individually.

[eriktorbjorn]

I went back to a much earlier version, and there it worked. I then tried bisecting for a bit. Here's one point where it broke:

25401cdf922ca629d8e6c8d8664a07b0c755b897 is the first bad commit
commit 25401cdf922ca629d8e6c8d8664a07b0c755b897
Author: Filippos Karapetis <bluegr@gmail.com>
Date:   Thu Jun 30 17:14:35 2022 +0300

    GUI: Add missing domain when checking for subtitle config - bug #13629

    This disallowed selecting "both" for speech and subtitles, as the
    subtitles setting was never saved. A regression from d6dbf721b62e773f.

 gui/options.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I don't know if this is the only point. I deliberately bisected a very small interval, because I had a hunch that it would break here.

[eriktorbjorn]

Originally, it would always write both "subtitles" and "speech_mute" to the config manager, like so:

ConfMan.setBool("subtitles", subtitles, _domain);
ConfMan.setBool("speech_mute", speech_mute, _domain);

This was changed in ​https://github.com/scummvm/scummvm/commit/d6dbf721b62e773f529276f5b1c0c3ce59df6f47 ("ALL: add support for --subtitles") to this:

if (subtitles != ConfMan.getBool("subtitles")) {
	ConfMan.setBool("subtitles", subtitles, _domain);
	_subToggleDesc->setFontColor(ThemeEngine::FontColor::kFontColorNormal); 
}
ConfMan.setBool("speech_mute", speech_mute, _domain);

And then, finally, ​https://github.com/scummvm/scummvm/commit/25401cdf922ca629d8e6c8d8664a07b0c755b897 ("GUI: Add missing domain when checking for subtitle config - bug #13629") changed it to:

if (subtitles != ConfMan.getBool("subtitles", _domain)) {
	ConfMan.setBool("subtitles", subtitles, _domain);
	_subToggleDesc->setFontColor(ThemeEngine::FontColor::kFontColorNormal); 
}
ConfMan.setBool("speech_mute", speech_mute, _domain);

I'm not sure I understand even the first change here. Maybe @sev knows?

[eriktorbjorn]

This is a tempting change to make, but I don't know if I'm on the right track:

diff --git a/gui/options.cpp b/gui/options.cpp
index d000413fae8..b586d17cae9 100644
--- a/gui/options.cpp
+++ b/gui/options.cpp
@@ -1086,7 +1086,7 @@ void OptionsDialog::apply() {
                                        break;
                                }
 
-                               if (subtitles != ConfMan.getBool("subtitles", _domain)) {
+                               if (!ConfMan.hasKey("subtitles", _domain) || subtitles != ConfMan.getBool("subtitles", _domain)) {
                                        ConfMan.setBool("subtitles", subtitles, _domain);
                                        _subToggleDesc->setFontColor(ThemeEngine::FontColor::kFontColorNormal);
                                }

[sev-]

Would be nice to get fixed before the release.

[sluicebox]

This seems to be fixed? Using the steps to reproduce from @antoniou79:

• I can reproduce with Windows release 2.8.1
• I can not reproduce with today's Windows daily build from master

Using KQ7 2.00b and a fresh default config for each test.

I don't see any recent commits referenced in this ticket; does anyone know what happened?

[antoniou79]

Looks like this commit may have fixed this:
​https://github.com/scummvm/scummvm/commit/52b7b3194cf496fa8a71c9c4208ec78b92e8aec8

The commit was by @sev to fix this ticket:
https://bugs.scummvm.org/ticket/15262

I didn't bisect. I just commented out the two lines added by the above commit, built ScummVM and tested, and confirmed that the bug was back.

For already added games, for which "Speech" was set in their specific settings, overriding the Global audio settings, and Global Audio was "Both", and no key-value for subtitles was written to ScummVM config file, this commit will "change" the game specific setting so it reflects the Global Audio setting (ie. Game Options -> Audio will now show "Both"). Changing the value will correctly write back to the config.ini.

[eriktorbjorn]

How about this case?

• Configure ScummVM's global setting to "Speech".
• Add the game. Override audio settings, but change only something else than the speech setting, e.g. change the AdLib emulator. The game is shown as having "Speech" as its setting, and starting the game works fine.
• Change the global setting to "Both". Now the game options also shows "Both", and the game will complain about the subtitles.

So I guess the game's override setting wasn't saved, meaning that when the global setting changed that also affected the game's setting? That's a bit confusing, since I don't think there's any visible indicator that it's still using the global setting.

[sluicebox]

Closing this ticket as fixed. I'm sure that @antoniou79 is right, and the reported behavior was fixed by @sev in 52b7b3194cf496fa8a71c9c4208ec78b92e8aec8 as part of #15262. Also, it doesn't appear that any of this was specific to SCI. KQ7 was just easily noticeable because of the immediate message box.

I suspect that there will always be a logic hole the matrix of:

• Three GUI radio buttons that map to two config booleans
• Overriding global config at the game level
• Engines altering these individual config values in-game (like SCI!)
• GUI override at the tab-level when config override is really value-level
• Default values
• Engine-specific behavior in response to these values and possibly their absence

If this ticket also requires every scenario in that matrix to make sense, it will never be closed =) Feel free to use that list to identity other inconsistencies, but I think we've gotten all we're going to get out of this ticket.

I'll be doing a lot of SCI testing in dual-mode games before the release to make sure 52b7b3194cf496fa8a71c9c4208ec78b92e8aec8 doesn't introduce anything unexpected there.

[eriktorbjorn]

I thought (I'm not at the computer so I can't check) that the problem in my scenario was that even though the audio settings were overridden it wouldn't actually write game-specific settings to scummvm.ini for all of them.

At least that would explain to me why the game setting would appear to change with the global setting. And that's what I found confusing.

[eriktorbjorn]

This is what it looks like in the GUI after I've added the game the way I described above:

So at least to me, it looks like I've overridden all audio settings so that the game will have speech, but no subtitles.

But that's not what scummvm.ini says:

[kq7]
description=King's Quest VII: The Princeless Bride (DOS/English)
gmm_save_enabled=false
path=/usr/local/games/scummvm/kq7
enable_video_upscale=true
engineid=sci
gameid=kq7
language=en
music_driver=fluidsynth
platform=pc
enable_hq_video=true
opl_driver=db
speech_mute=false
guioptions=sndLinkSpeechToSfx sndLinkMusicToSfx noAspect gameOptionA gameOptionD gameOptionI lang_English

It has opl_driver=db and speech_mute=false but no subtitles=false. So when the global subtitles setting changes, so does the game setting.

[eriktorbjorn]

Even sneakier:

Start with the global setting as "Subtitles", i.e. the [scummvm] section will have

subtitles=true
speech_mute=true

Add the game, overriding the Audio settings, but do not touch the Speech/Subtitles setting. The game will now have:

speech_mute=true

Because the speech_mute setting is always saved, even if subtitles is not.

Change the global setting to "Speech". The global options are now:

subtitles=false
speech_mute=false

And the game settings are still

speech_mute=true

When you start the game, technically it will have neither speech nor subtitles. Though fortunately the engine will treat this as voice only, it seems. When editing the game settings, ScummVM will complain but recover:

WARNING: Wrong configuration: Both subtitles and speech are off. Assuming subtitles only!

Not as bad as it could be, but it still feels wrong to me.

[eriktorbjorn]

I filed a new bug report about this, since I guess it's not specific to King's Quest VII: https://bugs.scummvm.org/ticket/15518