Opened 12 years ago

Closed 12 years ago

Last modified 7 months ago

#3192 closed defect (fixed)

DETECTOR: Launching undefined target adds launcher entry

Reported by: salty-horse Owned by: sev-
Priority: blocker Component: GUI
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 Changed 12 years ago by sev-

Owner: set to sev-

comment:2 Changed 12 years ago by salty-horse

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

comment:3 Changed 12 years ago by salty-horse

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

comment:4 Changed 12 years ago by eriktorbjorn

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 Changed 12 years ago by sev-

Priority: normalblocker

comment:6 Changed 12 years ago by sev-

Raising priority. This is a release-critical bug.

comment:7 Changed 12 years ago by bluegr

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 Changed 12 years ago by fingolfin

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 Changed 12 years ago by sev-

Fixed.

comment:10 Changed 12 years ago by sev-

Resolution: fixed
Status: newclosed

comment:11 Changed 7 months ago by digitall

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