-
Notifications
You must be signed in to change notification settings - Fork 394
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
Simplify SGB related UIs (and make it possible to use SGB under Gambatte link) #3283
Conversation
There's some issues here:
|
@@ -325,7 +322,6 @@ public void ResolveDefaults() | |||
[VSystemID.Raw.GB] = CoreNames.Gambatte, | |||
[VSystemID.Raw.GBC] = CoreNames.Gambatte, | |||
[VSystemID.Raw.GBL] = CoreNames.GambatteLink, | |||
[VSystemID.Raw.SGB] = CoreNames.Gambatte, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect, all AppliesTo
sysIDs are used here (note GB and GBC). Actually I don't think the above change to CorePickerUIData
is correct either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been restored in a commit. As for the other changes in the file, I have repeatedly switched between cores and it seems to work fine aside from the minor issue mentioned in the comment.
{ | ||
_linkedConts[i].Set(s.Replace($"P{i + 1} - ", "")); | ||
} | ||
else if (s.Contains($"P{i + 1} ")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This forced you to add a hack to Bk2MnemonicLookup.cs
. What does it actually do? Can you implement it another way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This deals with the fact that an SGB core has 4 controllers, as shown in the last image in the original description. I don't have a better implementation in mind for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number them 1-4 and make the next console start from 5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That wouldn't be intuitive on which core a controller corresponds to, especially when console modes are mixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well there's no precedent here. The closest thing is multitaps e.g. for SNES or Genesis. Anyone else want to weigh in on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://en.wikipedia.org/wiki/Super_Game_Boy#Super_Game_Boy_2
Currently, the SGB mode of Gambatte has to be turned on via Config > Cores > GB in SGB instead of in the sync setting and cannot be used in GB Link as a result.
This PR does three things related to SGB:
SGB is added (or rather, merged in) as a Console Mode option for Gambatte.
"GB in SGB" check box and SGB core picker are removed and the BSNES cores are added under the GB core picket. GB/GBC games will load under SGB mode of the corresponding BSNES core if one of those options are chosen.
As a result of the first change (with addition adjustments), it is now possible to use SGB mode in Gambatte Link too.
Each SGB core can have up to 4 controllers linked to it. Note that the tab parsing process is currently hardcoded to categorize every item that starts with "Px " under the tab "Player x" and everything else under "Console". There have been suggestions to use "Console x" for a core in link mode but that requires enough refactoring to be considered a separate task IMO.