Skip to content

feat: Add SDH-GameSync v1.0.0 #784

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

Merged
merged 8 commits into from
Apr 14, 2025

Conversation

AkazaRenn
Copy link
Contributor

@AkazaRenn AkazaRenn commented Mar 15, 2025

SDH-GameSync

A fork of Decky Cloud Save that I have rewritten (probably) over 70% of the code. The major differences between this plugin and Decky Cloud Save are:

  1. Support for per-game sync, it syncs games files based on the filter specific to the starting/stopping game, saved in the settings folder named <appId>.filter
  2. Support for screenshot upload, when a screenshot is taken, it can upload it to the cloud and remove the local copy automatically
  3. Refactored filters edit page that supports adding/removing both include and exclude sync rules in the same file with the ability to reorder entries
  4. Improved log page that supports separation of plugin logs and each sync logs, also a refresh button and allows resync to be triggered via controller

Task Checklist

Developer

  • I am the original author or an authorized maintainer of this plugin.
  • I have abided by the licenses of the libraries I am utilizing, including attaching license notices where appropriate.

Plugin

  • I have verified that my plugin works properly on the Stable and Beta update channels of SteamOS.
  • I have verified my plugin is unique or provides more/alternative functionality to a plugin already on the store.

Backend

  • No: I am using a custom backend other than Python.
  • No: I am using a tool or software from a 3rd party FOSS project that does not have it's dependencies statically linked.
  • Yes: I am using a custom binary that has all of it's dependencies statically linked.

Community

  • I have tested and left feedback on two other pull requests for new or updating plugins.
  • I have commented links to my testing report in this PR.

#771 (comment)
#778 (comment)

Testing

  • Tested by a third party on SteamOS Stable or Beta update channel.

@AkazaRenn AkazaRenn requested a review from a team as a code owner March 15, 2025 16:26
@EMERALD0874
Copy link
Member

Can you please give context as to why this needs to be a separate plugin listing/fork and not a PR to the original plugin?

@AkazaRenn
Copy link
Contributor Author

AkazaRenn commented Mar 15, 2025

@EMERALD0874 yeah it's actually pretty straightforward, the change is too large: https://github.com/GedasFX/decky-cloud-save/compare/main...AkazaRenn:SDH-CloudSaveFork:main?expand=1
This plugin's implementation is quite different from Decky Cloud Save under the hood. If I merge +4,566 -3,443 lines to Decky Cloud Save, it will simply be replace by SDH-CloudSaveFork, and there would definitely be a lot of current users unhappy with the change, that's something I want to avoid. Publishing it as a new plugin would allow users to choose between them.

@GedasFX
Copy link
Contributor

GedasFX commented Mar 15, 2025

@AkazaRenn crazy to see you again!

A bit of history: I have developed DCS as a means to just back up certain paths to somewhere as I had Lutris constantly delete my save files for no reason. It was my first plugin, and really my first time using python in a "more serious" setting, so that plugin really desperately needed a remake.

Over time people discovered the plugin and kept asking for features, that really I had no interest in adding, but as I was new to FOSS, and had time still, I added some requests. After a while, life hit hard, and I had honestly by far the busiest year of my life. Had no time to add features. Caved to pressure and managed to squeeze in the MUCH MUCH requested bi-sync feature, which flooded me with confused people how to use it. Left me really drained and just made me want to quit FOSS development entirely.

Final nail in the coffin, however, was the base plugin itself - it was purpose built for one feature only - backup, and the code base made any deviations very hard. Whether its backwards compatibility, changing users habit, or anything else, that plugin needed a complete rewrite. so that's why (almost a year ago) started work on ludusavi wrapper, which I finished today. Not much work, but thats how busy I just was xD

Simply put, I want to get rid myself of that plugin. It has outlived its usefulness, getting anything done is a delicate dance of backwards compatibility, convoluted testing, and support for features, I had no desire to see in the first place. If someone wants to take over, I am all for it, but as mentioned before, its best to just leave it deprecated, have people move on, and then remove it.

@GedasFX
Copy link
Contributor

GedasFX commented Mar 15, 2025

I am more than willing to review + test this, as regardless if it was gonna be a PR to original repo, then it would have to have my review anyway 😄

@AkazaRenn
Copy link
Contributor Author

Hiiiii @GedasFX , good to see you back! I'm so glad to hear the support from you and I cannot thank you enough for the ground work provided. Sorry that I said I could do it during the holidays but managed to delay it for over three months 😆

If you want to give a check on the plugin, fell free to get it from here https://github.com/AkazaRenn/SDH-CloudSaveFork/releases/tag/20250315161024

@GedasFX
Copy link
Contributor

GedasFX commented Mar 15, 2025

Its all good! Glad you are alright ^^ That was basically last week when you mentioned it to me anyway :D

I started to have a look at the code, and I believe it will have issues as you changed the base image of the Dockerfile, and I remember I had issues with CI, but may have been fixed since last I pushed.

@AkazaRenn
Copy link
Contributor Author

@GedasFX here comes the interesting part, the docker image now is no longer responsible for building the image since the decky binary will take care of it. That image, is where I steal the rclone binary from 😉

@EMERALD0874
Copy link
Member

@AkazaRenn Given Gedas' interest in releasing maintainership, could you handle this as either PR to their repo or a transfer of maintainership of the existing Decky Cloud Save? I'd prefer we not have a plugin on the store that isn't being actively maintained/supported.

@AkazaRenn
Copy link
Contributor Author

@AkazaRenn Given Gedas' interest in releasing maintainership, could you handle this as either PR to their repo or a transfer of maintainership of the existing Decky Cloud Save? I'd prefer we not have a plugin on the store that isn't being actively maintained/supported.

I'm not sure current users will be happy about it suddenly getting revamped into something totally different. To me the better way is to keep Gedas' plugin in store until it's broken, then we take it down and recommend users to migrate to mine.

@EMERALD0874
Copy link
Member

If it's something totally different, I'm concerned that repeatedly referring to your work as "a fork of Decky Cloud Save" and "Cloud Save Fork" could confuse users into thinking it's just an upgraded version of the original Decky Cloud Save like I thought. An original plugin name and a description change to "A fork of Decky Cloud Save with [features added]" may help here.

@AkazaRenn
Copy link
Contributor Author

@EMERALD0874 sure let me think about it, the major reason that it's called CloudSaveFork is because I suck at naming 🥹

@AkazaRenn
Copy link
Contributor Author

@EMERALD0874 how about "Game Sync"? Would it be too general?

@EMERALD0874
Copy link
Member

If that's a name you like, it's fine by me.

@AkazaRenn
Copy link
Contributor Author

@EMERALD0874 updated!

@AkazaRenn AkazaRenn changed the title feat: Add SDH-CloudSaveFork v1.0.0 feat: Add SDH-GameSync v1.0.0 Mar 16, 2025
@EMERALD0874
Copy link
Member

Please rebase from main and resolve merge conflicts so you aren't building multiple plugins. After that, I'll deploy to the testing store.

@AkazaRenn
Copy link
Contributor Author

@EMERALD0874 done, thanks for your help!

@AkazaRenn
Copy link
Contributor Author

AkazaRenn commented Mar 23, 2025

Updated with fixes in 859a91b

@GedasFX
Copy link
Contributor

GedasFX commented Mar 27, 2025

After I installed the plugin, the plugins stopped loading, but after a restart everything showed up normal... I will call this a fluke as been a while since i opened my deck again 😄

[1] Fixed ✅
[2] Not important for this release, will. call it resolved ✔️
[3] Fixed ✅
[4] Hmm, on my plugin it was not really an issue as bisync is opt-in, and usually users sync at least once before that. bisync copy does create an folder on remote. Bisync seems to not do that. Maybe can like try copy nothing initially? Can also catch exception and try doing initialize function: first copy, then run resync. This should avoid us all initial confusion.
[5] What issue, we have no issue 😄 ✔️
[6] Noted. ✅
[7] After adding the configuration option, I can't reproduce issue anymore, so I guess we are good! ✅
[8] Its really hard to explain, but here are my repro steps:

  1. select a row and click copy
  2. append new row
  3. select the new row (bottom)
  4. click "paste filter"
  5. observe the row getting deleted
    I will try fixing that because... 👇
    [9] I think I know what the issue is, will see if i can fix that.

The other 2 comments: ✅ and, epic! ✅

New stuff:
[10] Sync Failed toast opens the Target Filter page, which is a minor inconvenience. Did confuse me for a bit as to where I was 😄.

Some other thought I just got:
I find it kind of funny that screenshots upload destination is a big button, but sync destination is hidden under advanced settings 😄

@GedasFX
Copy link
Contributor

GedasFX commented Mar 27, 2025

So it turns out I cannot fork a fork, but just do this change, will fix it:
image

Because your GetFiltersFunction changes, it will rerun the effect, which will refetch the data.

@AkazaRenn
Copy link
Contributor Author

AkazaRenn commented Mar 27, 2025

@GedasFX
[4] I think I'll try to add a toast offering user to create the destination folder once they have setup a cloud provider, that should serve the purpose.
[8] copy/paste issue, I still cannot reproduce somehow...
[10] yeah I'm not sure if there's a way to set a different entry tab but couldn't figure out how

As for the screenshot destination, because screenshot is using copy and only syncs one way from local to cloud, it's harmless to change the destination, not like those involves downloading and even file deletions.

BTW I've figured out the filter tabs, turned out I really just need to create a wrap up function for shared filters.

Addressed everything but [10] in the latest commit db1f677

Copy link

github-actions bot commented Apr 4, 2025

This pull request is stale as no updates or testing reports have been posted within the last 7 days. Please ensure you are actively recruiting testers or resolving the issues discussed. If you do not remove the stale label or comment, this will be closed in 7 days. Please close this pull request if you need more time to resolve issues so we can keep our repo healthy.

@github-actions github-actions bot added the stale label Apr 4, 2025
@AkazaRenn
Copy link
Contributor Author

Couldn't figure out on [10] so I just made logs the first tab 😌Also renamed the context menu entry to Sync Logs & Filters to avoid confusion

@github-actions github-actions bot removed the stale label Apr 6, 2025
@GedasFX
Copy link
Contributor

GedasFX commented Apr 6, 2025

@AkazaRenn sorry for delays 😄 Planned to do testing on Thursday but we had store outages that day :/

Started checking, was the folder problem resolved on initial sync? I also recorded a video of the bug with the cut, I will post it to you on discord. Regardless of outcome its such a non-blocker, it can be fixed in a later version.

@GedasFX
Copy link
Contributor

GedasFX commented Apr 6, 2025

Plugin Testing Report

Installed Plugins

  • Decky Ludusavi - 1.1.0-alpha.1
  • ProtonDB Badges - 1.1.0
  • Decky Recorder - 0.4.1
  • PowerTools - 2.0.3
  • Emuchievements - 2.0.3
  • SteamGridDB - 1.5.1-loaderv2
  • Web Browser - 1.4.0
  • IsThereAnyDeal for Deck - 1.0.3
  • HLTB for Deck - 2.0.4
  • Game Sync - 1.0.0-641455d
  • Decky Cloud Save - 1.4.2
  • vibrantDeck - 2.0.1
  • Steam Deck HQ - 1.0-0338b3d
  • Junk-Store - 1.1.9

Specifications

  • SteamOS 3.6.24 (Stable)
  • Steam 1743717600 (Stable)
  • Decky 3.1.5 (Stable)

Issues

Has the following major blocking issue(s):
N/A

Has the following minor non-blocking issue(s):
[2-1] When user first installs the program, they need to run a resync. This needs to be done manually. It is possible to do within app, but the documentation is a bit lacking.

[2-2] The game sync configuration is hidden in game context menu. It is okay and a great place, however I found myself confused where it was. I did not test it last time as I thought it was the global filter with a game open. If you know where it is, it works great, but some guidance could be good.

[2-3] Caused by [2-1]. The sync on game start triggers global and per game syncs. While the game sync is successful, if user did not fix the [2-1], they will be plagued with error message that sync failed (even though game sync succeeded).

[2-4] After selecting provider we now create destination folder. Its good (up until resync), but changing the destination does not open folder creation popup.

Summary

Features Checklist
List of features I checked, mostly to keep track of this massive plugin 😄

Installation.✔️
Cloud Provider Selection. ✔️
Plugin Logs ✔️.
Upload Screenshots ✔️
Upload Screenshots. Destination ✔️
Global Filter Creation ✔️
Shared Filter Creation ✔️
Global Sync. ⚠️. See [2-1].
Global Sync. Sync on Stop. ✔️
Global Sync. Sync on Start. ✔️
Global Sync. Upload. ✔️
Global Sync. Download. ✔️
Game Sync. ⚠️. See [2-2].
Game Sync. Sync on Stop. ⚠️. See [2-3].
Game Sync. Sync on Start. ⚠️. See [2-3].
Game Sync. Upload. ✔️
Game Sync. Download. ✔️
Sync Root Selection ✔️
Sync Destination Selection ⚠️. See [2-4].
Strict Game Sync ✔️
Strict Game Sync. Warn when modifying paths and offer upload. ✔️

Previous Report... Review
In this sections, numbers [X] will refer to previous report.
Everything was fixed/resolved. Some aspects warrant additional comment.

[1-4] A popup now shows. It is a bit jank as it opens when sidebar opens, but from testing, its natural, so it will be good. I was unable to close the popup until it was ready, so I think we are good for now.

[1-8] Its an expectation issue. I guess not a bug. In my opinion the whole advanced section of filters can be scrapped, it adds a lot of maintenance vectors for like a grand total of 0 users. If you need more advanced usage, you can just manually edit the files themselves. The UI you see without the Advanced checkbox is in my opinion good enough!

[1-10] I see in the unreleased version you moved the sync to be first entry. That is good enough and I think will be fixed 😄.

@AkazaRenn
Copy link
Contributor Author

AkazaRenn commented Apr 6, 2025

[2-1] When user first installs the program, they need to run a resync. This needs to be done manually. It is possible to do within app, but the documentation is a bit lacking.

Actually global sync is not set to run by default, it's similar to game sync that will only start to happen when the user has configured so (a filter exists). I think by that time they should have some sense about the need of a resync. It's also mentioned in README.md:
image

[2-2] The game sync configuration is hidden in game context menu. It is okay and a great place, however I found myself confused where it was. I did not test it last time as I thought it was the global filter with a game open. If you know where it is, it works great, but some guidance could be good.

I did note it in README.md, and since it's common practice to put per-game things there (similar to SteamGridDB and CheatDeck), in my view it should be enough.

[2-3] Caused by [2-1]. The sync on game start triggers global and per game syncs. While the game sync is successful, if user did not fix the [2-1], they will be plagued with error message that sync failed (even though game sync succeeded).

A failure is a failure and should get some attention, I think that's fine.

[2-4] After selecting provider we now create destination folder. Its good (up until resync), but changing the destination does not open folder creation popup.

I have warned the user when changing the sync destination, I think I'll leave it to them to figure out.

Overall I would take them as non-issues, maybe they will be improved in a future update. Sorry that I have a pretty low bar for my own works.

@GedasFX
Copy link
Contributor

GedasFX commented Apr 6, 2025

Nah its all good! They are just polish issues and are non-blocking. If its an issue for people, they can create an issue on the repo (if issue creation is enabled 😄)

Other than that, would be all good to go from my side.

Copy link

This pull request is stale as no updates or testing reports have been posted within the last 7 days. Please ensure you are actively recruiting testers or resolving the issues discussed. If you do not remove the stale label or comment, this will be closed in 7 days. Please close this pull request if you need more time to resolve issues so we can keep our repo healthy.

@github-actions github-actions bot added the stale label Apr 14, 2025
@AkazaRenn
Copy link
Contributor Author

Hi @EMERALD0874 , I'm wondering if we are good to get it merged

@EMERALD0874 EMERALD0874 merged commit d764409 into SteamDeckHomebrew:main Apr 14, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in SDH Tracker Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants