TASEmulators / TASEmulators/BizHawk

ISettable<...> PutSyncSettings exists

Open
#4,104 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

App: EmuHawk Meta
Dominant language
C#
Stars
2.8k
Forks
468
PR merge metrics
No merged PRs in 30d

Description

On discord, a problem came up related to sync settings and how changing them via the settings dialog caused an exception due to those changed sync settings being used even though the core has not been restarted yet.

The problem here is that setting sync settings immediately sets them in the running core instance via PutSyncSettings. The core is then both expected to return those sync settings from GetSyncSettings (so it NEEDS to store them) but also not change its active sync settings because, well, they're sync settings and changing them in the middle of running the emulator could cause anything from desync to outright crashes, as seen above.

Now I'm left wondering: Is there ever a valid scenario where this function needs to exist? If a core is always expected to not use the passed in sync settings because otherwise it might crash, what's the point in passing sync settings to the core in the first place? It is not intuitive to me that this structure is expected and I expect that my wip branch for mupen would also fail due to this design as it's using sync settings outside of the ctor.

The documentation for this function helpfully notes that sync settings should never be changed while recording, but does that mean there's an expectation for trying to change them while NOT recording? https://github.com/TASEmulators/BizHawk/blob/152b3ad6f5441e6d1e1eaa3aa131fa0685370edd/src/BizHawk.Emulation.Common/Interfaces/Services/ISettable.cs#L50

Also, I feel like every sensible implementation will require a reboot for sync settings change, so I don't know how useful the return value for that function is.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with src/BizHawk.Emulation.Common/Interfaces/Services/ISettable.cs around the sync-settings documentation and trace how PutSyncSettings and GetSyncSettings are used by the settings dialog and emulator cores. The work is complete only after the project agrees on the expected behavior for recording, rebooting, and active sync settings, then reflects that decision in the interface and related documentation.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
game-dev
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.