TASEmulators / TASEmulators/BizHawk
ISettable<...> PutSyncSettings exists
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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