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

Preferred core picker is intransparent and inflexible #3960

Open
Morilli opened this issue Jul 2, 2024 · 9 comments · May be fixed by #4033
Open

Preferred core picker is intransparent and inflexible #3960

Morilli opened this issue Jul 2, 2024 · 9 comments · May be fixed by #4033
Labels
App: EmuHawk Relating to EmuHawk frontend Enhancement For feature requests or possible improvements Repro: Fixed/added in branch/fork

Comments

@Morilli
Copy link
Collaborator

Morilli commented Jul 2, 2024

The current core preferences picker (Config->Preferred Cores) is not fully transparent in how it works, and does not allow configuring everything. On top of that, it is also manually maintained and needs to be updated when cores get added or modified, which does / did not always happen.

Let's take this example:
image
What happens when Gambatte is selected here? The preferred core for GB and GBC will be set to Gambatte. The fact that both GB and GBC are affected is not made clear anywhere and can only be determined by testing it out.
One might argue that this could be "user-friendly" by not overloading them with redundant options, but I'd say having preferences made explicit is preferable over implicitly assuming GB implies GBC.
We can already see that it is impossible to set different core preferences for GB and GBC games, and the same is true for a number of other options.

One thing I was only recently made aware of in the context of #3635: It is currently impossible to choose the SubBSNESv115+ core for Satellaview games, even though it does have the corresponding CoreConstructor attribute, because it isn't exposed in the preferred core picker. Adding it to the SNES group is problematic because then the preference for SNES is applied to Satellaview, even though some SNES cores don't support Satellaview at all!

My suggestion is to make choices for all SystemIDs explicit and automatically generated. That would guarantee that

  • all SystemIDs can have a preferred core set via config
  • all cores can be selected without having to manually add them to the config
  • CoreConstructor attributes have user-facing meaning

I have #4033 that shows my suggested approach.

see also #2289

@YoshiRulz YoshiRulz added Enhancement For feature requests or possible improvements App: EmuHawk Relating to EmuHawk frontend labels Jul 2, 2024
@8bitZeta

This comment was marked as off-topic.

@CasualPokePlayer

This comment was marked as off-topic.

@YoshiRulz
Copy link
Member

YoshiRulz commented Jul 4, 2024

SubBSNESv115+ [...] Satellaview [...] Adding it to the SNES group is problematic because then the preference for SNES is applied to Satellaview, even though some SNES cores don't support Satellaview at all!

I think it would be fine. If you picked e.g. Snes9x your preference would just be ignored. It would be the same as if your preferred core had thrown an exception on init—which I believe is currently never done based on the sysID, but is done based on the mapper.

@Morilli
Copy link
Collaborator Author

Morilli commented Jul 4, 2024

If you picked e.g. Snes9x your preference would just be ignored.

Right, the way core choosing currently works just ignores cores that are specified that can't actually run the given system. But then what actually determines what core is preferred when a satellaview rom is loaded? Currently, it's the normal bsnes core, but why? There's no preference set and it's effectively pure chance that it's not the subframe core that gets picked.

I'm suggesting to make every choice explicit and automatically generated to prevent such headaches when considering core order and preference.

@YoshiRulz
Copy link
Member

I disagree. While it may make sense to list Satellaview separately from SNES, you can't extend that to every sysID. There would effectively be duplicate settings for GB/GBC, the TG-16 family, SG-1000 and SMS—users would have to keep these in sync manually.

@Morilli
Copy link
Collaborator Author

Morilli commented Jul 4, 2024

There would effectively be duplicate settings for GB/GBC, the TG-16 family, SG-1000 and SMS

Right, that's part of that change. I don't really know these systems well enough to judge whether a separation or merge makes more logical sense, but if the system id is different, I feel like allowing different preferences make sense.

If you don't want that duplication, we could also merge all identical lists (for example GB/GBC, as all emulators providing one also provide the other). This should then ideally be communicated through the UI.

@Morilli
Copy link
Collaborator Author

Morilli commented Jul 4, 2024

No matter whether generated or manually typed, what's your opinion on changing the UI to this (probably sorted, didn't do that yet):
image

@YoshiRulz
Copy link
Member

That's fine. Giving each set a manual label wouldn't be much of an improvement IMO. The order within each set is already good since the first was used as the label.

@Morilli
Copy link
Collaborator Author

Morilli commented Jul 8, 2024

Have updated the branch. I could incorporate even more of it into master without forcing the changed auto-generated logic.

One thing I noticed while doing the changes was that for NES, the quicknes core is the default preference, however in its CoreConstructor attribute it has its priority set to Low, which doesn't make sense. With my approach this is impossible.

Also, default core preferences are now applied even when the user has changed defaults, which is currently not the case. To be fair this is only relevant in very niche scenarios, but still a logical improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App: EmuHawk Relating to EmuHawk frontend Enhancement For feature requests or possible improvements Repro: Fixed/added in branch/fork
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants