Opened 16 years ago
Closed 16 years ago
Last modified 4 years ago
#3192 closed defect (fixed)
DETECTOR: Launching undefined target adds launcher entry
|Reported by:||salty-horse||Owned by:||sev-|
Using latest svn.
When trying to launch the "gob1" target and it is not defined, an error appears and the entry "Gob engine game" is added to the launcher.
The output: $ ./scummvm gob1 Using configuration file: /home/me/.scummvmrc Looking for gob1 Trying to start game 'Gobliiins' WARNING: No path was provided. Assuming the data files are in the current directory! WARNING: Target upgraded from gob1 to gob! WARNING: Gob Engine failed to instantiate engine: Unable to locate game data (target 'gob1', path '.')!
Ticket imported from: #1719463. Ticket imported from: bugs/3192.
Change History (11)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
|Summary:||GOB: Launching undefined target adds launcher entry → DETECTOR: Launching undefined target adds launcher entry|
comment:3 by , 16 years ago
comment:4 by , 16 years ago
It looks like it might be connected to the upgrading of obsolete targets. The "gob1", "ite" and "cruise" targets are all in the list of obsolete targets for their respective engines.
For the Goblins engine, "gob1" and "gob2" are obsolete, but "gob3" isn't. I can't reproduce the problem with the "gob3" target.
comment:5 by , 16 years ago
|Priority:||normal → blocker|
comment:6 by , 16 years ago
Raising priority. This is a release-critical bug.
comment:7 by , 16 years ago
I agree with eriktorbjorn, this seems to be connected to the upgrading of obsolete targets. If we look at function upgradeTargetIfNecessary() in common/advandedDetector.cpp we cam see that if it upgrades a target's details, it always saves the changes to disk (ConfMan.flushToDisk();). In this case, a bogus game is added before it's even detected. Perhaps upgradeTargetIfNecessary shouldn't save the updaded information to disk? Does it need to do that, as at that point we're not sure if the game even exists on the given path?
comment:8 by , 16 years ago
A simple "solution" would be to remove the ConfMan.flushToDisk(); from upgradeTargetIfNecessary().
The drawback would be that in the legit case (where we upgrade a target coming from the config file) we'd not write the change back to disk. But given the choice between the two "bugs", I'd prefer the latter one, as it causes far fewer irritating problems (the bug described here cursed me quite some wasted time recently, when it added a fake simon1 target to my config file, and I didn't realize that).
we could still flush the config file at a later point in detectGameForEngineCreation(), though.
Alternatively, we could be more clever about upgrading the target, and only do it if the target is detect as being non-temporary, i.e. only if the target exists as such in the config file. If it doesn't, don't upgrade, just return with an "invalid/obsolete target" error.
comment:9 by , 16 years ago
comment:10 by , 16 years ago
|Status:||new → closed|
comment:11 by , 4 years ago
The same happens with the "ite" and "cruise" targets.