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

src/BizHawk.Client.Common/RomLoader.cs

@@ -482,11 +482,6 @@ private void LoadOther(
 						);
 						return;
 					}
-
-					if (_config.GbAsSgb)
-					{
-						game.System = VSystemID.Raw.SGB;
-					}
 					break;
 				case VSystemID.Raw.PSX when ext is ".bin":
 					const string FILE_EXT_CUE = ".cue";
@@ -852,11 +847,6 @@ private void DispatchErrorMessage(Exception ex, string system, string path)
 			{
 				DoLoadErrorCallback(ex.Message, system, path, Deterministic, LoadErrorType.MissingFirmware);
 			}
-			else if (ex is CGBNotSupportedException)
-			{
-				// failed to load SGB bios or game does not support SGB mode.
-				DoLoadErrorCallback("Failed to load a GB rom in SGB mode.  You might try disabling SGB Mode.", system);
-			}
 			else if (ex is NoAvailableCoreException)
 			{
 				// handle exceptions thrown by the new detected systems that BizHawk does not have cores for

src/BizHawk.Client.Common/config/Config.cs

@@ -26,11 +26,9 @@ public class Config
 		public static readonly IReadOnlyList<(string[] AppliesTo, string[] CoreNames)> CorePickerUIData = new List<(string[], string[])>
 		{
 			([ VSystemID.Raw.GB, VSystemID.Raw.GBC ],
-				[ CoreNames.Gambatte, CoreNames.Sameboy, CoreNames.GbHawk, CoreNames.SubGbHawk ]),
+				[ CoreNames.Gambatte, CoreNames.Sameboy, CoreNames.GbHawk, CoreNames.SubGbHawk, CoreNames.Bsnes, CoreNames.Bsnes115, CoreNames.SubBsnes115 ]),
 			([ VSystemID.Raw.GBL ],
 				[ CoreNames.GambatteLink, CoreNames.GBHawkLink, CoreNames.GBHawkLink3x, CoreNames.GBHawkLink4x ]),
-			([ VSystemID.Raw.SGB ],
-				[ CoreNames.Gambatte, CoreNames.Bsnes115, CoreNames.SubBsnes115, CoreNames.Bsnes ]),
 			([ VSystemID.Raw.GEN ],
 				[ CoreNames.Gpgx, CoreNames.PicoDrive ]),
 			([ VSystemID.Raw.N64 ],
@@ -388,8 +386,6 @@ public void SetWindowScaleFor(string sysID, int windowScale)
 		public Dictionary<string, Dictionary<string, AnalogBind>> AllTrollersAnalog { get; set; } = new Dictionary<string, Dictionary<string, AnalogBind>>();
 		public Dictionary<string, Dictionary<string, FeedbackBind>> AllTrollersFeedbacks { get; set; } = new Dictionary<string, Dictionary<string, FeedbackBind>>();
 
-		/// <remarks>as this setting spans multiple cores and doesn't actually affect the behavior of any core, it hasn't been absorbed into the new system</remarks>
-		public bool GbAsSgb { get; set; }
 		public string LibretroCore { get; set; }
 
 		public Dictionary<string, string> PreferredCores = GenDefaultCorePreferences();

src/BizHawk.Client.Common/movie/import/LsmvImport.cs

@@ -159,8 +159,7 @@ protected override void RunImport()
 							break;
 						case "sgb_ntsc":
 						case "sgb_pal":
-							platform = VSystemID.Raw.SNES;
-							Config.GbAsSgb = true;
+							platform = VSystemID.Raw.SGB;
 							break;
 					}
 

src/BizHawk.Client.Common/movie/import/VbmImport.cs

@@ -122,9 +122,15 @@ recording time in Unix epoch format
 				Result.Movie.HeaderEntries.Add("IsCGBMode", "1");
 			}
 
+			var finalCoreName = Config.PreferredCores[VSystemID.Raw.GB];
 			if (isSGB)
 			{
-				Result.Errors.Add("SGB imports are not currently supported");
+				platform = VSystemID.Raw.SGB;
+				if (finalCoreName is not CoreNames.Gambatte)
+				{
+					Result.Warnings.Add($"{finalCoreName} doesn't support SGB; will use {CoreNames.Gambatte}");
+					finalCoreName = CoreNames.Gambatte;
+				}
 			}
 
 			Result.Movie.HeaderEntries[HeaderKeys.Platform] = platform;
@@ -276,13 +282,15 @@ A stream of 2-byte bitvectors which indicate which buttons are pressed at each p
 			}
 			else
 			{
-				Result.Movie.HeaderEntries[HeaderKeys.Core] = Config.PreferredCores[VSystemID.Raw.GB];
-				switch (Config.PreferredCores[VSystemID.Raw.GB])
+				Result.Movie.HeaderEntries[HeaderKeys.Core] = finalCoreName;
+				switch (finalCoreName)
 				{
 					case CoreNames.Gambatte:
 						Result.Movie.SyncSettingsJson = ConfigService.SaveWithType(new Gameboy.GambatteSyncSettings
 						{
-							ConsoleMode = is_GBC ? Gameboy.GambatteSyncSettings.ConsoleModeType.GBC : Gameboy.GambatteSyncSettings.ConsoleModeType.GB,
+							ConsoleMode = isSGB
+								? Gameboy.GambatteSyncSettings.ConsoleModeType.SGB2
+								: is_GBC ? Gameboy.GambatteSyncSettings.ConsoleModeType.GBC : Gameboy.GambatteSyncSettings.ConsoleModeType.GB,
 						});
 						break;
 					case CoreNames.GbHawk:

src/BizHawk.Client.EmuHawk/MainForm.cs

@@ -121,17 +121,9 @@ void ClickHandler(object clickSender, EventArgs clickArgs)
 				CoresSubMenu.DropDownItems.Add(submenu);
 			}
 			CoresSubMenu.DropDownItems.Add(new ToolStripSeparator { AutoSize = true });
-			var GBInSGBMenuItem = new ToolStripMenuItem { Text = "GB in SGB" };
-			GBInSGBMenuItem.Click += (_, _) =>
-			{
-				Config.GbAsSgb ^= true;
-				if (Emulator.SystemId is VSystemID.Raw.GB or VSystemID.Raw.GBC or VSystemID.Raw.SGB) FlagNeedsReboot();
-			};
-			CoresSubMenu.DropDownItems.Add(GBInSGBMenuItem);
 			var setLibretroCoreToolStripMenuItem = new ToolStripMenuItem { Text = "Set Libretro Core..." };
 			setLibretroCoreToolStripMenuItem.Click += (_, _) => RunLibretroCoreChooser();
 			CoresSubMenu.DropDownItems.Add(setLibretroCoreToolStripMenuItem);
-			CoresSubMenu.DropDownOpened += (_, _) => GBInSGBMenuItem.Checked = Config.GbAsSgb;
 
 			ToolStripMenuItemEx recentCoreSettingsSubmenu = new() { Text = "Recent" };
 			recentCoreSettingsSubmenu.DropDownItems.AddRange(CreateCoreSettingsSubmenus().ToArray());

src/BizHawk.Emulation.Cores/Consoles/Nintendo/BSNES/BsnesCore.cs

@@ -18,8 +18,9 @@ namespace BizHawk.Emulation.Cores.Nintendo.BSNES
 	[ServiceNotApplicable(new[] { typeof(IDriveLight) })]
 	public partial class BsnesCore : IEmulator, IDebuggable, IVideoProvider, ISaveRam, IStatable, IInputPollable, IRegionable, ISettable<BsnesCore.SnesSettings, BsnesCore.SnesSyncSettings>, IBSNESForGfxDebugger, IBoardInfo
 	{
+		[CoreConstructor(VSystemID.Raw.GB)]
+		[CoreConstructor(VSystemID.Raw.GBC)]
 		[CoreConstructor(VSystemID.Raw.Satellaview)]
-		[CoreConstructor(VSystemID.Raw.SGB)]
 		[CoreConstructor(VSystemID.Raw.SNES)]
 		public BsnesCore(CoreLoadParameters<SnesSettings, SnesSyncSettings> loadParameters) : this(loadParameters, false) { }
 		public BsnesCore(CoreLoadParameters<SnesSettings, SnesSyncSettings> loadParameters, bool subframe = false)
@@ -30,8 +31,8 @@ public BsnesCore(CoreLoadParameters<SnesSettings, SnesSyncSettings> loadParamete
 			this._romPath = Path.ChangeExtension(loadParameters.Roms[0].RomPath, null);
 			CoreComm = loadParameters.Comm;
 			_syncSettings = loadParameters.SyncSettings ?? new SnesSyncSettings();
-			SystemId = loadParameters.Game.System;
-			_isSGB = SystemId == VSystemID.Raw.SGB;
+			_isSGB = loadParameters.Game.System is VSystemID.Raw.GB or VSystemID.Raw.GBC;
+			SystemId = _isSGB ? VSystemID.Raw.SGB : loadParameters.Game.System;
 			_currentMsuTrack = new ProxiedFile();
 
 			byte[] sgbRomData = null;

src/BizHawk.Emulation.Cores/Consoles/Nintendo/BSNES/SubBsnesCore.cs

@@ -8,8 +8,9 @@ namespace BizHawk.Emulation.Cores.Nintendo.BSNES
 	[ServiceNotApplicable(new[] { typeof(IDriveLight) })]
 	public class SubBsnesCore : IEmulator, ICycleTiming
 	{
+		[CoreConstructor(VSystemID.Raw.GB)]
+		[CoreConstructor(VSystemID.Raw.GBC)]
 		[CoreConstructor(VSystemID.Raw.Satellaview)]
-		[CoreConstructor(VSystemID.Raw.SGB)]
 		[CoreConstructor(VSystemID.Raw.SNES)]
 		public SubBsnesCore(CoreLoadParameters<BsnesCore.SnesSettings, BsnesCore.SnesSyncSettings> loadParameters)
 		{

src/BizHawk.Emulation.Cores/Consoles/Nintendo/Gameboy/Gambatte.ISettable.cs

@@ -110,11 +110,12 @@ public enum ConsoleModeType
 				Auto,
 				GB,
 				GBC,
-				GBA
+				GBA,
+				SGB2,
 			}
 
 			[DisplayName("Console Mode")]
-			[Description("Pick which console to run, 'Auto' chooses from ROM header; 'GB', 'GBC', and 'GBA' chooses the respective system. Does nothing in SGB mode.")]
+			[Description("Which console to emulate ('Auto' picks based on the rom header)")]
 			[DefaultValue(ConsoleModeType.Auto)]
 			public ConsoleModeType ConsoleMode { get; set; }
 

src/BizHawk.Emulation.Cores/Consoles/Nintendo/Gameboy/Gambatte.cs

@@ -22,8 +22,7 @@ public partial class Gameboy : IInputPollable, IRomInfo, IGameboyCommon, ICycleT
 
 		[CoreConstructor(VSystemID.Raw.GB)]
 		[CoreConstructor(VSystemID.Raw.GBC)]
-		[CoreConstructor(VSystemID.Raw.SGB)]
-		public Gameboy(CoreComm comm, IGameInfo game, byte[] file, GambatteSettings settings, GambatteSyncSettings syncSettings, bool deterministic)
+		public Gameboy(CoreComm comm, GameInfo game, byte[] file, GambatteSettings settings, GambatteSyncSettings syncSettings, bool deterministic)
 		{
 			_serviceProvider = new(this);
 			_serviceProvider.Register<IDisassemblable>(_disassembler);
@@ -59,6 +58,10 @@ public Gameboy(CoreComm comm, IGameInfo game, byte[] file, GambatteSettings sett
 					case GambatteSyncSettings.ConsoleModeType.GBA:
 						flags |= LibGambatte.LoadFlags.CGB_MODE | LibGambatte.LoadFlags.GBA_FLAG;
 						break;
+					case GambatteSyncSettings.ConsoleModeType.SGB2:
+						flags |= LibGambatte.LoadFlags.SGB_MODE;
+						IsSgb = true;
+						break;
 					case GambatteSyncSettings.ConsoleModeType.Auto:
 						if (game.System == VSystemID.Raw.GBC)
 							flags |= LibGambatte.LoadFlags.CGB_MODE;
@@ -67,13 +70,6 @@ public Gameboy(CoreComm comm, IGameInfo game, byte[] file, GambatteSettings sett
 						throw new InvalidOperationException();
 				}
 
-				if (game.System == VSystemID.Raw.SGB)
-				{
-					flags &= ~(LibGambatte.LoadFlags.CGB_MODE | LibGambatte.LoadFlags.GBA_FLAG);
-					flags |= LibGambatte.LoadFlags.SGB_MODE;
-					IsSgb = true;
-				}
-
 				IsCgb = (flags & LibGambatte.LoadFlags.CGB_MODE) == LibGambatte.LoadFlags.CGB_MODE;
 
 				bool ForceBios()
@@ -637,7 +633,7 @@ public IGPUMemoryAreas LockGPU()
 				Vram = _vram,
 				Oam = _oam,
 				Sppal = _sppal,
-				Bgpal = _bgpal,	
+				Bgpal = _bgpal,
 			};
 		}
 

src/BizHawk.Emulation.Cores/Consoles/Nintendo/Gameboy/GambatteLink.IEmulator.cs

@@ -1,5 +1,7 @@
 using System;
+using System.Diagnostics;
 
+using BizHawk.Common.StringExtensions;
 using BizHawk.Emulation.Common;
 
 namespace BizHawk.Emulation.Cores.Nintendo.Gameboy
@@ -19,16 +21,26 @@ public bool FrameAdvance(IController controller, bool render, bool rendersound =
 
 			foreach (var s in GBLinkController.BoolButtons)
 			{
-				if (controller.IsPressed(s))
+				if (!controller.IsPressed(s)) continue;
+				Debug.Assert(s[0] is 'P');
+				var iSpace = s.IndexOf(' ');
+				var playerNum = int.Parse(s.Substring(startIndex: 1, length: iSpace - 1));
+				var consoleNum = 0;
+				var isSGB = false;
+				while (consoleNum < _numCores)
 				{
-					for (int i = 0; i < _numCores; i++)
-					{
-						if (s.Contains($"P{i + 1} "))
-						{
-							_linkedConts[i].Set(s.Replace($"P{i + 1} ", ""));
-						}
-					}
+					isSGB = IsSgb(consoleNum);
+					var playersForConsole = isSGB ? 4 : 1;
+					if (playerNum <= playersForConsole) break;
+					playerNum -= playersForConsole;
+					consoleNum++;
 				}
+				//TODO rather than this string manipulation, could construct a lookup ahead of time
+				_linkedConts[consoleNum].Set(s.EndsWithOrdinal("Power")
+					? "Power"
+					: isSGB
+						? $"P{playerNum} {s.Substring(startIndex: iSpace + 1)}"
+						: s.Substring(startIndex: iSpace + 1));
 			}
 
 			bool linkDiscoSignalNew = controller.IsPressed("Toggle Link Connection");
@@ -68,10 +80,11 @@ public bool FrameAdvance(IController controller, bool render, bool rendersound =
 
 			unsafe
 			{
-				fixed (int* fbuff = &FrameBuffer[0])
+				fixed (int* fbuff = &FrameBuffer[0], svbuff = SgbVideoBuffer)
 				{
 					// use pitch to have both cores write to the same frame buffer, interleaved
 					int Pitch = 160 * _numCores;
+					int sgbPitch = 256 * _numCores;
 
 					fixed (short* sbuff = &SoundBuffer[0])
 					{
@@ -104,6 +117,27 @@ public bool FrameAdvance(IController controller, bool render, bool rendersound =
 										{
 											Array.Copy(FrameBuffer, (i * 160) + (j * Pitch), VideoBuffer, (i * 160) + (j * Pitch), 160);
 										}
+										if (IsAnySgb)
+										{
+											// all SGB borders will be displayed when any of them has the option enabled
+											if (IsSgb(i))
+											{
+												if (LibGambatte.gambatte_updatescreenborder(
+													_linkedCores[i].GambatteState,
+													svbuff + (i * 256),
+													sgbPitch) is not 0)
+												{
+													throw new InvalidOperationException($"{nameof(LibGambatte.gambatte_updatescreenborder)}() returned non-zero (border error???)");
+												}
+											}
+											else
+											{
+												for (int j = 0; j < 144; j++)
+												{
+													Array.Copy(FrameBuffer, (i * 160) + (j * Pitch), SgbVideoBuffer, (i * 256 + 48) + (40 + j) * sgbPitch, 160);
+												}
+											}
+										}
 									}
 
 									n[i] += (int)nsamp;

src/BizHawk.Emulation.Cores/Consoles/Nintendo/Gameboy/GambatteLink.IVideoProvider.cs

@@ -4,10 +4,17 @@ namespace BizHawk.Emulation.Cores.Nintendo.Gameboy
 {
 	public partial class GambatteLink : IVideoProvider
 	{
-		public int VirtualWidth => 160 * _numCores;
-		public int VirtualHeight => 144;
-		public int BufferWidth => 160 * _numCores;
-		public int BufferHeight => 144;
+		public int VirtualWidth
+			=> (ShowAnyBorder() ? 256 : 160) * _numCores;
+
+		public int VirtualHeight
+			=> ShowAnyBorder() ? 224 : 144;
+
+		public int BufferWidth
+			=> VirtualWidth;
+
+		public int BufferHeight
+			=> VirtualHeight;
 
 		public int VsyncNumerator => _linkedCores[P1].VsyncNumerator;
 
@@ -17,19 +24,24 @@ public partial class GambatteLink : IVideoProvider
 
 		private readonly int[] FrameBuffer;
 
-		public int[] GetVideoBuffer() => VideoBuffer;
+		public int[] GetVideoBuffer()
+			=> ShowAnyBorder() ? SgbVideoBuffer : VideoBuffer;
 
 		private readonly int[] VideoBuffer;
-
+
+		private readonly int[] SgbVideoBuffer;
+
 		private int[] CreateVideoBuffer()
 		{
-			var b = new int[BufferWidth * BufferHeight];
+			var b = new int[160 * _numCores * 144];
 			for (int i = 0; i < b.Length; i++)
 			{
 				b[i] = -1; // GB/C screen is disabled on bootup, so it always starts as white, not black
 			}
 			return b;
 		}
 
+		private int[] CreateSGBVideoBuffer()
+			=> IsAnySgb ? new int[256 * _numCores * 224] : null;
 	}
 }