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

DolphinQt: Add function to import/export userdir from/to a zip file. #10711

Closed

Conversation

AdmiralCurtiss
Copy link
Contributor

Announced back in #10526 and reminded of by #10708, here's the userdir exporting feature from Android but in Qt.

This runs into the same small problem as my settings reset where some GUI elements don't understand how to react to config changes of values that have previously never changed within a session. Otherwise this should 'just work'.

@JosJuice Please test if the zips generated here are compatible with the zips from Android and vice-versa. Maybe we can also get Android to use this code to avoid duplication in Java?

@JosJuice
Copy link
Member

Maybe we can also get Android to use this code to avoid duplication in Java?

That won't happen unless we can get minizip to accept a file descriptor instead of a path. Scoped storage support is a necessity for this feature on Android.

I will try to test this cross-OS when I get time.

@AdmiralCurtiss AdmiralCurtiss force-pushed the import-export-userdir branch from 18a2db0 to b8d8661 Compare May 31, 2022 19:04
@iwubcode
Copy link
Contributor

@AdmiralCurtiss - just curious, what is the use case for this?

I haven't looked at the details yet but for instance, if this is a support feature for devs to support users when they say "show me all your settings", then it doesn't need to include things like the Wii nand, the load folder, screenshots, etc. If this is for users to backup files, then it shouldn't iterate over the directories directly because that's not likely what is intended. It should go to each of the directories in the paths tab and then also do any directories not covered by the paths tab.

If it's the former, a more concise method to copy it to the clipboard for easy pasting might be better. If it's the latter, well, I personally don't see a lot of value in that.

Just my 2cents.

@mbc07
Copy link
Member

mbc07 commented Jun 2, 2022

what is the use case for this?

Probably not the main motive behind this PR but exporting user data from Android to manage on Qt and vice-versa seems like a potential user case. Useful especially for handling settings not directly exposed on Android's GUI at the moment...

@AdmiralCurtiss
Copy link
Contributor Author

@iwubcode See the discussion over in #10526, it is indeed the use case of making a backup to restore on eg. a different system, be it another PC or your phone. But you do make a good point that this wouldn't catch files/directories that the user set to a place outside the userdir. How do we want to handle that?

Copy link
Member

@lioncash lioncash left a comment

Choose a reason for hiding this comment

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

LGTM, just one minor nit

Source/Core/UICommon/ImportExportUserdir.h Outdated Show resolved Hide resolved
@AdmiralCurtiss AdmiralCurtiss force-pushed the import-export-userdir branch from c16d28e to ddbf9ef Compare June 3, 2022 22:58
@iwubcode
Copy link
Contributor

iwubcode commented Jun 5, 2022

@AdmiralCurtiss - my thought would be we go through the configurable path parameters and copy each of those into a local folder in the zip. Then when we import, we don't unzip directly, we import into the directory that is configured.

Maybe a future PR but it'd be nice if the individual path sections were configurable (ex: I could select I don't want the Wii nand to be included in the zip, or imported from the zip).

@AdmiralCurtiss
Copy link
Contributor Author

Overwriting files outside of the Dolphin userdir when importing just because they happen to be configured in the current configuration sounds quite dangerous to me.

Arguably we could 'localize' the directory by (on export) packing from the configured paths instead of the default in-userdir paths, and then when importing unpacking into the default paths instead (and update the config accordingly to point at the default dirs again), that seems safer. Though I wonder if that's clear to users.

@iwubcode
Copy link
Contributor

iwubcode commented Jun 5, 2022

Overwriting files outside of the Dolphin userdir when importing just because they happen to be configured in the current configuration sounds quite dangerous to me.

I understand your point but I mean, this approach itself is dangerous. They are blowing away whatever they have, regardless of where it is located.

While the import approach you mention makes sense from a safety perspective, it seems like it'd be unexpected. Also overwriting whatever is in the local paths seems dangerous too when that might be unintended.

I'm not in complete disagreement, I'm just trying to give some alternative perspective.

Maybe a "diff" view of what will be added, along with the changes and a "ARE YOU SURE" would be nice. Though that would bloat this PR quite a bit...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants