TASEmulators / TASEmulators/BizHawk

Fix (by adding an assert and testing) cases where cores return an undeclared sysID

Open
#4,215 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Meta Request: Feature/Enhancement
Dominant language
C#
Stars
2.8k
Forks
468
No merged PRs in 30d

Description

prior comment

From https://github.com/TASEmulators/BizHawk/pull/3635#discussion_r1667733459:

How about this:

  • RomLoader never sets SGB on any rom (since any GB rom could be used with SGB, they should all be detected as the same, and GB/GBC is the better choice)
  • There is no core preference for SGB, only GB and maybe GBC
  • Cores are perfectly honest with setting IEmulator.SystemId (not necessarily matching the one passed in, since a sync setting can choose between GB/GBC/SGB)
  • SystemId always matches a [CoreConstructor]—if it doesn't make sense to have that ctor, like with SGB which RomLoader will never use, it can simply be private and call something like this(default)

I think that would give the consistency I'm after while being simpler for regular users.

Later comment from https://github.com/TASEmulators/BizHawk/issues/3960#issuecomment-2599915071:
The value returned from IEmulator.SystemId should always match one of the [CoreConstructor]s. There should not be any case where the core "knows better" and swaps sysID based on the rom, for example—it is the frontend's responsibility to identify roms—but for the time being those hacks can remain and the core can simply add another attribute, and possibly a dummy ctor.

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 by tracing IEmulator.SystemId and the [CoreConstructor] attributes across the emulator cores. Add assertions and tests for cases where a core returns an undeclared sysID; done means every returned SystemId matches a declared CoreConstructor, while documented compatibility hacks remain covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.