Skip to content

retroarch: detect api availability for snes - #19

Open
strotlog wants to merge 6 commits into
alttpo:mainfrom
strotlog:ra_snes9x_support
Open

retroarch: detect api availability for snes#19
strotlog wants to merge 6 commits into
alttpo:mainfrom
strotlog:ra_snes9x_support

Conversation

@strotlog

Copy link
Copy Markdown

Fixes incompatibility between SNI and RA snes9x core, which doesn't support RA's READ_CORE_MEMORY.

I'll test this some more, but would like to have feedback/discussion on the approach (and code style, of course).

This also fixes the issue someone brought up recently in which SNI inconveniently displays a non-SNES RetroArch device when they wanted to connect to a different SNES emulator.

@strotlog
strotlog force-pushed the ra_snes9x_support branch 2 times, most recently from 86d15a9 to ff9e9fe Compare September 25, 2022 06:55
@strotlog

Copy link
Copy Markdown
Author

Force pushed to fix Printf at raclient.go:207

Fixes incompatibility between SNI and RA snes9x core, which doesn't
support RA's READ_CORE_MEMORY
@strotlog

Copy link
Copy Markdown
Author

Force pushed to revert inadvertent spacing change in 5 unmodified lines in driver.go

@strotlog

Copy link
Copy Markdown
Author

Realized SNI READ_CORE_RAM implementation never knew about RAM @ 0, so added a third state in the latest commit:

  • RCR==false : use RCM
  • RCR==true && rcrHasBusMapping==true : old RCR has the semantics of modern RCM (to maintain compatibility with old versions of RA)
  • RCR==true && rcrHasBusMapping==false : RAM @ 0, SRAM @ 0x20000 (SRAM not available for SnesABus clients due to A-bus ambiguity with unknown mapping)

Changed 'Attach' command behavior so SNI no longer drops the connection if it can't access the ROM.

I do need to test this more.

@strotlog

Copy link
Copy Markdown
Author

2 issues with the current PR:

  1. Transitioning between bsnes-mercury and snes9x (let's say with different roms) causes the wrong memory API to be used. I will try to fix this with polling and/or error self recovery to update the mapping and RA API in use.
    .. * Also brought up with jsd that I hope there can be a 'rom change detection' feature in SNI since many clients want to be detecting it; moreover, the player switching between RA bsnes-mercury and RA snes9x cores presents both a) a more difficult, non-obvious client implementation for rom change detection, and b) new opportunities within SNI's own necessary detection to surface a known rom change error to the client.

  2. It breaks ROM access in (HiROM and ExHiROM games, where SRAM size <=0x2000) for RA versions between 1.9.1 and 1.10.1. 'Secret of Evermore' and DKC3 are the only example games I know of with a client. All LoROM games, and all (Ex)HiROM games with >0x2000 bytes of SRAM, are already broken in terms of ROM or SRAM access support in those RA versions, so providing ROM+SRAM only, via RCR, per the pull request, is sufficient in that majority of games. (For posterity: the HiROM large SRAM bug was actually fixed in RA 1.10.3.) black-sliver, the owner of the SoE client, said they don't care if we drop ROM support (and thus AP SoE support) for that range of RA versions, though others might. DKC3 client came out after RA 1.10.1 with instructions to use at least 1.10.3. So in the PR I am tempted to continue de-supporting ROM access in these old-ish RA versions, unless, by fixing 1., it becomes convenient to do some HiROM-only RCM mappings to fix this. The test matrix is already becoming quite large for RA.

@JamesDunne

Copy link
Copy Markdown
Contributor

Definitely in favor of dropping support for RA versions before 1.10.1 or whatever the latest best is. If there is a valid reason to expend extra effort to make an older version work then that can be discussed when/if that ask comes up.

@JamesDunne

Copy link
Copy Markdown
Contributor

Im also tempted to clean things up in RA (via
PR) and return extra metadata like core name, system, rom file name, rom hash, etc with each command response. Probably a memory domain system would be great to have there too.

@JamesDunne
JamesDunne force-pushed the main branch 3 times, most recently from 5447bc9 to a272362 Compare February 5, 2023 20:00
@JamesDunne
JamesDunne force-pushed the main branch 3 times, most recently from 1b4f90c to 0ba3416 Compare April 10, 2023 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants