Opened 17 years ago

Closed 17 years ago

Last modified 5 years ago

#3192 closed defect (fixed)

DETECTOR: Launching undefined target adds launcher entry

Reported by: salty-horse Owned by: sev-
Priority: blocker Component: GUI
Version: Keywords: detection
Cc: Game:

Description

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 sev-, 17 years ago

Owner: set to sev-

comment:2 by salty-horse, 17 years ago

Component: Engine: Gob
Game: Gobliiins
Keywords: detection added
Summary: GOB: Launching undefined target adds launcher entryDETECTOR: Launching undefined target adds launcher entry

comment:3 by salty-horse, 17 years ago

The same happens with the "ite" and "cruise" targets.

comment:4 by eriktorbjorn, 17 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 sev-, 17 years ago

Priority: normalblocker

comment:6 by sev-, 17 years ago

Raising priority. This is a release-critical bug.

comment:7 by bluegr, 17 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 fingolfin, 17 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 sev-, 17 years ago

Fixed.

comment:10 by sev-, 17 years ago

Resolution: fixed
Status: newclosed

comment:11 by digitall, 5 years ago

Component: GUI
Note: See TracTickets for help on using tickets.