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
Fix CloudSync on Windows, enable by default #16475
base: master
Are you sure you want to change the base?
Conversation
Hey, just checking in on this. Is there anything you need from our end? |
Hi, thanks for checking in. I didn't get to finishing the tests yet, I got stuck at trying to enable OpenSSL for Windows UWP builds. I ran out of time, and put the project on hold for the time being. They have a guide for compilation here: https://github.com/openssl/openssl/blob/master/NOTES-WINDOWS.md I remember getting stuck at the end, something failed to link (or compile) within OpenSSL itself. Then, there's the matter of how to integrate that into the Visual Studio project in such a way that the "steps" of getting it compiled can be defined and built by the retroarch project itself. In order to enable SSL for UWP, one would need a way of automating the build of OpenSSL while building retroarch. Windows is not particularly my cup of tea, so this is a bit slower. Although, enabling UWP SSL would also bring Cloud Sync to Xbox platforms, if I'm not mistaken. Maybe, if anyone has an Xbox with UWP, would you mind testing it there as-is, without SSL? |
Ah, okay, I don't have any xbox myself, but I'll see if I can get some eyes on this. Thanks for the update :) |
What if we excluded this for now for UWP builds so that we can at least merge this in for the non-UWP versions? Would that be a suitable compromise? Also, I take it the current implementation and how this works is:
That is the way I'd like to see it implemented at least. |
That could work, it seems that non-UWP builds work normally. In order to avoid confusion, I would've suggested that all enabling for a platform to be done at once. However, it seems that time is tight, and this feature works quite well as-is. However, there's still one more thing we could do: Enable this for UWP as well (and Android), but define a flag which, if missing, changes the text associated with Cloud Sync to mention that it's experimental and unencrypted. This would be needed anyway, since there are some platforms (such as the PS2 and Wii) where the "weight" of an SSL library would probably outweigh the benefits it brings.
Yes, you have to manually enable and configure it with credentials and a WebDav server, from the "Saving" menu. The reason for concern about SSL is particularly this logging in, which over unencrypted connections makes me feel uneasy. However, if the user is willing to take the risk, or is providing encryption some other way for the time being (like I am on Android, with a VPN), we might as well let them enable it. I was thinking of:
For example, a build where only HAVE_CLOUDSYNC is enabled would show the feature like We could probably drop this "tested" flag at some point. Given that this feature actually modifies user data, I would suggest to take this precaution, in order to avoid chaos and data corruption. Would that be alright? |
Fix Windows CloudSync and enable by default
Related Issues
#6875
Related Pull Requests
#6875
Reviewers
None yet (will edit when ready)
Pending to test