Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emulated SaveKey/AtariVox support for Stella libretro core #1007

Open
dionoid opened this issue Jan 16, 2024 · 11 comments
Open

Emulated SaveKey/AtariVox support for Stella libretro core #1007

dionoid opened this issue Jan 16, 2024 · 11 comments
Labels
Milestone

Comments

@dionoid
Copy link

dionoid commented Jan 16, 2024

The emulated SaveKey/AtariVox support in the Stella libretro core isn't working correctly.
While (emulated) SaveKey is supported and detected, the data written to the SaveKey is never written back to the savekey_eeprom.dat file, so highscores aren't saved across sessions in RetroArch. Note that SaveKey data is currently only written back to non-volatile storage in the destructor of the MT24LC256 class. My guess is that this destructor is never called by the libretro core.
I think saving SaveKey data back to file should be done when the libretro API retro_unload_game is called. I see libretro.cxx has implemented libretro's retro_unload_game and retro_deinit to do a stella.destroy(), which I think basically comes down to a OSystem.reset();, but appearently that doesn't destruct the SaveKey controller instance.

(EDIT: I did a wrong assumption, see discussion below)

@thrust26
Copy link
Member

theOSystem.reset() also causes the execution of the destructor of MT24LC256.

image

@dionoid
Copy link
Author

dionoid commented Jan 16, 2024

@thrust26 Ah, thanks for that. So my assumption that the destructor of MT24LC256 wasn't called, was wrong.
The RetroArch log console shows this error:

image

@sa666666
Copy link
Member

sa666666 commented Jan 16, 2024

There is additional work needed behind the scenes here for Stella to load/save files. Basically, we don't have a proper implementation of FSNodeLIBRETRO, so until that's fixed, none of this can proceed.

EDIT: I will add that the libretro port of Stella is very basic, and doesn't expose much configurability nor work with any external files.

@thrust26
Copy link
Member

thrust26 commented Jan 16, 2024

Ah, that's what I was already assuming. It seems that Stella is not allowed to write the file.

@sa666666
Copy link
Member

Ah, that's what I was already assuming. It seems that Stella is not allowed to write the file.

Not so much 'not allowed', but 'doesn't contain the code to even do it'.

@thrust26
Copy link
Member

EDIT: I will add that the libretro port of Stella is very basic, and doesn't expose much configurability nor work with any external files.

And that's why it is a pretty bad idea to base the 2600+ on it.

@dionoid
Copy link
Author

dionoid commented Jan 16, 2024

And that's why it is a pretty bad idea to base the 2600+ on it.

Libretro frontends (RetroArch, etc.) and OS-based distributions (RetroPi, Batocera) are pretty popular these days, so it's a pity that the Stella libretro core isn't as functional and configurable as the stand-alone version of Stella.

@thrust26
Copy link
Member

Maybe RetroArch is popular, but I really do not like its cumbersome UI.

And like @sa666666 already said, it only supports the basic functions of Stella. Also Libretro is split into two branches, one that is based on pre-4.0 Stella (no hardware acceleration) and a current one. Personally I am not very motivated to do any work for it.

@thrust26
Copy link
Member

Ah, that's what I was already assuming. It seems that Stella is not allowed to write the file.

Not so much 'not allowed', but 'doesn't contain the code to even do it'.

Oops. That's even worse.

@sa666666
Copy link
Member

And that's why it is a pretty bad idea to base the 2600+ on it.

Libretro frontends (RetroArch, etc.) and OS-based distributions (RetroPi, Batocera) are pretty popular these days, so it's a pity that the Stella libretro core isn't as functional and configurable as the stand-alone version of Stella.

The extra help we'd hoped to come for the libretro port didn't materialize. The first pass of porting Stella is done; the second pass was supposed to cover this extra stuff, but it didn't happen. I am more than willing to guide someone through the process, but I don't personally have the time to do it myself. At least not in the near future.

@thrust26 thrust26 added this to the Prio 3 milestone Jan 17, 2024
@dionoid
Copy link
Author

dionoid commented Mar 26, 2024

Libretro frontends (RetroArch, etc.) and OS-based distributions (RetroPi, Batocera) are pretty popular these days, so it's a pity that the Stella libretro core isn't as functional and configurable as the stand-alone version of Stella.

The extra help we'd hoped to come for the libretro port didn't materialize. The first pass of porting Stella is done; the second pass was supposed to cover this extra stuff, but it didn't happen. I am more than willing to guide someone through the process, but I don't personally have the time to do it myself. At least not in the near future.

At the moment I'm working on porting the Philips P2000T (M2000) emulator to Libretro and getting it included in their CI/CD process. As soon as that's done, I'm happy to help out with trying to get the Libretro virtual filesystem to work in Stella. Note that I'm a C/C#/Java/Python software developer, but I never worked with C++ before, so I may need some additional time to get familiar with the Stella codebase. @sa666666, what's the best way to contact you to give me a head start and guide me through the libretro core build process?

While I'm also not a huge fan of RetroArch's menu UI, I do think that it's a good idea to get the Stella libretro core to support things like Savekey and other filesystem-related features. Would be nice if these changes end up in the Atari 2600+, but if they don't, that's fine too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants