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

* merge SGB into a mode option of Gambatte

* merge the core choices for SGB

* GBL compatibility

* rename SGB mode to SGB2 for Gambatte and fix BSNES initialization

* SGB border in GBL

* adjust sgb core detection

* reformat controller button naming

* adjust for 2.9

* adjust for 2.9 again

* revert changes to BSNES CoreConstructor

The system id will still be SGB for BSNES cores by default

* one Player tab per SNES controller for SGB

* idiomatic rephrase for showAnyBorder

* no dummy for CreateSGBVideoBuffer

* fix SGB conversion in LsmvImport

* add legacy BSNES option for (S)GB

* minor cosmetic changes

* Fix `.vbm` importer when Gambatte isn't the preferred GB core

* Fix button mapping spaghetti

* Code style nitpicks

* minor cleanups in Gambatte.cs

* Remove special SGB handling and mark bsnes* as GB/GBC cores

* remove SGB from CorePickerUIData

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.

* Un-revert submodule pull

* Un-revert other submodule pulls

* fix the crash from Toggle Link Connection

* remove inaccurate description of how Auto works for console

* Remove unused import

* do not change overall GUI

* fix byte-order markers

* do not mutate sync setting

* text

---------

Co-authored-by: Morilli <35152647+Morilli@users.noreply.github.com>
Co-authored-by: YoshiRulz <OSSYoshiRulz+git@gmail.com>

Author: 3 people

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

@@ -158,8 +158,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;
@@ -277,13 +283,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.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("Picks which console to emulate. 'SGB2' is always in effect when 'GB in SGB' is checked.")]
 			[DefaultValue(ConsoleModeType.Auto)]
 			public ConsoleModeType ConsoleMode { get; set; }
 

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

@@ -47,7 +47,7 @@ public Gameboy(CoreComm comm, IGameInfo game, byte[] file, GambatteSettings sett
 
 				var flags = LibGambatte.LoadFlags.READONLY_SAV;
 
-				switch (_syncSettings.ConsoleMode)
+				switch (game.System is VSystemID.Raw.SGB ? GambatteSyncSettings.ConsoleModeType.SGB2 : _syncSettings.ConsoleMode)
 				{
 					case GambatteSyncSettings.ConsoleModeType.GB:
 						break;
@@ -57,6 +57,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;
@@ -65,13 +69,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()

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

@@ -1,3 +1,4 @@
+using BizHawk.Common.StringExtensions;
 using BizHawk.Emulation.Common;
 
 namespace BizHawk.Emulation.Cores.Nintendo.Gameboy
@@ -17,16 +18,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;
+				if (s[0] is not 'P') continue;
+				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");
@@ -66,10 +77,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])
 					{
@@ -102,6 +114,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,18 +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;
 	}
 }

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

@@ -48,7 +48,8 @@ public GambatteLink(CoreLoadParameters<GambatteLinkSettings, GambatteLinkSyncSet
 				_linkedCores[i] = new Gameboy(lp.Comm, lp.Roms[i].Game, lp.Roms[i].RomData, _settings._linkedSettings[i], _syncSettings._linkedSyncSettings[i], lp.DeterministicEmulationRequested);
 				_linkedCores[i].ConnectInputCallbackSystem(_inputCallbacks);
 				_linkedCores[i].ConnectMemoryCallbackSystem(_memoryCallbacks, i);
-				_linkedConts[i] = new SaveController(Gameboy.CreateControllerDefinition(sgb: false, sub: false, tilt: false, rumble: false, remote: false));
+				IsAnySgb |= IsSgb(i);
+				_linkedConts[i] = new SaveController(Gameboy.CreateControllerDefinition(sgb: IsSgb(i), sub: false, tilt: false, rumble: false, remote: false));
 				_linkedBlips[i] = new BlipBuffer(1024);
 				_linkedBlips[i].SetRates(2097152 * 2, 44100);
 				_linkedOverflow[i] = 0;
@@ -66,6 +67,7 @@ public GambatteLink(CoreLoadParameters<GambatteLinkSettings, GambatteLinkSyncSet
 
 			FrameBuffer = CreateVideoBuffer();
 			VideoBuffer = CreateVideoBuffer();
+			SgbVideoBuffer = CreateSGBVideoBuffer();
 
 			GBLinkController = CreateControllerDefinition();
 
@@ -142,12 +144,23 @@ public IGameboyCommon First
 
 		private ControllerDefinition CreateControllerDefinition()
 		{
+			static void AddGBButtonsForPlayer(int p, ControllerDefinition ret)
+			{
+				var pfx = $"P{p} ";
+				ret.BoolButtons.AddRange(new[] { "Up", "Down", "Left", "Right", "A", "B", "Select", "Start" }
+					.Select(s => pfx + s));
+			}
 			ControllerDefinition ret = new($"GB Link {_numCores}x Controller");
-			for (int i = 0; i < _numCores; i++)
+			for (int i = 0, p = 1; i < _numCores; i++)
 			{
-				ret.BoolButtons.AddRange(
-					new[] { "Up", "Down", "Left", "Right", "A", "B", "Select", "Start", "Power" }
-						.Select(s => $"P{i + 1} {s}"));
+				AddGBButtonsForPlayer(p, ret);
+				ret.BoolButtons.Add($"P{p} Power");
+				p++;
+				if (IsSgb(i))
+				{
+					// add 3 more gamepads without a Power button
+					for (int e = p + 3; p < e; p++) AddGBButtonsForPlayer(p, ret);
+				}
 			}
 			ret.BoolButtons.Add("Toggle Link Connection");
 			if (_numCores > 2)
@@ -158,6 +171,20 @@ private ControllerDefinition CreateControllerDefinition()
 			return ret.MakeImmutable();
 		}
 
+		private bool IsSgb(int i)
+			=> _linkedCores[i].IsSgb;
+
+		public bool IsAnySgb { get; private set; }
+
+		private bool ShowBorder(int i)
+			=> IsSgb(i) && _settings._linkedSettings[i].ShowBorder;
+
+		public bool ShowAnyBorder()
+		{
+			for (var i = 0; i < _numCores; i++) if (ShowBorder(i)) return true;
+			return false;
+		}
+
 		private const int P1 = 0;
 		private const int P2 = 1;
 		private const int P3 = 2;

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

@@ -95,6 +95,8 @@ public enum CDLog_Flags : int
 
 		[DllImport("libgambatte", CallingConvention = CallingConvention.Cdecl)]
 		public static extern int gambatte_updatescreenborder(IntPtr core, int[] videobuf, int pitch);
+		[DllImport("libgambatte", CallingConvention = CallingConvention.Cdecl)]
+		public static extern unsafe int gambatte_updatescreenborder(IntPtr core, int* videobuf, int pitch);
 
 		[DllImport("libgambatte", CallingConvention = CallingConvention.Cdecl)]
 		public static extern int gambatte_generatesgbsamples(IntPtr core, short[] soundbuf, out uint samples);