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

"CellList's distinctness invariant" when selecting frames #3971

Closed
RetroEdit opened this issue Jul 18, 2024 · 5 comments
Closed

"CellList's distinctness invariant" when selecting frames #3971

RetroEdit opened this issue Jul 18, 2024 · 5 comments
Labels

Comments

@RetroEdit
Copy link
Contributor

RetroEdit commented Jul 18, 2024

This is new to the dev build. It isn't present in 2.9.1.

I went through git bisect and it yielded 2a67cf9 unsurprisingly.

Maybe this wouldn't be present if I built BizHawk in release configuration, but I didn't test that.

Repro

  1. Open a game, open TAStudio (Tools > TAStudio)
  2. Emulate one frame
  3. (edit) Click frame 1 and press Ctrl+A to select all frames
  • This appears to happen every time you click on a frame and press Ctrl+A for the first time, regardless of whether it's been emulated or not.

Output

This error dialog doesn't have copy pastable contents, so I'll leave getting the rest as an exercise for the reviewer.
If you Ignore, it repeats for all cells in the row and then it seems to be fine.

CellList's distinctness invariant was almost broken!
CellList.Add(Cell(r: 1, c: "CursorColumn"))

CellList_invariant

Host env.

@YoshiRulz YoshiRulz added Tool: TAStudio Repro: Regression from 2.9.1 Repro: Could not reproduce The reporter hasn't given enough info, or the fix was made and not recorded Reproducible bug Should only be added to issues with a `Repro: Affects` label. and removed Repro: Could not reproduce The reporter hasn't given enough info, or the fix was made and not recorded labels Jul 18, 2024
@YoshiRulz
Copy link
Member

Repro'd with Ctrl+Shift+A keyboard shortcut but not Edit > Select All, and it only happened if the cursor was on frame 0, regardless of whether I'd advanced through frame 1 or not. Also the assert message is missing for some reason under Mono. (For future reference, you should be able to copy the full error text with Ctrl+C, even though it's not a textbox.)

my stacktrace FWIW
System.Configuration.ConfigurationErrorsException: Error Initializing the configuration system. ---> System.Configuration.ConfigurationErrorsException: Unrecognized configuration section <System.Windows.Forms.ApplicationConfigurationSection> (/home/yoshi/Documents/BizHawk/output/EmuHawk.exe.config line 52)
  at System.Configuration.ConfigInfo.ThrowException (System.String text, System.Xml.XmlReader reader) [0x00007] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.SectionGroupInfo.ReadContent (System.Xml.XmlReader reader, System.Configuration.Configuration config, System.Boolean overrideAllowed, System.Boolean root) [0x00135] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.SectionGroupInfo.ReadRootData (System.Xml.XmlReader reader, System.Configuration.Configuration config, System.Boolean overrideAllowed) [0x00007] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.Configuration.ReadConfigFile (System.Xml.XmlReader reader, System.String fileName) [0x000ce] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.Configuration.Load () [0x00043] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.Configuration.Init (System.Configuration.Internal.IConfigSystem system, System.String configPath, System.Configuration.Configuration parent) [0x0005d] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.Configuration..ctor (System.Configuration.InternalConfigurationSystem system, System.String locationSubPath) [0x00056] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.InternalConfigurationFactory.Create (System.Type typeConfigHost, System.Object[] hostInitConfigurationParams) [0x0000d] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.ConfigurationManager.OpenExeConfigurationInternal (System.Configuration.ConfigurationUserLevel userLevel, System.Reflection.Assembly calling_assembly, System.String exePath) [0x000ef] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.ClientConfigurationSystem.get_Configuration () [0x0000e] in <7a1212a135b54a118e899519ceb4e770>:0 
   --- End of inner exception stack trace ---
  at System.Configuration.ClientConfigurationSystem.get_Configuration () [0x0002a] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.ClientConfigurationSystem.System.Configuration.Internal.IInternalConfigSystem.GetSection (System.String configKey) [0x00000] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.ConfigurationManager.GetSection (System.String sectionName) [0x00005] in <7a1212a135b54a118e899519ceb4e770>:0 
  at System.Configuration.PrivilegedConfigurationManager.GetSection (System.String sectionName) [0x00000] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.DiagnosticsConfiguration.GetConfigSection () [0x00000] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.DiagnosticsConfiguration.Initialize () [0x0002a] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.DiagnosticsConfiguration.get_IndentSize () [0x00000] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.TraceInternal.InitializeSettings () [0x0004e] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.TraceInternal.get_UseGlobalLock () [0x00000] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.TraceInternal.Fail (System.String message) [0x00000] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.TraceInternal.Assert (System.Boolean condition, System.String message) [0x00004] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at System.Diagnostics.Debug.Assert (System.Boolean condition, System.String message) [0x00000] in <6102ebb4f7b1424fb59b6e2b8a46c863>:0 
  at BizHawk.Client.EmuHawk.CellList.Add (BizHawk.Client.EmuHawk.Cell item) [0x0003a] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at BizHawk.Client.EmuHawk.InputRoll.SelectCell (BizHawk.Client.EmuHawk.Cell cell, System.Boolean toggle) [0x0011e] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at BizHawk.Client.EmuHawk.InputRoll.SelectRow (System.Int32 index, System.Boolean val) [0x00056] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at BizHawk.Client.EmuHawk.InputRoll.SelectAll () [0x00015] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at (wrapper remoting-invoke-with-check) BizHawk.Client.EmuHawk.InputRoll.SelectAll()
  at BizHawk.Client.EmuHawk.TAStudio.SelectAllMenuItem_Click (System.Object sender, System.EventArgs e) [0x00001] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at BizHawk.Client.EmuHawk.TAStudio.SelectAllExternal () [0x00000] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at (wrapper remoting-invoke-with-check) BizHawk.Client.EmuHawk.TAStudio.SelectAllExternal()
  at BizHawk.Client.EmuHawk.MainForm.CheckHotkey (System.String trigger) [0x02075] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at BizHawk.Client.EmuHawk.MainForm.<ProcessInput>b__155_0 (System.Boolean current, System.String trigger) [0x00000] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at System.Linq.Enumerable.Aggregate[TSource,TAccumulate] (System.Collections.Generic.IEnumerable`1[T] source, TAccumulate seed, System.Func`3[T1,T2,TResult] func) [0x0002e] in <d10d7221895a42f490b54d1cfd6b83c7>:0 
  at BizHawk.Client.EmuHawk.MainForm.ProcessInput (BizHawk.Client.Common.InputCoalescer hotkeyCoalescer, BizHawk.Client.Common.ControllerInputCoalescer finalHostController, System.Func`2[T,TResult] searchHotkeyBindings, System.Func`2[T,TResult] activeControllerHasBinding) [0x00187] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at BizHawk.Client.EmuHawk.MainForm.ProgramRunLoop () [0x000b7] in <207abf082c154fe2a6e6c50f192ee1eb>:0 
  at (wrapper remoting-invoke-with-check) BizHawk.Client.EmuHawk.MainForm.ProgramRunLoop()
  at BizHawk.Client.EmuHawk.Program.SubMain (System.String[] args) [0x004b3] in <207abf082c154fe2a6e6c50f192ee1eb>:0 

@RetroEdit
Copy link
Contributor Author

RetroEdit commented Jul 18, 2024

I was able to reproduce this even when my cursor wasn't on frame 0. It happens basically anytime I try to Ctrl+A select for the first time on a particular frame. My previous repro steps before I simplified them (which I just retested now) were as follows:

Original:

  1. Open a ROM, open TAStudio
  2. Frame advance once
  3. Inserted 99999 frames to the end (instant)
  4. Marker at 10000, marker at 20000
  5. Seek frame 20000; wait for seek to finish
  6. Click on frame 19999 to select it.
  7. Ctrl+A

Simplify 1: same, but insert only 500 frames, markers on frames 200 and 400, don't seek, and select frame 201.

Simplify 2: same, but insert only 10 frames, markers on frames 2 and 11, don't seek, and select frame 10.

I only frame-advanced for the sake of my original methodology of inserting frames, since I didn't want to insert at frame 0.

@Morilli
Copy link
Collaborator

Morilli commented Sep 8, 2024

I'm just not sure what the "correct" fix here is. The base problem is pretty simple: Some way or another an attempt is made to select a certain number of rows from the input roll, without regard for whether any of them are already selected.

One way to fix this could be to just remove the Debug.Assert. We already have the logic to prevent duplicate selections and could just silently reject them.

Alternatively, every time a cell is to be selected, the calling code needs to make sure that this cell isn't already selected (I did something like that in 8278b30 already).

YoshiRulz added a commit that referenced this issue Sep 8, 2024
I believe this method was already non-atomic so this clear operation
shouldn't change anything
@YoshiRulz
Copy link
Member

Try that ^

@YoshiRulz YoshiRulz added the Repro: Patch pending Potentially fixed in dev build, see readme for download label Sep 8, 2024
@Morilli Morilli closed this as completed in a955868 Sep 9, 2024
@Morilli
Copy link
Collaborator

Morilli commented Sep 9, 2024

Reopen once this inevitably happens again on some other code path.

@YoshiRulz YoshiRulz added Repro: Fixed/added in 2.10 dev and removed Repro: Patch pending Potentially fixed in dev build, see readme for download labels Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants