From 6c1cb1e951e8f0bff734ffea82cd1f13e642480d Mon Sep 17 00:00:00 2001 From: SuuperW Date: Sun, 8 Oct 2023 20:58:39 -0500 Subject: [PATCH] Refactor to make loading external tools simpler by not requiring passing around a load callback method. Add TestExternalToolCanUseApi (passes) --- .../tools/ExternalToolManager.cs | 38 ++++--------------- .../tools/Interfaces/IToolManager.cs | 4 +- .../tools/ToolManagerBase.cs | 16 +++++--- src/BizHawk.Client.EmuHawk/MainForm.Events.cs | 4 +- src/BizHawk.Client.EmuHawk/MainForm.cs | 8 +--- src/BizHawk.Client.EmuHawk/tools/ToolBox.cs | 2 +- .../Client.Common/Api/ExternalToolTests.cs | 21 +++++++++- .../Implementations/TestToolManager.cs | 10 +++-- .../Mocks/MockMainFormForTools.cs | 3 +- 9 files changed, 53 insertions(+), 53 deletions(-) diff --git a/src/BizHawk.Client.Common/tools/ExternalToolManager.cs b/src/BizHawk.Client.Common/tools/ExternalToolManager.cs index 88cdbcba42..ff5523d8b6 100644 --- a/src/BizHawk.Client.Common/tools/ExternalToolManager.cs +++ b/src/BizHawk.Client.Common/tools/ExternalToolManager.cs @@ -14,13 +14,9 @@ namespace BizHawk.Client.Common { public struct MenuItemInfo { - private readonly string _asmChecksum = ""; + public readonly string AsmChecksum = ""; - private readonly string _entryPointTypeName = ""; - - private readonly ExternalToolManager _extToolMan; - - private bool _skipExtToolWarning = false; + public readonly string EntryPointTypeName = ""; public readonly string AsmFilename = ""; @@ -33,7 +29,6 @@ namespace BizHawk.Client.Common public readonly bool Enabled = false; public MenuItemInfo( - ExternalToolManager extToolMan, string asmChecksum, string asmFilename, string entryPointTypeName, @@ -42,10 +37,8 @@ namespace BizHawk.Client.Common string toolTip, bool enabled) { - _asmChecksum = asmChecksum; - _entryPointTypeName = entryPointTypeName; - _extToolMan = extToolMan; - _skipExtToolWarning = _extToolMan._config.TrustedExtTools.TryGetValue(asmFilename, out var s) && s == _asmChecksum; + AsmChecksum = asmChecksum; + EntryPointTypeName = entryPointTypeName; AsmFilename = asmFilename; Text = text; Icon = icon; @@ -53,31 +46,17 @@ namespace BizHawk.Client.Common Enabled = enabled; } - public MenuItemInfo(ExternalToolManager extToolMan, string text, string toolTip) + public MenuItemInfo(string text, string toolTip) { - _extToolMan = extToolMan; Text = text; ToolTip = toolTip; } - - public void TryLoad() - { - var success = _extToolMan._loadCallback( - /*toolPath:*/ AsmFilename, - /*customFormTypeName:*/ _entryPointTypeName, - /*skipExtToolWarning:*/ _skipExtToolWarning); - if (!success || _skipExtToolWarning) return; - _skipExtToolWarning = true; - _extToolMan._config.TrustedExtTools[AsmFilename] = _asmChecksum; - } } private Config _config; private readonly Func<(string SysID, string Hash)> _getLoadedRomInfoCallback; - private readonly Func _loadCallback; - private FileSystemWatcher DirectoryMonitor; private readonly List MenuItems = new List(); @@ -86,11 +65,9 @@ namespace BizHawk.Client.Common public ExternalToolManager( Config config, - Func<(string SysID, string Hash)> getLoadedRomInfoCallback, - Func loadCallback) + Func<(string SysID, string Hash)> getLoadedRomInfoCallback) { _getLoadedRomInfoCallback = getLoadedRomInfoCallback; - _loadCallback = loadCallback; Restart(config); } @@ -179,7 +156,6 @@ namespace BizHawk.Client.Common toolTip = toolAttribute.Description; return new MenuItemInfo( - this, asmChecksum: SHA1Checksum.ComputePrefixedHex(asmBytes), asmFilename: fileName, entryPointTypeName: entryPoint.FullName, @@ -205,7 +181,7 @@ namespace BizHawk.Client.Common ReflectionTypeLoadException => "Something went wrong while trying to load the assembly.", _ => $"An exception of type {e.GetType().FullName} was thrown while trying to load the assembly and look for an IExternalToolForm:\n{e.Message}" }; - return new MenuItemInfo(this, text, toolTip); + return new MenuItemInfo(text, toolTip); } } diff --git a/src/BizHawk.Client.Common/tools/Interfaces/IToolManager.cs b/src/BizHawk.Client.Common/tools/Interfaces/IToolManager.cs index 906235fdbe..bfeaa8f221 100644 --- a/src/BizHawk.Client.Common/tools/Interfaces/IToolManager.cs +++ b/src/BizHawk.Client.Common/tools/Interfaces/IToolManager.cs @@ -8,6 +8,8 @@ namespace BizHawk.Client.Common { public interface IToolManager { + IReadOnlyCollection ExternalToolInfos { get; } + /// /// Loads the tool dialog T (T must implements ) , if it does not exist it will be created, if it is already open, it will be focused /// This method should be used only if you can't use the generic one @@ -29,7 +31,7 @@ namespace BizHawk.Client.Common where T : class, IToolForm; /// Loads the external tool's entry form. - IExternalToolForm LoadExternalToolForm(string toolPath, string customFormTypeName, bool focus = true, bool skipExtToolWarning = false); + IExternalToolForm LoadExternalToolForm(ExternalToolManager.MenuItemInfo info, bool focus = true); void AutoLoad(); diff --git a/src/BizHawk.Client.Common/tools/ToolManagerBase.cs b/src/BizHawk.Client.Common/tools/ToolManagerBase.cs index 634f2a1d46..ecb512f044 100644 --- a/src/BizHawk.Client.Common/tools/ToolManagerBase.cs +++ b/src/BizHawk.Client.Common/tools/ToolManagerBase.cs @@ -27,6 +27,8 @@ namespace BizHawk.Client.Common protected readonly IMovieSession _movieSession; protected IGameInfo _game; + public IReadOnlyCollection ExternalToolInfos => _extToolManager.ToolStripItems; + // TODO: merge ToolHelper code where logical // For instance, add an IToolForm property called UsesCheats, so that a UpdateCheatRelatedTools() method can update all tools of this type // Also a UsesRam, and similar method @@ -140,9 +142,9 @@ namespace BizHawk.Client.Common } /// Loads the external tool's entry form. - public IExternalToolForm LoadExternalToolForm(string toolPath, string customFormTypeName, bool focus = true, bool skipExtToolWarning = false) + public IExternalToolForm LoadExternalToolForm(ExternalToolManager.MenuItemInfo info, bool focus = true) { - var existingTool = _tools.OfType().FirstOrDefault(t => t.GetType().Assembly.Location == toolPath); + var existingTool = _tools.OfType().FirstOrDefault(t => t.GetType().Assembly.Location == info.AsmFilename); if (existingTool != null) { if (existingTool.IsActive) @@ -158,7 +160,8 @@ namespace BizHawk.Client.Common _tools.Remove(existingTool); } - var newTool = (IExternalToolForm)CreateInstance(typeof(IExternalToolForm), toolPath, customFormTypeName, skipExtToolWarning: skipExtToolWarning); + bool skipWarning = _config.TrustedExtTools.TryGetValue(info.AsmFilename, out var s) && s == info.AsmChecksum; + var newTool = (IExternalToolForm)CreateInstance(typeof(IExternalToolForm), info.AsmFilename, info.EntryPointTypeName, skipExtToolWarning: skipWarning); if (newTool == null) return null; SetFormParent(newTool); if (!(ServiceInjector.UpdateServices(_emulator.ServiceProvider, newTool) && ApiInjector.UpdateApis(ApiProvider, newTool))) return null; @@ -166,14 +169,16 @@ namespace BizHawk.Client.Common // auto settings if (newTool is IToolFormAutoConfig autoConfigTool) { - AttachSettingHooks(autoConfigTool, _config.CommonToolSettings.GetValueOrPutNew(customFormTypeName)); + AttachSettingHooks(autoConfigTool, _config.CommonToolSettings.GetValueOrPutNew(info.EntryPointTypeName)); } // custom settings if (HasCustomConfig(newTool)) { - InstallCustomConfig(newTool, _config.CustomToolSettings.GetValueOrPutNew(customFormTypeName)); + InstallCustomConfig(newTool, _config.CustomToolSettings.GetValueOrPutNew(info.EntryPointTypeName)); } + _config.TrustedExtTools[info.AsmFilename] = info.AsmChecksum; + newTool.Restart(); newTool.Show(); return newTool; @@ -467,7 +472,6 @@ namespace BizHawk.Client.Common try { - //tool = Activator.CreateInstanceFrom(dllPath, toolTypeName ?? "BizHawk.Client.EmuHawk.CustomMainForm").Unwrap() as IExternalToolForm; tool = CreateInstanceFrom(dllPath, toolTypeName); if (tool == null) { diff --git a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs index 1ec004ce06..2c69a123ba 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.Events.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.Events.cs @@ -1182,9 +1182,9 @@ namespace BizHawk.Client.EmuHawk private void ExternalToolMenuItem_DropDownOpening(object sender, EventArgs e) { ExternalToolMenuItem.DropDownItems.Clear(); - ExternalToolMenuItem.DropDownItems.AddRange(ExtToolManager.ToolStripItems.Select(static (info) => + ExternalToolMenuItem.DropDownItems.AddRange(ExtToolManager.ToolStripItems.Select((info) => { - return new ToolStripMenuItem(info.Text, info.Icon, (_, _) => info.TryLoad()) + return new ToolStripMenuItem(info.Text, info.Icon, (_, _) => Tools.LoadExternalToolForm(info)) { ToolTipText = info.ToolTip, Enabled = info.Enabled, diff --git a/src/BizHawk.Client.EmuHawk/MainForm.cs b/src/BizHawk.Client.EmuHawk/MainForm.cs index b78d1499ff..aea3091e59 100644 --- a/src/BizHawk.Client.EmuHawk/MainForm.cs +++ b/src/BizHawk.Client.EmuHawk/MainForm.cs @@ -196,7 +196,7 @@ namespace BizHawk.Client.EmuHawk .FirstOrNull(info => info.AsmFilename == requestedExtToolDll || Path.GetFileName(info.AsmFilename) == requestedExtToolDll || Path.GetFileNameWithoutExtension(info.AsmFilename) == requestedExtToolDll); - if (found is not null) found.Value.TryLoad(); + if (found is not null) Tools.LoadExternalToolForm(found.Value); else Console.WriteLine($"requested ext. tool dll {requestedExtToolDll} could not be loaded"); } @@ -473,11 +473,7 @@ namespace BizHawk.Client.EmuHawk ExtToolManager = new( Config, - () => (Emulator.SystemId, Game.Hash), - (toolPath, customFormTypeName, skipExtToolWarning) => Tools!.LoadExternalToolForm( - toolPath: toolPath, - customFormTypeName: customFormTypeName, - skipExtToolWarning: skipExtToolWarning) is not null); + () => (Emulator.SystemId, Game.Hash)); Tools = new ToolManager(this, Config, DisplayManager, ExtToolManager, InputManager, Emulator, MovieSession, Game); // TODO GL - move these event handlers somewhere less obnoxious line in the On* overrides diff --git a/src/BizHawk.Client.EmuHawk/tools/ToolBox.cs b/src/BizHawk.Client.EmuHawk/tools/ToolBox.cs index 7c884b3d37..a039084575 100644 --- a/src/BizHawk.Client.EmuHawk/tools/ToolBox.cs +++ b/src/BizHawk.Client.EmuHawk/tools/ToolBox.cs @@ -77,7 +77,7 @@ namespace BizHawk.Client.EmuHawk Image = tsi.Icon ?? IconMissingIcon.Value, Text = tsi.Text, }; - tsb.Click += (_, _) => tsi.TryLoad(); + tsb.Click += (_, _) => Tools.LoadExternalToolForm(tsi); ToolBoxStrip.Items.Add(tsb); } } diff --git a/src/BizHawk.Tests/Client.Common/Api/ExternalToolTests.cs b/src/BizHawk.Tests/Client.Common/Api/ExternalToolTests.cs index a160549cf1..59a2a92b61 100644 --- a/src/BizHawk.Tests/Client.Common/Api/ExternalToolTests.cs +++ b/src/BizHawk.Tests/Client.Common/Api/ExternalToolTests.cs @@ -31,10 +31,16 @@ namespace BizHawk.Tests.Client.Common.Api )!.Path = "./extTools"; } + private TestExternalAPI LoadExternalTool(ToolManagerBase toolManager) + { + var info = toolManager.ExternalToolInfos.First(static (i) => i.Text == "TEST"); + return (toolManager.LoadExternalToolForm(info) as TestExternalAPI)!; + } + [TestMethod] public void TestExternalToolIsFound() { - ExternalToolManager manager = new ExternalToolManager(config, () => ("", ""), (p1, p2, p3) => true); + ExternalToolManager manager = new ExternalToolManager(config, () => ("", "")); Assert.IsTrue(manager.ToolStripItems.Count != 0); var item = manager.ToolStripItems.First(static (info) => info.Text == "TEST"); @@ -48,11 +54,22 @@ namespace BizHawk.Tests.Client.Common.Api DisplayManagerBase displayManager = new TestDisplayManager(mainFormApi.Emulator); TestToolManager toolManager = new TestToolManager(mainFormApi, config, displayManager); - TestExternalAPI externalApi = toolManager.Load(); + TestExternalAPI externalApi = LoadExternalTool(toolManager); Assert.AreEqual(0, externalApi.frameCount); toolManager.UpdateToolsBefore(); toolManager.UpdateToolsAfter(); Assert.AreEqual(1, externalApi.frameCount); } + + [TestMethod] + public void TestExternalToolCanUseApi() + { + IMainFormForApi mainFormApi = new MockMainFormForApi(new NullEmulator()); + DisplayManagerBase displayManager = new TestDisplayManager(mainFormApi.Emulator); + TestToolManager toolManager = new TestToolManager(mainFormApi, config, displayManager); + + TestExternalAPI externalApi = LoadExternalTool(toolManager); + Assert.AreEqual(mainFormApi.Emulator.AsVideoProviderOrDefault().BufferHeight, externalApi.APIs.EmuClient.BufferHeight()); + } } } diff --git a/src/BizHawk.Tests/Implementations/TestToolManager.cs b/src/BizHawk.Tests/Implementations/TestToolManager.cs index 6de05ce549..cd5464ee17 100644 --- a/src/BizHawk.Tests/Implementations/TestToolManager.cs +++ b/src/BizHawk.Tests/Implementations/TestToolManager.cs @@ -15,7 +15,7 @@ namespace BizHawk.Tests.Implementations mainFormApi, config, displayManager, - new ExternalToolManager(config, () => ("", ""), (p1, p2, p3) => false), + new ExternalToolManager(config, () => ("", "")), null, mainFormApi.Emulator, mainFormApi.MovieSession, @@ -29,7 +29,7 @@ namespace BizHawk.Tests.Implementations .ToList(); protected override bool CaptureIconAndName(object tool, Type toolType, ref Image? icon, ref string name) { - ExternalToolAttribute? eta = toolType.GetCustomAttribute(); + ExternalToolAttribute? eta = tool.GetType().GetCustomAttribute(); if (eta == null) throw new NotImplementedException(); // not an external tool @@ -41,6 +41,11 @@ namespace BizHawk.Tests.Implementations protected override void SetFormParent(IToolForm form) { } protected override void SetBaseProperties(IToolForm form) { } + protected override IExternalToolForm CreateInstanceFrom(string dllPath, string toolTypeName) + { + return (Activator.CreateInstanceFrom(dllPath, toolTypeName)!.Unwrap() as IExternalToolForm)!; + } + public override IEnumerable AvailableTools => throw new NotImplementedException(); public override (Image Icon, string Name) GetIconAndNameFor(Type toolType) => throw new NotImplementedException(); @@ -49,7 +54,6 @@ namespace BizHawk.Tests.Implementations public override void UpdateCheatRelatedTools(object sender, CheatCollection.CheatListEventArgs e) => throw new NotImplementedException(); protected override void AttachSettingHooks(IToolFormAutoConfig tool, ToolDialogSettings settings) => throw new NotImplementedException(); - protected override IExternalToolForm CreateInstanceFrom(string dllPath, string toolTypeName) => throw new NotImplementedException(); protected override void MaybeClearCheats() => throw new NotImplementedException(); protected override void SetFormClosingEvent(IToolForm form, Action action) => throw new NotImplementedException(); } diff --git a/src/BizHawk.Tests/Mocks/MockMainFormForTools.cs b/src/BizHawk.Tests/Mocks/MockMainFormForTools.cs index dafac87937..24c6ef4f87 100644 --- a/src/BizHawk.Tests/Mocks/MockMainFormForTools.cs +++ b/src/BizHawk.Tests/Mocks/MockMainFormForTools.cs @@ -11,6 +11,8 @@ namespace BizHawk.Tests.Mocks { public EmuClientApi? EmuClient { get ; set; } + public bool ShowMessageBox2(IDialogParent? owner, string text, string? caption = null, EMsgBoxIcon? icon = null, bool useOKCancel = false) => true; + public CheatCollection CheatList => throw new NotImplementedException(); public string CurrentlyOpenRom => throw new NotImplementedException(); @@ -55,7 +57,6 @@ namespace BizHawk.Tests.Mocks public IReadOnlyList? ShowFileMultiOpenDialog(IDialogParent dialogParent, string? filterStr, ref int filterIndex, string initDir, bool discardCWDChange = false, string? initFileName = null, bool maySelectMultiple = false, string? windowTitle = null) => throw new NotImplementedException(); public string? ShowFileSaveDialog(IDialogParent dialogParent, bool discardCWDChange, string? fileExt, string? filterStr, string initDir, string? initFileName, bool muteOverwriteWarning) => throw new NotImplementedException(); public void ShowMessageBox(IDialogParent? owner, string text, string? caption = null, EMsgBoxIcon? icon = null) => throw new NotImplementedException(); - public bool ShowMessageBox2(IDialogParent? owner, string text, string? caption = null, EMsgBoxIcon? icon = null, bool useOKCancel = false) => throw new NotImplementedException(); public bool? ShowMessageBox3(IDialogParent? owner, string text, string? caption = null, EMsgBoxIcon? icon = null) => throw new NotImplementedException(); public bool StartNewMovie(IMovie movie, bool record) => throw new NotImplementedException(); public void StartSound() => throw new NotImplementedException();