Lemuroid icon indicating copy to clipboard operation
Lemuroid copied to clipboard

Implement Sync via SyncManager

Open newhinton opened this issue 1 year ago • 21 comments

Closes #508 Closes #356 Closes #310 Closes #264 Closes #158 Closes #7 Closes #552 Closes #572

This superseeds #552. Here i use the already implemented SaveSyncManagerImpl to bring this feature to fdroid.

Todo:

  • [x] cleanup: logging, splitting functions
  • [x] cleanup: splitting functions
  • [x] fault tolerance: proper handling of exceptions and nullpointers when files do not exist
  • [x] fix "ping-pong" syncing, because syncing also updates the timestamp. Touch source file to fix.

This is a replacement implementation for google drive, and therefore currently only available on the fdroid build. If we want this to be available to everyone, we need to think about migrating google drive users.

Long Term Todo:

  • [ ] Migrate Google Drive integration to SAF
  • [x] Sync states
  • [x] Calculate state-storage usage
  • [ ] Think about deletions

newhinton avatar Apr 09 '23 18:04 newhinton

Hi @newhinton . Thank you very much for the PR. It looks good. I left a few comments which I think should be addressed. Also I believe we should also handle States to keep the behaviour consistent with the Play Store version before we can release it otherwise the UI will be incomplete.

Regarding removing support for Google Drive and using this one instead I'm not sure it's the right approach for the Play Store release since it's a little bit more involved to configure (although more versatile).

Swordfish90 avatar Apr 13 '23 06:04 Swordfish90

The thing with google-drive is that we lock people to google drive on the play version.

I guess it would be possible to create a selector somewhere that switches "SaveSyncManagerImpl", but in the long term there are two implementation that both require maintenance, and the google-drive-one is a subset of SAF.

Does the google drive implementation handle deletes properly?

I left a few comments

Sorry, but what comments? I cant see any...

newhinton avatar Apr 13 '23 07:04 newhinton

@newhinton No. Sadly the current Google Drive implementation doesn't handle deletes. Deleted files will basically appear again at the next sync from the other devices. Handling those is possible, but would require to add a db table to keep track of them.

Swordfish90 avatar Apr 16 '23 11:04 Swordfish90

Generally i think we should try to unify play and free variants. It will reduce the codebase, and since external storage also allows access to google drive, we are not artificially locking in users to just one cloud service. With this implementation, only f-droid users will get access to SAF, and google play users HAVE to use google drive.

So my suggestion to this issue is to create a "bar" that is only available to the play-variant, that will explain that in future versions the google-drive integration will be removed in favor of SAF. Then users will know whats going on, and then we promt them to update their settings when we actually switch.

newhinton avatar Apr 20 '23 13:04 newhinton

Note to me:

The "computeSavesSpace" might throw an exception. Needs investigating&catching

"Complex" Cores need special handling, see citra which has folders instead of files. This does not work with my implementation.

newhinton avatar May 06 '23 17:05 newhinton

@Swordfish90 So, states are now supported aswell! However, i can currently only do all states. So the system-chooser has no actual functionality.

newhinton avatar May 07 '23 19:05 newhinton

There is still a bug somewhere. With each successive run of the sync, it creates a new folder, aka. "Saves (1)" so that there are many duplicate folders. I think i know why, but i need to investigate.

Edit:

~Actually, i have no clue. If someone with deep knowledge about the SAF could help out, that would be great.~

newhinton avatar May 16 '23 08:05 newhinton

I also implemented storage calculation. I used the "remote" location, because i assumed that this would be the important location, however, it seems that the original implementation had the local storage in mind. Please advise me which location to count.

With this, i should have reached feature parity to the google implementation.

I still think we should switch the google play variant over in the long term, the benefits are that there are no two implementations that need to be maintained, since SAF already allows google drive usage. Also the google play variant severely limits users in their choice of remotes.

newhinton avatar May 16 '23 20:05 newhinton

Is this PR still being worked on? I just found out the hard way that non-savestate saves don't seem to work for my gba games, losing hours of play, which I believe this aims to resolve.
If this PR is not being worked on anymore, or is gonna take a long time, it would not be a bad idea to at least display a warning in the app about the lack of functional saving/the risk of losing saves.

SharkWipf avatar Nov 09 '23 14:11 SharkWipf

This pull request is done, at least from my side. I have found a good way, but it needs to be reviewed and merged. Though i dont know if it solves your specific problem, and it will only be available on the f-droid-release.

newhinton avatar Nov 09 '23 19:11 newhinton

I struggle to understand why such a basic feature isn't implemented already, but thanks @newhinton . I built a copy of your branch, and it works great.

Baardi avatar Nov 24 '23 23:11 Baardi

Note: Lemuroid is struggling to load external changes to the save-files. Sort of understandable because Lemuroid doesn't officially support external save-files. Just a nice to know for others building this branch, or for the Lemuroid maintainers/newhinton before/if they decide to merge this

Baardi avatar Dec 28 '23 22:12 Baardi

@newhinton @Swordfish90 Is anything blocking these changes getting merged? Would be great to have this feature

johntringham avatar Feb 27 '24 20:02 johntringham

@johntringham Not really, at least not from my side. I've been using it. It needs a review to check for bugs, but otherwise there are no blockers

newhinton avatar Feb 27 '24 20:02 newhinton

This would be a killer feature for an already excellent emulation project. I'd really like to be able to sync saves to my Nextcloud across devices.

wallzero avatar Apr 18 '24 18:04 wallzero

I haven't had a chance to test this branch yet, but just adding that this sort of change would be really beneficial for my use case.

I am trying to avoid Google wherever possible, so my setup is running GrapheneOS and I use Syncthing for copying files back and forth from my computers, including ROMs. It would be really nice if my saves/savestates could just be included as part of that.

I appreciate all the work that goes into this project and even without being able to backup saves (in the way that I prefer), it's still probably the best emulation frontend I've used.

Edit: forgot to explicitly mention that I'm running the F-droid version.

gjbianco avatar May 02 '24 12:05 gjbianco

There is no new review since https://github.com/Swordfish90/Lemuroid/pull/632#issuecomment-1510298379 / 2023-04-16 (1.5 year ago), normally i would say that project is unmaintained but i still see commit on the main branch (most just bump of versions) ...

hope @Swordfish90 will take here another look soon.

PS: @newhinton thanks to your work till, now. (and also to @Swordfish90 for this project). Would you like to add #900 to your solve list? Any maybe publish an dirty debug .apk for testing?

genofire avatar Jul 23 '24 19:07 genofire

@genofire Sure, i can add a dirty apk. Though its really dirty, its my local build. It basically contains all of my changes, even those from some of the other PR's. I've also added another way to access the internal storage (#936). It should be very close to the latest master branch in this repository.

https://github.com/newhinton/Lemuroid/releases/tag/demo-release-0001

newhinton avatar Aug 07 '24 08:08 newhinton

@genofire Oh, and it reorders the savegames! so if you already have a savegame folder, you might need to reorder them into core-specific folders. (eg, game boy advance savegames need to be in the "mgba" folder)

newhinton avatar Aug 07 '24 08:08 newhinton

@newhinton wow thanks the "cloud-sync" works fantastic.

genofire avatar Aug 08 '24 08:08 genofire