TASEmulators / BizHawk

BizHawk is a multi-system emulator written in C#. BizHawk provides nice features for casual gamers such as full screen, and joypad support in addition to full rerecording and debugging tools for all system cores.
http://tasvideos.org/BizHawk.html
Other
2.18k stars 382 forks source link

TAStudio hash mismatch message error on "Yes" to discard greenzone #4079

Open RetroEdit opened 1 week ago

RetroEdit commented 1 week ago

So in essence, currently even the "safer" option has issues. (I'm skeptical that it would work sensibly anyway, but at the moment it's completely broken.)

https://github.com/TASEmulators/BizHawk/blob/97785b2af2824622766b64b810b78a21e26aabb0/src/BizHawk.Client.Common/movie/MovieSession.cs#L198-L203

Repro

  1. Open game, File > Movie > Record movie... and then Tools > TAStudio to open TAStudio so it gets saved as a .tasproj. Close BizHawk
  2. Open another different game of the same platform.
  3. Use File > Movie > Recent to open the previously created .tasproj
  4. Press 'Yes' on the dialog that appears.

Output

System.NullReferenceException: Object reference not set to an instance of an object.
   at BizHawk.Client.Common.TasMovie.InvalidateAfter(Int32 frame) in C:\Emu\_repo\BizHawk\src\BizHawk.Client.Common\movie\tasproj\TasMovie.cs:line 125
   at BizHawk.Client.Common.TasMovie.InvalidateEntireGreenzone() in C:\Emu\_repo\BizHawk\src\BizHawk.Client.Common\movie\tasproj\TasMovie.cs:line 140
   at BizHawk.Client.Common.MovieSession.QueueNewMovie(IMovie movie, String systemId, String loadedRomHash, PathEntryCollection pathEntries, IDictionary`2 preferredCores) in C:\Emu\_repo\BizHawk\src\BizHawk.Client.Common\movie\MovieSession.cs:line 207
   at BizHawk.Client.EmuHawk.MainForm.StartNewMovie(IMovie movie, Boolean newMovie) in C:\Emu\_repo\BizHawk\src\BizHawk.Client.EmuHawk\MainForm.Movie.cs:line 42
   at BizHawk.Client.EmuHawk.MainForm.LoadMoviesFromRecent(String path) in C:\Emu\_repo\BizHawk\src\BizHawk.Client.EmuHawk\MainForm.cs:line 2111
   at BizHawk.Client.EmuHawk.ToolExtensions.ToolExtensions.<>c__DisplayClass0_1.<RecentMenu>b__3(Object o, EventArgs ev) in C:\Emu\_repo\BizHawk\src\BizHawk.Client.EmuHawk\Extensions\ToolExtensions.cs:line 65
   at System.Windows.Forms.ToolStripItem.RaiseEvent(Object key, EventArgs e)
   at System.Windows.Forms.ToolStripMenuItem.OnClick(EventArgs e)
   at System.Windows.Forms.ToolStripItem.HandleClick(EventArgs e)
   at System.Windows.Forms.ToolStripItem.HandleMouseUp(MouseEventArgs e)
   at System.Windows.Forms.ToolStrip.OnMouseUp(MouseEventArgs mea)
   at System.Windows.Forms.ToolStripDropDown.OnMouseUp(MouseEventArgs mea)
   at System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons button, Int32 clicks)
   at System.Windows.Forms.Control.WndProc(Message& m)
   at System.Windows.Forms.ToolStrip.WndProc(Message& m)
   at System.Windows.Forms.ToolStripDropDown.WndProc(Message& m)
   at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

Host env.

Morilli commented 1 week ago

This issue seems to go back to 84fd85c74933e802546f620737c39d89deda2765 where this feature was first introduced. GreenzoneInvalidated is only set when starting a movie via TAStudio, but you're opening the .tasproj via recent movies outside of TAStudio.

Morilli commented 1 week ago

And yes you're right, state 0 is never invalidated, so trying to seek to frame 1 will crash, even if yes was selected.

RetroEdit commented 1 week ago

My comment about work sensibly was more about a repeated workflow where it will prompt you every time you open the movie since the hash itself doesn't seem to be adjusted.

But mostly, in an ideal world it's not even desirable to have the movie open in hash mismatch cases in my opinion. Since autoload makes mistakes like this easy, and reopening with the correct game is what the user wants in those cases, not altering their movie.

I personally would have no issue with the movie open operation being rejected by default like system mismatch, but judging by #2328 and my rejected PR #2335 (which I'd forgotten about till now), this would probably be harmful to certain emulated systems and movie configurations like multi-disk bundler.

Morilli commented 3 days ago

Perhaps this feature should be reworked to instead of prompting for deleting greenzone, prompt whether to load the project in the first place and not touch the greenzone.

Even clearing greenzone would not invalidate branches, so trying to load a branch would also result in a crash assuming savestates are not valid. The best way is probably to warn the user that bizhawk might outright crash and let them proceed if they choose to.

RetroEdit commented 1 day ago

Branches could technically be repaired by playing back the branch input log from the start and re-creating the savestate.

But I agree: prompting whether to load the project would probably be best. Using a tasproj with a different game is not really supported behavior and probably not worth the effort to implement right now.