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

[games] Implement in-game saves #11380

Merged
merged 2 commits into from Feb 5, 2017
Merged

[games] Implement in-game saves #11380

merged 2 commits into from Feb 5, 2017

Conversation

fetzerch
Copy link
Member

@fetzerch fetzerch commented Jan 5, 2017

Implement in-game (save ram) saves.

Description

This implements in-game saves where the frontend accesses the game's memory and saves/loads it to/from disk. This is mostly used in emulators for SRAM (battery backed up ram on cartridges) or memory cards.

For reference, these are the other ways how saving is implemented in the Game API:

Differences to save states:

  • Works only for supported games (e.g. emulated games with SRAM support)
  • Often works emulator independent (and can be used to start a game with one emulator and continue with another)
  • Visible in-game (e.g. in-game save game selection menus)
  • Closer to how saving in the good old days worked where one could only save at certain points where the game allowed it.

Previously PRed to the retroplayer banch in garbear#73
API-Change: Merging has to be synced with kodi-game/game.libretro#10.

Motivation and Context

Save games.

How Has This Been Tested?

Best tested with 2048 or SNES/GBA emulators.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@fetzerch fetzerch added Type: Feature non-breaking change which adds functionality Component: Games labels Jan 5, 2017
@fetzerch fetzerch requested a review from garbear January 5, 2017 22:42
@fetzerch fetzerch mentioned this pull request Jan 5, 2017
5 tasks
@@ -161,6 +162,9 @@ class CGameClient : public ADDON::CAddonDll
std::unique_ptr<IGameClientPlayback> m_playback; // Interface to control playback
GAME_REGION m_region; // Region of the loaded game

// In-game saves
std::unique_ptr<CGameClientInGameSaves> m_inGameSaves;

This comment was marked as spam.

This comment was marked as spam.


void CGameClientInGameSaves::Load()
{
Load(GAME_MEMORY_SAVE_RAM);

This comment was marked as spam.


void CGameClientInGameSaves::Save()
{
Save(GAME_MEMORY_SAVE_RAM);

This comment was marked as spam.

if (size > 0)
{
const std::string path = GetPath(memoryType);

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

/*!
* \brief This class implements in-game saves.
*
* Some games do not implement state persistence on their own, but rely on the frontend for saving their current

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@akva2
Copy link
Contributor

akva2 commented Jan 6, 2017

@akva2
Copy link
Contributor

akva2 commented Jan 6, 2017

(and it looks all good to me now).

Change GetMemory to use a non-const pointer. This is necessary for
in-game (SRAM) saves that need to write into the memory.
@fetzerch
Copy link
Member Author

fetzerch commented Jan 6, 2017

Thanks!

Does it make sense to save periodically (maybe every minute)? We don't control cores and they might crash before we're able to stop a game. Guess that could easily be implemented with a CTimer in CGameClient and could even be reused later for savestates. If it makes sense this can however come in a separate PR.

@akva2
Copy link
Contributor

akva2 commented Jan 6, 2017

maybe, but then we'd definitely need to ping-pong on the sram file, or you may end up dumping corrupted data (either because the core has stomped ram, or because of timing, core may be busy updating sram).

@garbear
Copy link
Member

garbear commented Jan 7, 2017

I would like to abstract the various software and hardware details of managing one's saved data.

I think the best interface would be a "Saved game" concept associated with a single game/file. The saved game would include the player's current progress, including awards/achievements (scraped from a ROM achievements service) and a history of saved data. The saved data would include all forms of the game's storage at a point in time, and we could eventually allow more fine-grained memory management like loading/saving/importing/exporting SRAM saves (per-file) or savestates (per-emulator).


#define INGAME_SAVES_DIRECTORY "InGameSaves"
#define INGAME_SAVES_EXTENSION_SAVE_RAM ".sav"
#define INGAME_SAVES_EXTENSION_RTC ".rtc"

This comment was marked as spam.

@garbear
Copy link
Member

garbear commented Feb 4, 2017

@fetzerch is this ready to go in?

@@ -276,6 +277,9 @@ bool CGameClient::OpenFile(const CFileItem& file, IGameAudioCallback* audio, IGa
if (!InitializeGameplay(file.GetPath(), audio, video))
return false;

m_inGameSaves.reset(new CGameClientInGameSaves(this, &m_struct));

This comment was marked as spam.

This comment was marked as spam.

@fetzerch
Copy link
Member Author

fetzerch commented Feb 5, 2017

From my side it's ready - but I thought you don't want it in its current from (without the abstraction).
Imho it would be nice to have save games working in nightly-builds and we can refactor it to a more fancy solution afterwards, but then at least the code doing the actual saving/loading is there.
Another advantage would be that we don't have to sync the game.libretro API change any more. It's always something you have to remember when rebasing the retroplayer branches.

Therefore: Your button, (needs a merge of kodi-game/game.libretro#10 as well).

@garbear
Copy link
Member

garbear commented Feb 5, 2017

jenkins build this please

@garbear garbear merged commit 6b29488 into xbmc:master Feb 5, 2017
@MartijnKaijser MartijnKaijser added this to the L 18.0-alpha1 milestone Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Games Type: Feature non-breaking change which adds functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants