CLOUD: Add new storage connection flow #4860
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sev-
requested changes
Apr 1, 2023
| @@ -0,0 +1,669 @@ | |||
| /* ScummVM - Graphic Adventure Engine | |||
There was a problem hiding this comment.
This file needs to be added to po/POTFILES
common/formats/json.cpp
Outdated
| */ | ||
| JSON::JSON() {} | ||
|
|
||
| char *JSON::getPreparedContents(Common::MemoryWriteStreamDynamic &stream) { |
There was a problem hiding this comment.
I'd recommend renaming this to JSON::untaintContents()
gui/cloudconnectionwizard.cpp
Outdated
| _nextStepButton->setEnabled(serverIsRunning); | ||
|
|
||
| if (_button0) { | ||
| _button0->setLabel(_(serverIsRunning ? "Stop server" : "Run server")); |
There was a problem hiding this comment.
I think these should be marked for translation. e.g. wrap them into _(). Same for the buttons below.
gui/cloudconnectionwizard.cpp
Outdated
| _label1->setLabel(Common::U32String()); | ||
|
|
||
| // get the code entered | ||
| Common::String code = ""; |
There was a problem hiding this comment.
This assignment is redundant: the String constructor initializes it with an empty string already.
gui/cloudconnectionwizard.cpp
Outdated
| _connecting = false; | ||
| delete callback; | ||
| if (_label1) | ||
| _label1->setLabel(_("There is likely a mistake in the code.")); |
There was a problem hiding this comment.
Which code? Please clarify, especially for the translators.
Made the method that prepares the JSON received via network static and moved to Common::JSON, so it could be used not only in CurlJsonRequest.
Local webserver's helper class, Reader, supported simple GET requests and POST form/multipart ones. The code for the latter was a bit specific, and didn't allow handling more generic POST requests. This change adds ReaderMode enum, with RM_HTTP_GENERIC mode, that allows such generic requests to be handled. In Networking::Client class, that uses Reader, readContent() was renamed to readFirstContent(), and corresponds to previously existing POST form/multipart mode. readContent() should now be used for generic POST requests handling.
Just a small helper method to make a Common::SeekableReadStream for local webserver's handlers from Common::String contents.
- allow OPTIONS HTTP method to be handled by local webserver; - add /connect_cloud endpoint; - add new DropboxStorage constructor that works with JSON response instead of requesting that via shortcode; - add CloudManager::connectStorage() overload for this new JSON flow. /connect_cloud endpoint allows cross-origin request (via CORS HTTP headers) from cloud.scummvm.org, so user's browser sends the JSON response directly to ScummVM app, instead of the app requesting that response from the site. This commit is WIP, introducing a new constructor for Dropbox only, and not changing the GUI part at all.
This new version looks for "storage" key in JSON and uses it to determine which storage needs to be connected.
- remove Options widgets of the old wizard; - add CloudConnectionWizard dialog; - remove old widgets and add new ones in the layouts; - update local webserver to allow passing a callback that needs to be called if storage was connected via /connect_cloud.
- rename JSON::getPreparedContents() to JSON::untaintContents(); - minor changes to ConnectCloudClientHandler and CloudConnectionWizard.
- regenerate themes .zips; - minor cleanup.
Tkachov
force-pushed
the
cloud-2023-change
branch
from
f779ce8 to
733be3b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes how users would connect Cloud storages in ScummVM. Instead of the old flow, where user would enter a short code to retrieve data stored on cloud.scummvm.org, now there is a GUI wizard that allows "quick" and "manual" modes. In both modes, cloud.scummvm.org doesn't store the data anymore (see the corresponding PR for scummvm-sites: #21).
In quick mode, user is asked to enable Local Webserver, so their browser could send an AJAX request to /connect_cloud.
In manual mode, the data of that request is displayed in browser, and user copies it into ScummVM.
I've tried to split the changes into small logically finished commits for easier review. The GUI change is quite big though.