Add way to unregister auto-registered services with BasicServiceProvider

allows for better handling with SubNESHawk and Libretro statable, maybe others

src/BizHawk.Emulation.Common/Base Implementations/BasicServiceProvider.cs

@@ -14,7 +14,7 @@ namespace BizHawk.Emulation.Common
 	/// <seealso cref="IEmulatorServiceProvider"/>
 	public class BasicServiceProvider : IEmulatorServiceProvider
 	{
-		private readonly Dictionary<Type, object> _services = new Dictionary<Type, object>();
+		private readonly Dictionary<Type, object> _services = [ ];
 
 		public BasicServiceProvider(IEmulator core)
 		{
@@ -23,7 +23,7 @@ namespace BizHawk.Emulation.Common
 			// find the field), but we're going to keep such logic out of the basic provider.  anything the passed
 			// core doesn't implement directly needs to be added with Register()
 			// this also fully allows services that are not IEmulatorService
-			Type coreType = core.GetType();
+			var coreType = core.GetType();
 
 			foreach (var service in coreType.GetInterfaces().Where(static t => typeof(IEmulatorService).IsAssignableFrom(t)
 				&& t != typeof(IEmulatorService) && t != typeof(ISpecializedEmulatorService)))
@@ -45,30 +45,32 @@ namespace BizHawk.Emulation.Common
 		/// <exception cref="ArgumentNullException"><paramref name="provider"/> is null</exception>
 		public void Register<T>(T provider)
 			where T : class, IEmulatorService
-		{
+			=> _services[typeof(T)] = provider;
-			_services[typeof(T)] = provider;
+
-		}
+		/// <summary>
+		/// the core can call this to unregister an existing service
+		/// this is particularly useful wrt the auto-registration of services
+		/// in case the core has some condition which renders a service unusable
+		/// </summary>
+		/// <typeparam name="T">The <see cref="IEmulatorService"/> to unregister</typeparam>
+		public void Unregister<T>()
+			where T : class, IEmulatorService
+			=> _services.Remove(typeof(T));
 
 		public T GetService<T>()
 			where T : IEmulatorService
 			=> (T) GetService(typeof(T))!;
 
 		public object? GetService(Type t)
-		{
+			=> _services.TryGetValue(t, out var service) ? service : null;
-			return _services.TryGetValue(t, out var service) ? service : null;
-		}
 
 		public bool HasService<T>()
 			where T : IEmulatorService
-		{
+			=> HasService(typeof(T));
-			return HasService(typeof(T));
-		}
 
 		public bool HasService(Type t)
-		{
+			=> _services.ContainsKey(t);
-			return _services.ContainsKey(t);
-		}
 
-		public IEnumerable<Type> AvailableServices =>_services.Select(d => d.Key);
+		public IEnumerable<Type> AvailableServices => _services.Select(d => d.Key);
 	}
 }

src/BizHawk.Emulation.Cores/Consoles/Nintendo/NES/NES.Core.cs

@@ -10,7 +10,7 @@ using BizHawk.Emulation.Cores.Components.M6502;
 
 namespace BizHawk.Emulation.Cores.Nintendo.NES
 {
-	public partial class NES : IEmulator, ISoundProvider/*, ICycleTiming*/
+	public partial class NES : IEmulator, ISoundProvider, ICycleTiming
 	{
 		internal static class RomChecksums
 		{

src/BizHawk.Emulation.Cores/Consoles/Nintendo/NES/NES.cs

@@ -71,6 +71,12 @@ namespace BizHawk.Emulation.Cores.Nintendo.NES
 				}
 			}
 
+			// only the subframe core should have ICycleTiming registered
+			if (!subframe)
+			{
+				ser.Unregister<ICycleTiming>();
+			}
+
 			ResetControllerDefinition(subframe);
 		}
 

src/BizHawk.Emulation.Cores/Consoles/Nintendo/SubNESHawk/SubNESHawk.cs

@@ -4,8 +4,7 @@ using BizHawk.Emulation.Cores.Nintendo.NES;
 namespace BizHawk.Emulation.Cores.Nintendo.SubNESHawk
 {
 	[Core(CoreNames.SubNesHawk, "")]
-	public partial class SubNESHawk : IEmulator, IStatable, IInputPollable,
+	public partial class SubNESHawk : IEmulator, IStatable, IInputPollable, ISettable<NES.NES.NESSettings, NES.NES.NESSyncSettings>
-		ISettable<NES.NES.NESSettings, NES.NES.NESSyncSettings>, ICycleTiming
 	{
 		[CoreConstructor(VSystemID.Raw.NES, Priority = CorePriority.SuperLow)]
 		public SubNESHawk(CoreComm comm, GameInfo game, byte[] rom, /*string gameDbFn,*/ NES.NES.NESSettings settings, NES.NES.NESSyncSettings syncSettings)
@@ -36,6 +35,7 @@ namespace BizHawk.Emulation.Cores.Nintendo.SubNESHawk
 			ser.Register(_nesCore.ServiceProvider.GetService<IRegionable>());
 			ser.Register(_nesCore.ServiceProvider.GetService<ICodeDataLogger>());
 			ser.Register(_nesCore.ServiceProvider.GetService<IDriveLight>());
+			ser.Register(_nesCore.ServiceProvider.GetService<ICycleTiming>());
 
 			const string TRACE_HEADER = "6502: PC, machine code, mnemonic, operands, registers (A, X, Y, P, SP), flags (NVTBDIZCR), CPU Cycle, PPU Cycle";
 			_tracer = new TraceBuffer(TRACE_HEADER);
@@ -76,8 +76,5 @@ namespace BizHawk.Emulation.Cores.Nintendo.SubNESHawk
 		public NES.NES.NESSyncSettings GetSyncSettings() => _nesCore.GetSyncSettings();
 		public PutSettingsDirtyBits PutSettings(NES.NES.NESSettings o) => _nesCore.PutSettings(o);
 		public PutSettingsDirtyBits PutSyncSettings(NES.NES.NESSyncSettings o) => _nesCore.PutSyncSettings(o);
-
-		public long CycleCount => _nesCore.CycleCount;
-		public double ClockRate => _nesCore.ClockRate;
 	}
 }

src/BizHawk.Emulation.Cores/Libretro/Libretro.IStatable.cs

@@ -5,40 +5,29 @@ using BizHawk.Emulation.Common;
 
 namespace BizHawk.Emulation.Cores.Libretro
 {
-	// not all Libretro cores implement savestates
+	// note: Not all Libretro cores implement savestates
-	// we use this so we can optionally register IStatable
+	public partial class LibretroHost : IStatable
-	// todo: this can probably be genericized
-	public class StatableLibretro : IStatable
 	{
-		private readonly LibretroHost _host;
+		private byte[] _stateBuf = [ ];
-		private readonly LibretroApi _api;
-		private readonly byte[] _stateBuf;
-
-		public StatableLibretro(LibretroHost host, LibretroApi api, int maxSize)
-		{
-			_host = host;
-			_api = api;
-			_stateBuf = new byte[maxSize];
-		}
 
 		public bool AvoidRewind => false;
 
 		public void SaveStateBinary(BinaryWriter writer)
 		{
-			var len = checked((int)_api.retro_serialize_size());
+			var len = checked((int)api.retro_serialize_size());
 			if (len > _stateBuf.Length)
 			{
 				throw new Exception("Core attempted to grow state size. This is not allowed per the libretro API.");
 			}
 
-			_api.retro_serialize(_stateBuf, len);
+			api.retro_serialize(_stateBuf, len);
 			writer.Write(len);
 			writer.Write(_stateBuf, 0, len);
 
 			// host variables
-			writer.Write(_host.Frame);
+			writer.Write(Frame);
-			writer.Write(_host.LagCount);
+			writer.Write(LagCount);
-			writer.Write(_host.IsLagFrame);
+			writer.Write(IsLagFrame);
 		}
 
 		public void LoadStateBinary(BinaryReader reader)
@@ -49,13 +38,13 @@ namespace BizHawk.Emulation.Cores.Libretro
 				throw new Exception("State buffer size exceeded the core's maximum state size!");
 			}
 
-			reader.Read(_stateBuf, 0, len);
+			_ = reader.Read(_stateBuf, 0, len);
-			_api.retro_unserialize(_stateBuf, len);
+			api.retro_unserialize(_stateBuf, len);
 
 			// host variables
-			_host.Frame = reader.ReadInt32();
+			Frame = reader.ReadInt32();
-			_host.LagCount = reader.ReadInt32();
+			LagCount = reader.ReadInt32();
-			_host.IsLagFrame = reader.ReadBoolean();
+			IsLagFrame = reader.ReadBoolean();
 		}
 	}
 }

src/BizHawk.Emulation.Cores/Libretro/Libretro.cs

@@ -12,7 +12,7 @@ namespace BizHawk.Emulation.Cores.Libretro
 	// nb: multiple libretro cores could theoretically be ran at once
 	// but all of them would need to be different cores, a core itself is single instance
 	[PortedCore(CoreNames.Libretro, "CasualPokePlayer", singleInstance: true, isReleased: false)]
-	[ServiceNotApplicable(new[] { typeof(IDriveLight) })]
+	[ServiceNotApplicable([ typeof(IDriveLight) ])]
 	public partial class LibretroHost
 	{
 		private static readonly LibretroBridge bridge;
@@ -28,32 +28,24 @@ namespace BizHawk.Emulation.Cores.Libretro
 		}
 
 		private readonly LibretroApi api;
-		private IStatable _stateWrapper;
-
 		private readonly IntPtr cbHandler;
-		private readonly BridgeGuard _guard;
 
-		private class BridgeGuard : IMonitor
+		private class BridgeGuard(IntPtr parentHandler) : IMonitor
 		{
 			private static readonly object _sync = new();
 			private static IntPtr _activeHandler;
 			private static int _refCount;
 
-			private readonly IntPtr _parentHandler;
-
-			public BridgeGuard(IntPtr parentHandler)
-				=> _parentHandler = parentHandler;
-
 			public void Enter()
 			{
 				lock (_sync)
 				{
 					if (_activeHandler == IntPtr.Zero)
 					{
-						_activeHandler = _parentHandler;
+						_activeHandler = parentHandler;
-						bridge.LibretroBridge_SetGlobalCallbackHandler(_parentHandler);
+						bridge.LibretroBridge_SetGlobalCallbackHandler(parentHandler);
 					}
-					else if (_activeHandler != _parentHandler)
+					else if (_activeHandler != parentHandler)
 					{
 						throw new InvalidOperationException("Multiple callback handlers cannot be active at once!");
 					}
@@ -94,10 +86,9 @@ namespace BizHawk.Emulation.Cores.Libretro
 					throw new Exception("Failed to create callback handler!");
 				}
 
-				_guard = new(cbHandler);
+				var guard = new BridgeGuard(cbHandler);
-
 				api = BizInvoker.GetInvoker<LibretroApi>(
-					new DynamicLibraryImportResolver(corePath, hasLimitedLifetime: false), _guard, CallingConventionAdapters.Native);
+					new DynamicLibraryImportResolver(corePath, hasLimitedLifetime: false), guard, CallingConventionAdapters.Native);
 
 				_serviceProvider = new(this);
 
@@ -140,23 +131,17 @@ namespace BizHawk.Emulation.Cores.Libretro
 			}
 		}
 
-		private class RetroData : IDisposable
+		private class RetroData(object o, long len = 0) : IDisposable
 		{
-			private readonly GCHandle _handle;
+			private GCHandle _handle = GCHandle.Alloc(o, GCHandleType.Pinned);
 
 			public IntPtr PinnedData => _handle.AddrOfPinnedObject();
-			public long Length { get; }
+			public long Length { get; } = len;
-
-			public RetroData(object o, long len = 0)
-			{
-				_handle = GCHandle.Alloc(o, GCHandleType.Pinned);
-				Length = len;
-			}
 
 			public void Dispose() => _handle.Free();
 		}
 
-		private byte[] RetroString(string managedString)
+		private static byte[] RetroString(string managedString)
 		{
 			var ret = Encoding.UTF8.GetBytes(managedString);
 			Array.Resize(ref ret, ret.Length + 1);
@@ -205,11 +190,11 @@ namespace BizHawk.Emulation.Cores.Libretro
 					success = api.retro_load_no_game();
 					break;
 				case RETRO_LOAD.PATH:
-					game = new() { path = path.PinnedData };
+					game = new() { path = path!.PinnedData };
 					success = api.retro_load_game(ref game);
 					break;
 				case RETRO_LOAD.DATA:
-					game = new() { path = path.PinnedData, data = data.PinnedData, size = data.Length };
+					game = new() { path = path!.PinnedData, data = data!.PinnedData, size = data.Length };
 					success = api.retro_load_game(ref game);
 					break;
 				default:
@@ -237,8 +222,11 @@ namespace BizHawk.Emulation.Cores.Libretro
 			var len = checked((int)api.retro_serialize_size());
 			if (len > 0)
 			{
-				_stateWrapper = new StatableLibretro(this, api, len);
+				_stateBuf = new byte[len];
-				_serviceProvider.Register(_stateWrapper);
+			}
+			else
+			{
+				_serviceProvider.Unregister<IStatable>();
 			}
 
 			_region = api.retro_get_region();