feat: Allow filtering based on Store/root type #329
Conversation
…eature/filter-root-type # Conflicts: # src/path.rs # src/scan.rs
Signed-off-by: kekkon <[email protected]>
Signed-off-by: kekkon <[email protected]>
Signed-off-by: kekkon <[email protected]>
Signed-off-by: kekkon <[email protected]>
| impl std::hash::Hash for PathMetadata { | ||
| fn hash<H: std::hash::Hasher>(&self, state: &mut H) { | ||
| self.is_case_sensitive.hash(state); | ||
| self.store.hash(state); | ||
| } | ||
| } |
There was a problem hiding this comment.
We should be able to derive this.
| pub fn is_case_sensitive(&self) -> bool { | ||
| self.is_case_sensitive | ||
| } | ||
|
|
||
| #[allow(dead_code)] | ||
| pub fn store(&self) -> Option<Store> { | ||
| self.store | ||
| } |
There was a problem hiding this comment.
Instead of having methods for these, we can just make the fields pub.
There was a problem hiding this comment.
is_case_sensitive is from your code, just to be clear. But I have no issue doing that refactor.
There was a problem hiding this comment.
Not sure what you mean; this is part of the new PathMetadata struct introduced in this PR. Previously, this was just a loose bool in fn parse_paths.
| /// Optional. The store the file originated from. | ||
| pub store: Option<Store>, |
There was a problem hiding this comment.
Technically, this should be a HashSet<Store>, since one path could be found in multiple roots. Let's say someone has their Epic and Amazon games installed together in D:\Games, so they make two roots pointing there. I think the current code would insert two ScannedFiles that are the same except for the store, and that might lead to a duplicate path in the results. ScannedFiles are tracked by their overall hash, not just the hash of the path (I should refactor that eventually 😅).
I see two options:
- Refactor
fn scan_game_for_backupto change the local variablefound_filesfrom aHashSet<ScannedFile>to aHashMap<StrictPath, ScannedFile>. If a path is already in the map, then just add the store to it. We'd still convert it into aHashSetbefore we return it as part of theScanInfo, to minimize the impact for now. - Since we're not doing any file-level filtering anyway, we could put the
HashSet<Store>inScanInfodirectly. Infn scan_game_for_backup, as we check each path, we can just build up a hash set of stores with matches and then put that in the final results.
The first option gives us more info if we want to use it in the future. The second option keeps things simpler and just gives us what we need right now. I'm fine with either one you feel like doing, although I'd probably vote for the second option if I had to pick.
|
@kekonn Would you like any help finishing this up? I'd like to include it in the upcoming release. |
|
I probably won't have time to have a look at this before end of May or early June, sorry. If you want to include it, go ahead and finish it up. |
|
Hmm, after testing this out, it might be more difficult than I thought to get it working 100%. Right now, there's an issue with paths that aren't specific to the root. For example, one of the paths for 9 Years of Shadow is Maybe we could check if the save path is inside of the root's path, and if so, then add the root's store to the list, but that wouldn't handle cases like a Heroic root where the Wine prefixes are stored outside of the Heroic folder itself. Plus, I think a typical user would expect the filter to show them "games installed from Steam" (which would include the AppData path), not "saves inside of a Steam root" (which would exclude the AppData path), even though the latter makes sense in terms of how Ludusavi works. We could instead check if a game is installed in a particular root, but then you can also have saves from games that are no longer installed, and those saves may or may not even be in the root where the game used to be installed. In my case, 9 Years of Shadow is no longer installed. I'm not sure how to solve that in a way that's both useful and easy to understand. In the situations where you'd like to use this filter, what problem are you typically trying to solve? For example, do you want to make separate backups for different stores, or are you trying to find old saves from a store you don't use anymore? Maybe we can find another way to solve the same problems. |
|
The main reason for this is that I want to exlude steam from the preview most of the time. Almost all steam games have cloud sync anyway, so I don't need ludusavi for those (some exceptions apply). So I don't want to delete the store in the settings, but most of the time I don't want my steam games listed in the preview (because that makes the list several orders of magnitude larger). |
|
Gotcha. It sounds like #44 would be a better solution for your use case. That way, you could filter based on whether the game supports Steam Cloud, regardless of where the saves are located. You could also individually disable some games in Ludusavi and then disable the "show deselected games" option, but perhaps that's not as convenient when new games show up in the list enabled by default. Of course, it gets trickier if you use multiple stores. For example, you might enable a filter to hide games that support Steam Cloud, but maybe you own one of those games on GOG where it doesn't support cloud saves. Not sure how much of a concern that would be for your use case. |
|
I've started tracking cloud support in the manifest: mtkennerly/ludusavi-manifest@adf4da5 . I haven't backfilled it for all games yet, but it's enough that I can start working on #44. I'll close this PR since I think #44 is the best way forward, but thank you again for prototyping this approach :) |
Part 2 of #225 finally here.
Still in draft because somehow it is detecting Lutris as OtherWine.
Any assistance with perhaps adding the relevant tests would be nice. Unless you think this doesn't require any tests?