alttpo / sni

SNES Interface with gRPC API
MIT License
46 stars 11 forks source link

retroarch: detect api availability for snes #19

Open strotlog opened 1 year ago

strotlog commented 1 year ago

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 commented 1 year ago

Force pushed to fix Printf at raclient.go:207

strotlog commented 1 year ago

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

strotlog commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.