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

Library folder sync #83

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

Library folder sync #83

wants to merge 21 commits into from

Conversation

AkazaRenn
Copy link
Contributor

@AkazaRenn AkazaRenn commented Jun 19, 2024

For #45

Sorry that it's not using your implementation of configs because I found it doesn't work very well with dictionaries, here's what it looks like in the new settings I created for this change:

{
    "Documents": {
        "enabled": false,
        "destination": "Documents"
    },
    "Music": {
        "enabled": false,
        "destination": "Music"
    },
    "Pictures": {
        "enabled": false,
        "destination": "Pictures"
    },
    "Videos": {
        "enabled": true,
        "destination": "Videos"
    }
}

A new section will be added to the path config page like this:
Screenshot 2024-06-19 214102

@GedasFX There are some minor stupid problems with the settings UI, I'm a frontend noob so I could really use some help if you happen to have some time. Here are the problems:

  1. The most deadly one, keyboard would cover the text box we are inputting to We now enter the sync path from a dialog
  2. Navigation on these three elements can only be done by up/down not by left/right, even if they are visually on the same row They can now be navigated by left/right keys
  3. By adding overflow, I can scroll down to the end of this page, but going back to the top becomes impossible because the focus will jump to the search bar. Luckily we can just exit this page and come back, so this one is relatively minor I adjusted the page padding, now we can access all elements in the page without exiting, it's just the top title that may not be visible if you go to the bottom and go back top, since that one is not select-able.

@AkazaRenn AkazaRenn marked this pull request as ready for review June 20, 2024 01:33
@GedasFX
Copy link
Owner

GedasFX commented Jun 20, 2024

Personally, I never really liked how I did the plugin properties; the plugin was made quite early, and config was just jank enough but worked 😄

I know decky now has some built-in config manager (which looks eerily familiar to your thing here?), so I wouldn't mind migrating all to that. Having 2 separate sources of truth for config will very much confuse users, and we should avoid that.

@AkazaRenn
Copy link
Contributor Author

@GedasFX sure I can look into combining them into one config.json, or maybe that could be in a separate PR to be cleaner?

@GedasFX
Copy link
Owner

GedasFX commented Jun 20, 2024

@AkazaRenn hmmm for me it wouldn't matter too much, as ultimately I will do it as part of one release due to leisurely review times on the decky store. But it would make it simpler to do that first, then add this feature.

@AkazaRenn
Copy link
Contributor Author

@GedasFX it's actually easier than I had expected, I was able to reuse most of the codes and now we have one single config.json like this:

{
    "destination_directory": "decky111-cloud-savevvv",
    "experimental_menu": true,
    "toast_auto_sync": true,
    "bisync_enabled": false,
    "log_level": "INFO",
    "sync_on_game_exit": false,
    "library_sync": {
        "Documents": {
            "enabled": false,
            "bisync": true,
            "destination": "deck-22libraries/Documentsggggggg"
        },
        "Music": {
            "enabled": false,
            "bisync": true,
            "destination": "deck-33libraries/Music"
        },
        "Pictures": {
            "enabled": false,
            "bisync": false,
            "destination": "deck44-libraries/Picturesffff"
        },
        "Videos": {
            "enabled": false,
            "bisync": false,
            "destination": "deck-libra55ries/Videosfvfgghhhhhhhhh"
        }
    }
}

@AkazaRenn
Copy link
Contributor Author

Screenshot 2024-06-21 210631
Now destination input will be handled by a dialog

@AkazaRenn
Copy link
Contributor Author

There should be no other issues that came to my mind. Please let me know if you want something else to be changed.

@GedasFX
Copy link
Owner

GedasFX commented Jul 29, 2024

The bisync PR is taking absolutely forever right now... Terribly sorry but its crazy how it is... I haven't forgotten about this PR ^^

@AkazaRenn
Copy link
Contributor Author

@GedasFX no worries! It's totally understandable, my plugin is pending pipeline and reviews as well 😂

@jkluch
Copy link

jkluch commented Aug 9, 2024

@AkazaRenn @GedasFX I just started playing a game that doesn't have cloud sync and saw this plugin and PR. I might be wrong but my understanding looking over this plugin is the full path of a game save directory gets synced over to the folder specified by the destination_directory

If that's right, what are your thoughts on allowing each include path the ability to specify it's own destination path? This is very similar to what is being proposed here in this PR by the looks of it, but this PR is written as a hard coded special case.

I figure the destination_directory could be used as a default_root_dir.
When adding a new includes directory there would be a source and destination field.
The destination field would be defaulted to default_root_dir + source_path but be a value you can overwrite with something else.
This way the default behavior would align with existing behavior but give the user more flexible options.. solving requests like #45 but more generally.

My use-case is an existing dropbox directory I've been using for years for syncing game saves which I setup in this format: GameSaves/GameName/<folder_or_file_to_sync>

The way this plugin is currently designed I'd have to move all these folders to match up with steamdeck paths and all my existing save file syncs on existing PCs would need to be re-done. The current implementation while simple also creates a lot of unnecessary folder depth.

Thoughts?

@AkazaRenn
Copy link
Contributor Author

AkazaRenn commented Aug 9, 2024

@jkluch there's too little benefits to allow a custom destination for every folder synced, let alone this would make bidirectional syncing way more complicated. I would not consider implementing it.


exec "$PLUGIN_DIR/rclone" "--config" "$CONFIG_DIR/rclone.conf" "$@"
exec "$BIN_DIR/rclone" "--config" "$CONFIG_DIR/rclone.conf" "$@"
Copy link
Contributor Author

@AkazaRenn AkazaRenn Aug 24, 2024

Choose a reason for hiding this comment

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

Not sure why but my own build uses Decky Cloud Save for the folder name, so I changed it to a more generic solution

@@ -0,0 +1,82 @@
name: Build Plugin
Copy link
Contributor Author

@AkazaRenn AkazaRenn Aug 24, 2024

Choose a reason for hiding this comment

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

This action builds and creates a prerelease with the tag using a timestamp like 20240824153302 (when the action runs), it would be triggered on every push on the main branch.
You can check my fork as an example https://github.com/AkazaRenn/decky-cloud-save/tags

@AkazaRenn AkazaRenn closed this Aug 25, 2024
@AkazaRenn AkazaRenn deleted the lib-sync branch August 25, 2024 04:05
@AkazaRenn
Copy link
Contributor Author

Oops, didn't mean that

@AkazaRenn
Copy link
Contributor Author

AkazaRenn commented Sep 24, 2024

Hey @GedasFX , since 1.4.2 has been released I'm thinking if we could get this PR merged as a new release candidate. If you need any more changes just let me know I can take care of it. Once it's merged I can make another PR for the new Decky API introduced in v3.0.0

@AkazaRenn
Copy link
Contributor Author

AkazaRenn commented Nov 16, 2024

I want to add another option that allows library sync to always run on regardless whether the game supports Steam Cloud or not, since this is for syncing screenshots and recordings.
Actually it just should always run. I'll change the code in that way.

@AkazaRenn AkazaRenn closed this Feb 17, 2025
@AkazaRenn AkazaRenn reopened this Feb 17, 2025
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.

3 participants