Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENGINES: Remove use of getEngineId() in MetaEngine subclasses #3646

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@ccawley2011
Copy link
Member

@ccawley2011 ccawley2011 commented Jan 5, 2022

getEngineId() is only implemented in MetaEngineDetection subclasses; in MetaEngine subclasses, it always returns nullptr. This PR replaces all uses of getEngineId() in MetaEngines with getName() and removes the default implementation from the base PluginObject class.

@bluegr
Copy link
Member

@bluegr bluegr commented Jan 7, 2022

Unfortunately, we can't use getName() instead of getEngineId() in these cases, as they don't return the same strings.
For example, in AGS, getEngineId() returns "ags" whereas getName() returns "Adventure Game Studio", and in Cine, getEngineId() returns "cine" whereas getName() returns "Cinematique evo 1".

If these MetaEngine functions worked so far, perhaps we should remove getEngineId() AND getName() completely from them, i.e. instead of:

return Common::String::format("%s.%d", target == nullptr ? getEngineId() : target, saveGameIdx);

we should do:

assert(target);
return Common::String::format("%s.%d", target, saveGameIdx);

@ccawley2011
Copy link
Member Author

@ccawley2011 ccawley2011 commented Jan 7, 2022

This PR is meant as part of a larger set of changes intended to make MetaEngine and MetaEngineDetection more consistent with each other - right now, MetaEngineDetection::getName() returns the user-readable name and MetaEngineDetection::getEngineID() returns the engine ID, while MetaEngine::getName() returns the engine ID and MetaEngine::getEngineID() returns a null pointer. All of the changes in this PR should exclusively affect MetaEngine subclasses, and I'm planning on a follow-up PR that changes MetaEngineDetection to better match how MetaEngine behaves.

@bluegr
Copy link
Member

@bluegr bluegr commented Jan 9, 2022

This PR is meant as part of a larger set of changes intended to make MetaEngine and MetaEngineDetection more consistent with each other - right now, MetaEngineDetection::getName() returns the user-readable name and MetaEngineDetection::getEngineID() returns the engine ID, while MetaEngine::getName() returns the engine ID and MetaEngine::getEngineID() returns a null pointer. All of the changes in this PR should exclusively affect MetaEngine subclasses, and I'm planning on a follow-up PR that changes MetaEngineDetection to better match how MetaEngine behaves.

In this case, it makes more sense to use the engine ID instead of the user-friendly name, i.e. make getEngineid() actually return the engine ID. getName() is a bit confusing when used like in this PR

@bluegr
Copy link
Member

@bluegr bluegr commented Jan 18, 2022

@ccawley2011 any news on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants