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.

Sign up for GitHub

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

Jump to bottom

Simplify SGB related UIs and make it possible to use SGB under Gambatte link #3635

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

Simplify SGB related UIs and make it possible to use SGB under Gambatte link #3635

wants to merge 30 commits into from

Conversation

Fortranm
Copy link

@Fortranm Fortranm commented Apr 16, 2023 •
edited
Loading

This is a revision of #3283 made to work on BizHawk 2.9.

https://en.wikipedia.org/wiki/Super_Game_Boy#Super_Game_Boy_2

The Super Game Boy 2 was released exclusively in Japan in 1998. Additions included a link port to allow a user to access two-player mode via the link cable...

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:

  1. SGB is added as a Console Mode option for Gambatte. "GB in SGB" check box forces the SGB2 option if it's checked when Gambatte is run.
    image

  2. As a result of the first change (with addition adjustments), it is now possible to use SGB mode in Gambatte Link too.
    image
    image
    image
    image
    Each SGB core can have up to 4 controllers linked to it. The Power button for each SGB core is included on the first Player tab of the core for the time being.
    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.

Note: The SGB sound is not implemented yet but that can be done along with the MBC sound fix for Gambatte Link altogether later.

Partially addresses #2289

@YoshiRulz YoshiRulz mentioned this pull request Apr 26, 2023
Closed
@YoshiRulz YoshiRulz self-requested a review June 23, 2023 22:17
TiKevin83
TiKevin83 previously approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@TiKevin83 TiKevin83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving from a code perspective, haven't done a QA of the functionality yet

Morilli
Morilli reviewed Jul 13, 2023
View reviewed changes
Morilli
Morilli previously approved these changes May 29, 2024
View reviewed changes
Copy link
Collaborator

@Morilli Morilli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't tested the gambatte changes extensively but I think this is solid enough to get in. The "gb as sgb" UX flow was pretty bad imo so this is a definite improvement.

YoshiRulz
YoshiRulz previously requested changes May 30, 2024
View reviewed changes
Copy link
Member

@YoshiRulz YoshiRulz left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious of what @CasualPokePlayer has to say, specifically about the video buffer stuff. But this now LGTM in terms of code. (I think you deserve some kind of trophy for hacking together the button mapping by doing it in the wrong direction and with 3 nested loops.)
As for functionality, opening a 4x bundle and immediately closing it results in:

stacktrace 
System.NullReferenceException: Object reference not set to an instance of an object
  at BizHawk.Emulation.Common.LinkedSaveRam.CloneSaveRam () [0x0002f] in <f3a4da15026e4f05ab7072e9a135677c>:0
  at BizHawk.Client.EmuHawk.MainForm.FlushSaveRAM (System.Boolean autosave) [0x000f0] in <5e7f3991f9b246bb80e8d9725eda76e1>:0
  at (wrapper remoting-invoke-with-check) BizHawk.Client.EmuHawk.MainForm.FlushSaveRAM(bool)
  at BizHawk.Client.EmuHawk.MainForm.CloseGame (System.Boolean clearSram) [0x0006f] in <5e7f3991f9b246bb80e8d9725eda76e1>:0
  at BizHawk.Client.EmuHawk.MainForm.CloseRom (System.Boolean clearSram) [0x00010] in <5e7f3991f9b246bb80e8d9725eda76e1>:0
  at (wrapper remoting-invoke-with-check) BizHawk.Client.EmuHawk.MainForm.CloseRom(bool)
  at BizHawk.Client.EmuHawk.MainForm.CheckHotkey (System.String trigger) [0x01588] in <5e7f3991f9b246bb80e8d9725eda76e1>:0
  at BizHawk.Client.EmuHawk.MainForm.<ProcessInput>b__143_0 (System.Boolean current, System.String trigger) [0x00000] in <5e7f3991f9b246bb80e8d9725eda76e1>:0
  at System.Linq.Enumerable.Aggregate[TSource,TAccumulate] (System.Collections.Generic.IEnumerable`1[T] source, TAccumulate seed, System.Func`3[T1,T2,TResult] func) [0x0002e] in <89a8dc5bc33447a4a98f23a2d52e8aee>:0
  at BizHawk.Client.EmuHawk.MainForm.ProcessInput (BizHawk.Client.Common.InputCoalescer hotkeyCoalescer, BizHawk.Client.Common.ControllerInputCoalescer finalHostController, System.Func`2[T,TResult] searchHotkeyBindings, System.Func`2[T,TResult] activeControllerHasBinding) [0x00135] in <5e7f3991f9b246bb80e8d9725eda76e1>:0
  at BizHawk.Client.EmuHawk.MainForm.ProgramRunLoop () [0x0009c] in <5e7f3991f9b246bb80e8d9725eda76e1>:0
  at (wrapper remoting-invoke-with-check) BizHawk.Client.EmuHawk.MainForm.ProgramRunLoop()
  at BizHawk.Client.EmuHawk.Program.SubMain (System.String[] args) [0x0042b] in <5e7f3991f9b246bb80e8d9725eda76e1>:0

@Morilli

This comment was marked as resolved.

Sign in to view
@YoshiRulz

This comment was marked as resolved.

Sign in to view
@YoshiRulz
Copy link
Member

YoshiRulz commented Jun 1, 2024 •
edited
Loading

I still don't have a nice repro, but I found the culprit with print-debugging, and it wasn't related to this PR. Pushed 64db1fe.
In working on that, I've seen that SGB in GambatteLink is working, so I won't block this any further. @CasualPokePlayer?

Fortranm and others added 2 commits July 1, 2024 12:04
@Fortranm
remove SGB from CorePickerUIData
c56f047 
This fixes failures in CorePickerStabilityTests (https://ci.appveyor.com/project/zeromus/bizhawk-udexo/builds/50119675). It's not necessary to put SGB there as the system id is only ever switched to SGB after a relevant core is already initiated.
@YoshiRulz
Un-revert submodule pull
f148272
Morilli
Morilli reviewed Jul 1, 2024
View reviewed changes
@Morilli Morilli mentioned this pull request Jul 2, 2024
Closed
@YoshiRulz YoshiRulz dismissed their stale review July 4, 2024 12:13

Outdated

@YoshiRulz YoshiRulz self-requested a review February 5, 2025 10:32
YoshiRulz
YoshiRulz approved these changes Feb 8, 2025
View reviewed changes
Copy link
Member

@YoshiRulz YoshiRulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New approach LGTM, just need to clean up the diff.

@YoshiRulz YoshiRulz mentioned this pull request Feb 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Morilli Morilli Morilli left review comments

@YoshiRulz YoshiRulz YoshiRulz approved these changes

@TiKevin83 TiKevin83 TiKevin83 left review comments

@CasualPokePlayer CasualPokePlayer Awaiting requested review from CasualPokePlayer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Fortranm @Morilli @YoshiRulz @TiKevin83 @CasualPokePlayer