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

Auto-generate preferred cores from source #4033

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Morilli
Copy link
Collaborator

@Morilli Morilli commented Sep 16, 2024

RFC.

This auto-generates the UI options for the core preferences picker from the list of available cores. This also requires changing the core attribute for many cores to signal that they are the preferred core for a specific system.
Basically, instead of having the core preference list stored entangled with the UI picker data and having to be manually maintained, the list is now based purely on the available cores.

This and the added tests ensure that all system ids with multiple core choices have an entry in the core picker (preventing mistakes like what d74b130 fixed) and that the default preference is always known, even when the user has configured a different preferred core for a given system id.

This will also make it impossible for fa08002 to occur, where quick(er)nes was both the preferred core and had lower than default priority. In reality this doesn't really show itself, but it's best to fix it nonetheless.

@YoshiRulz
Copy link
Member

So this makes CoreConstructor.Priority the single-source-of-truth? I like a SSoT but I think we should remove the priority system completely.

@YoshiRulz YoshiRulz self-requested a review September 16, 2024 12:55
@Morilli
Copy link
Collaborator Author

Morilli commented Sep 16, 2024

So this makes CoreConstructor.Priority the single-source-of-truth?

Yes, effectively. I'm myself not 100% sold on it, but it felt better than the current system at least. If we can get something set up that enforces that all systems that have multiple cores emulating it also have a preference choice in the UI, that would probably be enough already.

I think we should remove the priority system completely.

Oh? So just have preferences set manually and don't allow cores to alter their own priority?

@YoshiRulz
Copy link
Member

YoshiRulz commented Sep 16, 2024

Yeah, either in Config or CoreInventory.

I'm concerned about the no reflection case (#3768) and the case of adding cores at runtime (obviously with reflection) but mainly the former.

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

Successfully merging this pull request may close these issues.

Preferred core picker is intransparent and inflexible
2 participants