Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor InputDisplayGenerator and LogEntryGenerator handling #3742

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Morilli
Copy link
Collaborator

@Morilli Morilli commented Aug 16, 2023

This PR aims to simplify the Bk2InputDisplayGenerator and Bk2LogEntryGenerator handling, especially in regards to the many IController classes that exist, and improve caching.

Is this safe? It should be. Further simplifying the different controller types would help with the clarity here.

Basically, there is now one InputDisplayGenerator field on the InputManager class that always uses the ActiveController's ControllerDefinition as basis. As far as I understand the code, that's all that's required but it was being handled much more complicated before.

Additionally I've added a ILogEntryController : IController interface that is now applied to IMovieController as well, and just contains one Bk2LogEntryGenerator cached as a field on the ILogEntryController to be used for log entry generation from that controller, without having to create a new one every time or caching it on a Bk2Movie (which was apparently problematic before: f6135ab).

@Morilli Morilli requested a review from adelikat August 16, 2023 15:00
@kalimag
Copy link
Contributor

kalimag commented Aug 16, 2023

I get NullReferenceExceptions because IMovieSession.MovieController is null when trying to open TAStudio or record a movie straight after launching BizHawk. Doesn't happen if I play a movie first.

   at BizHawk.Client.EmuHawk.TAStudio.get_ControllerType() in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.EmuHawk\tools\TAStudio\TAStudio.ListView.cs:line 69
   at BizHawk.Client.EmuHawk.TAStudio.SetupBoolPatterns() in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.EmuHawk\tools\TAStudio\TAStudio.cs:line 436
   at BizHawk.Client.EmuHawk.TAStudio.Engage() in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.EmuHawk\tools\TAStudio\TAStudio.cs:line 221
   at BizHawk.Client.EmuHawk.TAStudio.Tastudio_Load(Object sender, EventArgs e) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.EmuHawk\tools\TAStudio\TAStudio.cs:line 161
   at System.Windows.Forms.Form.OnLoad(EventArgs e)
   at BizHawk.Client.EmuHawk.FormBase.OnLoad(EventArgs e) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.EmuHawk\FormBase.cs:line 71
   at BizHawk.Client.Common.Bk2Movie.WriteInputLog(TextWriter writer) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\movie\bk2\Bk2Movie.InputLog.cs:line 18
   at BizHawk.Client.Common.ZipStateSaver.<>c__DisplayClass7_0.<PutLump>b__0(Stream s) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\savestates\ZipStateSaver.cs:line 56
   at BizHawk.Client.Common.FrameworkZipWriter.WriteItem(String name, Action`1 callback, Boolean zstdCompress) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\FrameworkZipWriter.cs:line 44
   at BizHawk.Client.Common.ZipStateSaver.PutLump(BinaryStateLump lump, Action`1 callback, Boolean zstdCompress) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\savestates\ZipStateSaver.cs:line 37
   at BizHawk.Client.Common.ZipStateSaver.PutLump(BinaryStateLump lump, Action`1 callback) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\savestates\ZipStateSaver.cs:line 53
   at BizHawk.Client.Common.Bk2Movie.AddBk2Lumps(ZipStateSaver bs) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\movie\bk2\Bk2Movie.IO.cs:line 90
   at BizHawk.Client.Common.Bk2Movie.AddLumps(ZipStateSaver bs, Boolean isBackup) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\movie\bk2\Bk2Movie.IO.cs:line 81
   at BizHawk.Client.Common.Bk2Movie.Write(String fn, Boolean isBackup) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\movie\bk2\Bk2Movie.IO.cs:line 44
   at BizHawk.Client.Common.Bk2Movie.Save() in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.Common\movie\bk2\Bk2Movie.IO.cs:line 15
   at BizHawk.Client.EmuHawk.RecordMovie.Ok_Click(Object sender, EventArgs e) in H:\Projekte\Visual Studio\BizHawk-upstream\src\BizHawk.Client.EmuHawk\movie\RecordMovie.cs:line 270
   ...

@YoshiRulz YoshiRulz self-requested a review August 16, 2023 18:39
@tommai78101
Copy link
Contributor

I get a NullReferenceException when attempting to load TAStudio. I need TAStudio to open before I can run my external tool.

System.NullReferenceException: Object reference not set to an instance of an object.
   at BizHawk.Client.EmuHawk.TAStudio.UpdateAutoFire(String button, Nullable`1 isOn)
   at BizHawk.Client.EmuHawk.TAStudio.UpdateAutoFire()
   at BizHawk.Client.EmuHawk.TAStudio.LoadFile(FileInfo file, Boolean startsFromSavestate, Int32 gotoFrame)
   at BizHawk.Client.EmuHawk.TAStudio.Engage()
   at BizHawk.Client.EmuHawk.TAStudio.Tastudio_Load(Object sender, EventArgs e)
   at System.Windows.Forms.Form.OnLoad(EventArgs e)
   at BizHawk.Client.EmuHawk.FormBase.OnLoad(EventArgs e)

@Morilli
Copy link
Collaborator Author

Morilli commented Aug 20, 2023

I get a NullReferenceException when attempting to load TAStudio.

When does this happen? Just opening TAStudio works fine for me, are there certain steps required?

@tommai78101
Copy link
Contributor

tommai78101 commented Aug 20, 2023

I get a NullReferenceException when attempting to load TAStudio.

When does this happen? Just opening TAStudio works fine for me, are there certain steps required?

Yes, You need to have an external tool set to auto-load a game when EmuHawk starts, which means TAStudio will also need to be set to auto-load a TASPROJ file when EmuHawk launches as well.

So, it's more like:

  1. Launch EmuHawk. (Don't worry about this step for now.)
  2. Open TAStudio.
  3. Open an external tool that interfaces with Bk2LogEntryGenerator. You can use mine as a quick way for testing: https://github.com/tommai78101/Bizhawk-GeneticAlgorithmBot/tree/experiment/neat (I will put out a new build soon for you.)
  4. Set it up so that TAStudio will auto-load when EmuHawk launches.
  5. Set it up so that TAStuio will open a TASPROJ file when it launches.
  6. Set up EmuHawk so that preferences are saved automatically. TAStudio preferences shall then be stored in the config.ini file, along with a reference path to the most recently opened TASPROJ file.
  7. Set up EmuHawk to auto-launch a game that the TASPROJ uses when EmuHawk launches. (Wordy, I know...)
  8. Properly close all tools first.
  9. Properly close EmuHawk.
  10. If possible, set up Visual Studio 2022 to attach automatically to EmuHawk when the debugger starts debugging.
  11. Otherwise, skip 10, and launch EmuHawk.
  12. I believe you will first get 2 dialogs showing up from the external tool. Those are warning alert boxes that tells the user to load TAStudio first. Go ahead and close them.
  13. EmuHawk should then auto-load TAStudio and attempts to open the TASPROJ. This is where NullReferenceException starts occuring.

Let me know if you need more information.

@Morilli
Copy link
Collaborator Author

Morilli commented Aug 21, 2023

I just get BENIGN: Couldn't find type GeneticAlgorithmBot.GeneticAlgorithmBot and it doesn't even autoload the external tool at all...

@tommai78101
Copy link
Contributor

tommai78101 commented Aug 21, 2023

I just get BENIGN: Couldn't find type GeneticAlgorithmBot.GeneticAlgorithmBot and it doesn't even autoload the external tool at all...

You're using the wrong build. That's an older version prior to the Bot settings file changes, prior to the v2.9 stable release. Here's the actual latest build.

GeneticAlgorithmBot-v1.0.4-dev.zip

(Pre-release here: https://github.com/tommai78101/Bizhawk-GeneticAlgorithmBot/releases/tag/neat-1.0.4)

I compiled my bot with the latest BizHawk dev build, so it should at least get you past that benign error.


UPDATE: Actually I got a different error when I don't use VS2022 to attach to the EmuHawk.exe process. YoshiRulz said this might be a completely different bug that's happening inside TAStudio.AskSaveChanges().

************** Exception Text **************
System.NullReferenceException: Object reference not set to an instance of an object.
   at System.Windows.Forms.ToolStripControlHost.SetVisibleCore(Boolean visible)
   at BizHawk.Client.EmuHawk.TAStudio.UpdateProgressBar()
   at BizHawk.Client.EmuHawk.TAStudio.StopSeeking(Boolean skipRecModeCheck)
   at BizHawk.Client.EmuHawk.TAStudio.AskSaveChanges()
   at System.Linq.Enumerable.WhereSelectListIterator`2.MoveNext()
   at System.Linq.Enumerable.All[TSource](IEnumerable`1 source, Func`2 predicate)
   at BizHawk.Client.EmuHawk.MainForm.<.ctor>b__6_9(Object o, CancelEventArgs e)
   at System.Windows.Forms.Form.OnClosing(CancelEventArgs e)
   at System.Windows.Forms.Form.WmClose(Message& m)
   at BizHawk.Client.EmuHawk.MainForm.WndProc(Message& m)
   at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

Now I'm thinking it might not be related to your Bk2LogEventGenerator changes. But I never gotten any issues like this on the latest master branch before, only when checking out this PR.


UPDATE 2: Found the source of the crash I reported earlier 18 hours ago.

image

It's located in TAStudio.ListView.cs, line 506. AxisPatterns here is null when attempting to open TAStudio while the game is loaded.


FINAL UPDATE: I figured out my issue now. SetupBoolPatterns() is missing in Engage() in TAStudio.cs file, somewhere between line 219 and 222. It was there in the master branch, thus this PR may have accidentally removed it.

https://github.com/TASEmulators/BizHawk/pull/3742/files#diff-f8f29ffce77520db2b91d44295addeb70d8c3ea00d4f0b8201252c82b648ce1cL221-L222

@adelikat
Copy link
Contributor

adelikat commented May 7, 2024

I'm really excited to see this change

@Morilli Morilli force-pushed the small-controller-refactor branch from 97690f7 to 62b1d80 Compare May 7, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants