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

Implement SRAM saves #73

Closed
wants to merge 2 commits into from
Closed

Conversation

fetzerch
Copy link

@fetzerch fetzerch commented Nov 15, 2016

Libretro supports 2 different ways for saving SRAM data. Either the core can save directly into the save directory. Or the frontend needs to save from/load into the game RAM. While the first mechanism was implemented already, this PR implements the second approach - used for example in SNES emulators (with games such as Chrono Trigger or SMW).

RetroArch's implementation: https://github.com/libretro/RetroArch/blob/master/tasks/task_save.c

So far that's an early working implementation.

Related PRs:

ToDos:

  • Decide where to put the files. (SRAM saves are mostly emulator independent).
  • Move into class (decide on interface)
  • Save also RTC
  • Create save directory if it does not exist
  • (Autosave)

@garbear
Copy link
Owner

garbear commented Nov 15, 2016

Can you bump addons/kodi.game/addon.xml?

@fetzerch
Copy link
Author

Sure, done.

@Hedda
Copy link

Hedda commented Dec 19, 2016

Was this ever implemented?

@fetzerch
Copy link
Author

@garbear: Can you give some direction how to proceed with this?

I did a bit of testing and it seems to work fine for snes and nes. Even if you switch to a different emulator.

@Hedda: This is not yet part of any RetroPlayer builds, but I'd like to get it there.

@garbear
Copy link
Owner

garbear commented Dec 27, 2016

What if we call them in-game saves? This more clear for non-techies than SRAM saves

std::string path = URIUtils::AddFileToFolder(CProfilesManager::GetInstance().GetSavestatesFolder(), "SRAM");
path = URIUtils::AddFileToFolder(path, URIUtils::GetFileName(m_gamePath));

uint8_t *data;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialize these two variables

size_t size;
m_pStruct->GetMemory(GAME_MEMORY_SAVE_RAM, &data, &size);

std::string sramPath = path + ".sram";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Define this extension with the rest of the #defines at the top of the file

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SRAM doesn't make sense for web apps like 2048. I'm not against this extension, but what extensions do other emulators use?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Others use .srm or .sav. If we stick to .sav it's generic enough also for non-emulated games.

if (sramFile.Open(sramPath))
{
ssize_t read = sramFile.Read(data, size);
if (read != (ssize_t)size)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static_cast

ssize_t read = sramFile.Read(data, size);
if (read != (ssize_t)size)
{
CLog::Log(LOGERROR, "GAME: Failed read from SRAM file %s", sramPath.c_str());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redact path

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the size in bytes might be helpful, we might as well take advantage of knowing it

sramFile.Close();
if (written != (ssize_t)size)
{
CLog::Log(LOGERROR, "GAME: Unable to write complete SRAM file %s", sramPath.c_str());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include size written and total size?

@@ -97,6 +97,10 @@ class CGameClient : public ADDON::CAddonDll<DllGameClient, GameClient, game_clie
bool Serialize(uint8_t* data, size_t size);
bool Deserialize(const uint8_t* data, size_t size);

// SRAM saves
void SaveSRAM();
void LoadSRAM();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put these in a new class?

Change GetMemory to use a non-const pointer. This is necessary for
in-game (SRAM) saves that need to write into the memory.
@fetzerch fetzerch changed the base branch from retroplayer-17beta5 to retroplayer-17rc2 January 4, 2017 16:17
@fetzerch
Copy link
Author

fetzerch commented Jan 4, 2017

@garbear: I've updated the PR according to your comments. Autosave can come later I'd say. Do you want me to close this and PR to master directly?

For testing I'd recommend: 2048, snes games (e.g. Chrono Trigger and SMW)

@garbear
Copy link
Owner

garbear commented Jan 4, 2017

Looks much better. Can you send upstream? That'll put a few more eyes on it.

@fetzerch
Copy link
Author

fetzerch commented Jan 5, 2017

Sure, done in xbmc#11380.

@fetzerch fetzerch closed this Jan 5, 2017
garbear pushed a commit that referenced this pull request Jan 9, 2017
[addons] add first global add-on callback interface
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.

None yet

3 participants