GUI: Game executed did not match window title or selection

[g5ppc]

Using 1.10.0git4661-gd7d75d97fd

I added EcoQuest I French and then immediately added EcoQuest II French. I then selected EcoQuest I and hit start.

Game window appeared that was titled EcoQuest: The Search for Cetus.. However, the game that executed was EcoQuest II.

I quit ScummVM, restarted, and everything was fine.

[g5ppc]

This is reproducible and the Status Window indicates it is launching ecoquest-fr, as would be expected.

[csnover]

Thanks for your report! I am not able to reproduce this issue, following these instructions, on macOS 10.11.6, using SDL2, with HQ2x + AR correction + filtering default graphics mode options and modern ScummVM theme:

1. Remove EcoQuest & EcoQuest 2 from the games list if they exist
2. Quit and restart ScummVM
3. Click Add Game...
4. Navigate to EcoQuest 1 directory and choose directory
5. Click OK on Game Add dialogue
6. Click Add Game...
7. Navigate to EcoQuest 2 directory and choose directory
8. Click OK on Game Add dialogue
9. Click on EcoQuest 1 in game list
10. Click Start

I am using English versions of these games since I do not have access to French versions.

Please, provide additional details about your configuration (at least matching the above list of information) and the list of steps to reliably reproduce the issue. Please also say whether either of these games are being picked up by the fallback detector (in this case ScummVM will print additional information in the console and ask you to submit the unknown game version). Thanks!

[g5ppc]

I am using the 8/27/2017 daily build on win7 sp1 x64. I have hq3x, modern theme, and AR set as general default, but not filtering. The games were added exactly as your steps describe, and status window shows nothing during that process. Nothing unusual about game detection, they are both floppy/DOS/French. Only exception is that Ecoquest I allows selection from French to English, while II is French only (green).

If it doesn't reproduce for you on the daily, then perhaps the bug is somehow particular to the French versions(or any foreign version?) or could be in the SCI unit and not the GUI.

I tried saving a game during the mismatch. It followed the mismatch, but subsequent renaming allows the savegame to work on future loads that aren't mismatched.

[csnover]

OK. I also tried adding a game I own with multiple language selection (Torin) for the first game added and still can’t get this to happen. macOS has a platform-specific directory chooser so I’m checking to see if another dev on Windows can reproduce.

[csnover]

bad script offset when adding eq1 en floppy, then eq2 en floppy, then starting eq1 en floppy

[csnover]

Another user on Windows was able to reproduce this bug privately to me. The French versions of EcoQuest did what is described above, following the repro steps with English versions did this instead:

After a restart of ScummVM the games would load fine. 54514 is an invalid offset for script 0 no matter what which EcoQuest game you are playing.

So, this is pretty scary, but at least it seems whatever is going on is reproducible across different Windows computers so hopefully a Windows dev with a debugger can look at this and figure out what is going on.

Statically, I was unable to see anything that looked like a logic error in the detector code, nor in the game execution code. SciEngine doesn’t seem to be doing anything terrible to cause this, and SearchMan looks like it should never end up in some partially populated state (it should actually not end up in any populated state at all until the first game starts). The LauncherDialog seems to have a bit more responsibility than perhaps it should (it sets the path for the game entry) but it doesn’t seem like it could be screwing things up since it is just sending that stuff to ConfMan, so it would seem difficult for anything there to be wrong since it seems like the wrongness would then be persisted to the next restart (though I suppose it is possible that it is some issue with in-memory representation that gets corrected when it re-reads everything from config after a restart, though I would expect this bug to be cross-platform then).

My best guess right now is that there is some caching problem in FSNode/FSDir, or, seemingly most likely since this is currently only reproduced on Windows, a similar sort of problem somewhere in the backends/fs/windows code.

[wjp]

I can reproduce this in Windows with the latest master build from buildbot, but not with the latest stable build from buildbot.

I have not been able to reproduce this with a self-compiled version of master in Windows with mingw64-i686.

[wjp]

Turns out this was not caused by the build or platform, but rather by the set of enabled engines.

A number of engines, including at least cge, cge2, director and sludge update SearchMan in their fallback detection.

We should either fix this, or have our common code clear SearchMan after calling each detector.

[csnover]

As much as I would like the engines to be good citizens and clean up after themselves (or just use a SearchSet instance instead of the singleton if they need it, especially since they probably don’t want to mix in stuff from the cwd during detection?), I’m not sure if there is any significant downside to clearing SearchMan after each detector runs. I guess the biggest problem would be that it doesn’t actually clear itself, but clears and then re-adds the cwd right away, which could be a performance problem. If so, and we can’t just stop adding the cwd and fix whatever that fixed another way, it could either get a dirty flag or something more aggressive to keep engines from trying to use it like raising an error when something tries to use it when an engine isn’t starting/running.

[wjp]

The way SearchMan is used by sludge and director looks like it should be removed, since the fallbackDetect function already has an FSNode available for the files they try to open.

CGE and CGE2 on the other hand use their own ResourceManager class, which goes through SearchMan, so that's less direct. SearchMan could of course be avoided there if we want.

We should also be able to add some kind of push/pop model to SearchMan where we can store its state and later restore it without re-scanning the disk.

[wjp]

This is addressed by ​https://github.com/scummvm/scummvm/pull/1024 , which needs review and testing.

[wjp]

This is now merged, so this should be fixed. Thanks for catching this!

[g5ppc]

Problem no longer reproduces on the 9/21 daily build. Thank you!