fix passing numbers for string args in .net lua functions (old engine had this behavior, granted "bad user" if they relied on this), add appropriate test

fix passing sbyte/char as args, add appropriate tests
cleanup the lua auto unlock hack, using a nice ref struct + dispose to handle it

Author: CasualPokePlayer

Assets/dll/NLua.dll

@@ -151,12 +151,23 @@ public void WithSurface(DisplaySurfaceID surfaceID, Action drawingCallsFunc)
 			}
 		}
 
-		public void ThisIsTheLuaAutounlockHack()
-		{
-			UnlockSurface(DisplaySurfaceID.EmuCore);
-			UnlockSurface(DisplaySurfaceID.Client);
+		public readonly ref struct LuaAutoUnlockHack
+		{
+			private readonly GuiApi _guiApi;
+
+			internal LuaAutoUnlockHack(GuiApi guiApi)
+				=> _guiApi = guiApi;
+
+			public void Dispose()
+			{
+				_guiApi.UnlockSurface(DisplaySurfaceID.EmuCore);
+				_guiApi.UnlockSurface(DisplaySurfaceID.Client);
+			}
 		}
 
+		public LuaAutoUnlockHack ThisIsTheLuaAutoUnlockHack()
+			=> new(this);
+
 		public void DrawNew(string name, bool clear)
 		{
 			switch (name)

References/NLua.dll

@@ -185,84 +185,92 @@ public void Restart(
 
 		public void CallSaveStateEvent(string name)
 		{
-			try
+			using (GuiAPI.ThisIsTheLuaAutoUnlockHack())
 			{
-				foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_SAVESTATE).ToList())
+				try
 				{
-					lf.Call(name);
+					foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_SAVESTATE).ToList())
+					{
+						lf.Call(name);
+					}
+				}
+				catch (Exception e)
+				{
+					LogToLuaConsole($"error running function attached by lua function event.onsavestate\nError message: {e.Message}");
 				}
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-			}
-			catch (Exception e)
-			{
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-				LogToLuaConsole($"error running function attached by lua function event.onsavestate\nError message: {e.Message}");
 			}
 		}
 
 		public void CallLoadStateEvent(string name)
 		{
-			try
+			using (GuiAPI.ThisIsTheLuaAutoUnlockHack())
 			{
-				foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_LOADSTATE).ToList())
+				try
 				{
-					lf.Call(name);
+					foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_LOADSTATE).ToList())
+					{
+						lf.Call(name);
+					}
+				}
+				catch (Exception e)
+				{
+					LogToLuaConsole($"error running function attached by lua function event.onloadstate\nError message: {e.Message}");
 				}
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-			}
-			catch (Exception e)
-			{
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-				LogToLuaConsole($"error running function attached by lua function event.onloadstate\nError message: {e.Message}");
 			}
 		}
 
 		public void CallFrameBeforeEvent()
 		{
 			if (IsUpdateSupressed) return;
-			try
+
+			using (GuiAPI.ThisIsTheLuaAutoUnlockHack())
 			{
-				foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_PREFRAME).ToList())
+				try
+				{
+					foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_PREFRAME).ToList())
+					{
+						lf.Call();
+					}
+				}
+				catch (Exception e)
 				{
-					lf.Call();
+					LogToLuaConsole($"error running function attached by lua function event.onframestart\nError message: {e.Message}");
 				}
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-			}
-			catch (Exception e)
-			{
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-				LogToLuaConsole($"error running function attached by lua function event.onframestart\nError message: {e.Message}");
 			}
 		}
 
 		public void CallFrameAfterEvent()
 		{
 			if (IsUpdateSupressed) return;
-			try
+
+			using (GuiAPI.ThisIsTheLuaAutoUnlockHack())
 			{
-				foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_POSTFRAME).ToList())
+				try
 				{
-					lf.Call();
+					foreach (var lf in RegisteredFunctions.Where(static l => l.Event == NamedLuaFunction.EVENT_TYPE_POSTFRAME).ToList())
+					{
+						lf.Call();
+					}
+				}
+				catch (Exception e)
+				{
+					LogToLuaConsole($"error running function attached by lua function event.onframeend\nError message: {e.Message}");
 				}
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-			}
-			catch (Exception e)
-			{
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-				LogToLuaConsole($"error running function attached by lua function event.onframeend\nError message: {e.Message}");
 			}
 		}
 
 		public void CallExitEvent(LuaFile lf)
 		{
-			foreach (var exitCallback in RegisteredFunctions
-				.Where(l => l.Event == NamedLuaFunction.EVENT_TYPE_ENGINESTOP
-					&& (l.LuaFile.Path == lf.Path || ReferenceEquals(l.LuaFile.Thread, lf.Thread)))
-				.ToList())
+			using (GuiAPI.ThisIsTheLuaAutoUnlockHack())
 			{
-				exitCallback.Call();
+				foreach (var exitCallback in RegisteredFunctions
+					.Where(l => l.Event == NamedLuaFunction.EVENT_TYPE_ENGINESTOP
+						&& (l.LuaFile.Path == lf.Path || ReferenceEquals(l.LuaFile.Thread, lf.Thread)))
+					.ToList())
+				{
+					exitCallback.Call();
+				}
 			}
-			GuiAPI.ThisIsTheLuaAutounlockHack();
 		}
 
 		public void Close()
@@ -319,31 +327,31 @@ public void ExecuteString(string command)
 
 		public (bool WaitForFrame, bool Terminated) ResumeScript(LuaFile lf)
 		{
-			_currThread = lf.Thread;
-
-			try
+			using (GuiAPI.ThisIsTheLuaAutoUnlockHack())
 			{
-				LuaLibraryBase.SetCurrentThread(lf);
+				_currThread = lf.Thread;
 
-				var execResult = _currThread.Resume();
-				GuiAPI.ThisIsTheLuaAutounlockHack();
+				try
+				{
+					LuaLibraryBase.SetCurrentThread(lf);
 
-				_currThread = null;
-				var result = execResult == KeraLua.LuaStatus.OK
-					? (WaitForFrame: false, Terminated: true) // terminated
-					: (WaitForFrame: FrameAdvanceRequested, Terminated: false); // yielded
+					var execResult = _currThread.Resume();
 
-				FrameAdvanceRequested = false;
-				return result;
-			}
-			catch (Exception)
-			{
-				GuiAPI.ThisIsTheLuaAutounlockHack();
-				throw;
-			}
-			finally
-			{
-				LuaLibraryBase.ClearCurrentThread();
+					_currThread = null;
+					var result = execResult switch
+					{
+						KeraLua.LuaStatus.OK => (WaitForFrame: false, Terminated: true),
+						KeraLua.LuaStatus.Yield => (WaitForFrame: FrameAdvanceRequested, Terminated: false),
+						_ => throw new InvalidOperationException($"{nameof(_currThread.Resume)}() returned {execResult}?")
+					};
+
+					FrameAdvanceRequested = false;
+					return result;
+				}
+				finally
+				{
+					LuaLibraryBase.ClearCurrentThread();
+				}
 			}
 		}
 

src/BizHawk.Client.Common/Api/Classes/GuiApi.cs

@@ -591,7 +591,9 @@ public void Net_Return_UIntPtr()
 		public void Net_Return_Char()
 		{
 			ReturnValue = 'a';
-			Assert.IsTrue((bool)LuaInstance.DoString($"return return_char() == {(byte)'a'}")[0]);
+			Assert.IsTrue((bool)LuaInstance.DoString($"return return_char() == {(ushort)'a'}")[0]);
+			ReturnValue = 'こ';
+			Assert.IsTrue((bool)LuaInstance.DoString($"return return_char() == {(ushort)'こ'}")[0]);
 		}
 
 		[TestMethod]
@@ -612,7 +614,7 @@ public void Net_Return_String_Utf8()
 		public void Net_Return_Color()
 		{
 			ReturnValue = ExpectedValue = Color.Aqua;
-			LuaInstance.DoString("return pass_color(return_color())");
+			LuaInstance.DoString("pass_color(return_color())");
 		}
 
 		[TestMethod]
@@ -679,16 +681,14 @@ public void Net_Argument_Bool()
 			LuaInstance.DoString("pass_bool(true)");
 		}
 
-		// this doesn't work for some reason
-		// just results in an exception due to "Invalid arguments to method call"
-		/*[TestMethod]
+		[TestMethod]
 		public void Net_Argument_S8()
 		{
 			ExpectedValue = (sbyte)123;
 			LuaInstance.DoString("pass_s8(123)");
 			ExpectedValue = (sbyte)-123;
 			LuaInstance.DoString("pass_s8(-123)");
-		}*/
+		}
 
 		[TestMethod]
 		public void Net_Argument_U8()
@@ -772,13 +772,11 @@ public void Net_Argument_Decimal()
 			LuaInstance.DoString("pass_decimal(-123.0)");
 		}
 
-		// these don't work either, although these make a bit more sense
+		// these don't work, although there is reasoning behind this
 		// IntPtr/UIntPtr are meant as handles to "userdata"
 		// so raw integers result in "Invalid arguments to method call"
-		// not sure why char doesn't work (same exception here)
 
-		/*
-		[TestMethod]
+		/*[TestMethod]
 		public void Net_Argument_IntPtr()
 		{
 			ExpectedValue = (IntPtr)123;
@@ -787,20 +785,21 @@ public void Net_Argument_IntPtr()
 			LuaInstance.DoString("pass_intptr(-123)");
 		}*/
 
-		/*
-		[TestMethod]
+		/*[TestMethod]
 		public void Net_Argument_UIntPtr()
 		{
 			ExpectedValue = (UIntPtr)123;
 			LuaInstance.DoString("pass_uintptr(123)");
 		}*/
 
-		/*[TestMethod]
+		[TestMethod]
 		public void Net_Argument_Char()
 		{
 			ExpectedValue = 'a';
-			LuaInstance.DoString($"pass_char({(byte)'a'})");
-		}*/
+			LuaInstance.DoString($"pass_char({(ushort)'a'})");
+			ExpectedValue = 'こ';
+			LuaInstance.DoString($"pass_char({(ushort)'こ'})");
+		}
 
 		[TestMethod]
 		public void Net_Argument_String()
@@ -816,6 +815,19 @@ public void Net_Argument_String_Utf8()
 			LuaInstance.DoString($"pass_string(\"こんにちは\")");
 		}
 
+		[TestMethod]
+		public void Net_Argument_String_Implicit_Number_Conversion()
+		{
+			ExpectedValue = "123";
+			LuaInstance.DoString("pass_string(123)");
+			ExpectedValue = "-123";
+			LuaInstance.DoString("pass_string(-123)");
+			ExpectedValue = "0.321";
+			LuaInstance.DoString("pass_string(0.321)");
+			ExpectedValue = "-0.321";
+			LuaInstance.DoString("pass_string(-0.321)");
+		}
+
 		[TestMethod]
 		public void Net_Argument_Color()
 		{