Skip to content

Add battery save import/export methods - #68

Merged
Frostbyte0x70 merged 7 commits into
SkyTemple:masterfrom
Konstantysz:feature/backup-import-export-api
Dec 9, 2025
Merged

Add battery save import/export methods#68
Frostbyte0x70 merged 7 commits into
SkyTemple:masterfrom
Konstantysz:feature/backup-import-export-api

Conversation

@Konstantysz

Copy link
Copy Markdown
Contributor

Summary

Adds Python bindings for battery save import/export to match the new DeSmuME interface API.

Dependencies

⚠️ Requires DeSmuME PR: https://github.com/TASEmulators/desmume/pull/930]

This PR temporarily points the desmume_src submodule to my fork until the upstream PR is merged.

Changes

  • Added DeSmuME_Backup class with three methods:
    • backup.import_file(path) - auto-detects format (.sav, .dsv, .duc, .dss)
    • backup.import_raw(path, size) - manual size override
    • backup.export_file(path) - export to .dsv
  • Added backup property to DeSmuME class
  • Temporarly updated submodule to konstantysz/desmume fork

Usage Example

from desmume.emulator import DeSmuME

emu = DeSmuME()
emu.open('pokemon_black2.nds')

# Import battery save
if emu.backup.import_file('my_save.sav'):
    print("Save loaded!")

Use Case

I'm building a Pokemon shiny hunting tool that needs to load battery saves programmatically across multiple emulator instances. Currently there's no way to do this without manual GUI interaction.

Testing

Functional testing pending - will compile custom libdesmume.dll and test with Pokemon ROMs once DeSmuME PR is reviewed.
Once the upstream DeSmuME PR merges, I'll update the submodule to point back to SkyTemple/desmume.


@Konstantysz

Konstantysz commented Oct 30, 2025

Copy link
Copy Markdown
Contributor Author

Update: DeSmuME PR has been merged! The backup import/export API is now part of the official DeSmuME repository.

Submodule updated I've updated the desmume_src submodule to point to upstream master instead of my fork. The changes are now based on the official codebase. Tested and working.

  • Import: Successfully tested with .sav files
  • Export: Successfully tested (exports to .sav format)

Python bindings work correctly with the merged C++ API. This PR is now fully ready for review. Let me know if you need any changes!

I think I can ping you @mike8699 as a maintainer.

@theCapypara theCapypara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great stuff! Thanks a lot!
Unfortunately this project is currently without a maintainer, but as the original author it LGTM!

@Konstantysz

Copy link
Copy Markdown
Contributor Author

Great stuff! Thanks a lot! Unfortunately this project is currently without a maintainer, but as the original author it LGTM!

So there is no possibility for merge?

@Konstantysz

Copy link
Copy Markdown
Contributor Author

@mike8699 what do you think about this PR?

@mike8699

Copy link
Copy Markdown
Contributor

@mike8699 what do you think about this PR?

Looks great. Unfortunately, I am not a maintainer but just a past contributor 🙃

@theCapypara

Copy link
Copy Markdown
Contributor

@Frostbyte0x70

@Frostbyte0x70

Copy link
Copy Markdown
Member

Hi, sorry for the delay, I was not receiving notifications until I was pinged.

Unfortunately, it seems like the build is failing for reasons unrelated to your changes (I opened #70 to fix the workflow, but that one fails with an error). There's currently no one available to fix the build, so I'm afraid that both #70 and by extension this PR will remain unmerged for now.
If someone happens to fix the build in the future, I'll let you know so you can rebase this on top of #70 after it's merged, then we can run the CI on this PR again.

Thanks for taking the time to contribute, and sorry for the disappointment.

@Konstantysz

Copy link
Copy Markdown
Contributor Author

Hi, sorry for the delay, I was not receiving notifications until I was pinged.

Unfortunately, it seems like the build is failing for reasons unrelated to your changes (I opened #70 to fix the workflow, but that one fails with an error). There's currently no one available to fix the build, so I'm afraid that both #70 and by extension this PR will remain unmerged for now. If someone happens to fix the build in the future, I'll let you know so you can rebase this on top of #70 after it's merged, then we can run the CI on this PR again.

Thanks for taking the time to contribute, and sorry for the disappointment.

@Frostbyte0x70, I created fixes for the Github CI in #71. It passed all runs, beside macos-12, which was waiting indefinitely.

@Konstantysz
Konstantysz force-pushed the feature/backup-import-export-api branch from d09c55d to dd4392e Compare December 1, 2025 20:20
Comment thread .gitmodules Outdated
[submodule "desmume_src"]
path = desmume_src
url = https://github.com/SkyTemple/desmume.git
url = https://github.com/TASEmulators/desmume.git

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't this mean we'll lose the commits added in our fork of desmume? The commit that's currently checked out in the submodule has a few commits not present upstream, and the branch said commit is in has those plus a few more that were added later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So should I port my recent changes made to TASEmulators/desmume to your fork? It's actually just +43 lines of code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are those the only changes needed from the Desmume repo to get your feature to work? If so, I could just cherry-pick that commit into the branch used by the submodule.

@Konstantysz Konstantysz Dec 2, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just need changes from this PR. But I don't get which branch should I pull from? I'll pull from the correct place, port changes and test, but I don't understand the starting point for me :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIRC I just backported some compilation fixes onto an older commit because I didn't want to fully update in case things break, there shouldn't be any functional difference and it's probably fine to update.
I'd recommend seeing what the base commit was that exists in the upstream repo and what commits I actually added on top of that, should be fairly easy with the git CLI, just kinda hard with GitHub's UI, though the network graph should also show it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd recommend seeing what the base commit was that exists in the upstream repo and what commits I actually added on top of that

I believe the second link I posted above shows exactly that. For now, I'll just replicate the changes from Konstantysz's PR on the branch used by the submodule (which seems to be interface-build).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alright, I've added the changes from the PR. @Konstantysz you can now checkout commit SkyTemple/desmume@42ee6ea on the Desmume submodule and push again.

@Frostbyte0x70 Frostbyte0x70 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It finally works! 🎉
Thanks!

@Frostbyte0x70
Frostbyte0x70 merged commit 147f8f1 into SkyTemple:master Dec 9, 2025
10 checks passed
@Konstantysz

Copy link
Copy Markdown
Contributor Author

It finally works! 🎉
Thanks!

Thanks too!

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.

4 participants