Port some fixes/improvements from the retroachievements branch #17594
Conversation
hrydgard
added
the
Code Cleanup
Common/Net/HTTPClient.h
Outdated
| @@ -202,6 +202,7 @@ class Downloader { | |||
| std::shared_ptr<Download> AsyncPostWithCallback( | |||
| const std::string &url, | |||
| const std::string &postData, | |||
| const std::string &postMime, // Use postMime = "application/x-www-form-urlencoded" for standard form-style posts, such as used by retroachievements. | |||
There was a problem hiding this comment.
Note: we already have UrlEncoder which has a getMimeType() that uses this method. Probably multipart/form-data (i.e. from MultipartFormDataEncoder) is supported as well (and would be required for certain types of data, like file uploads with screenshots; some achievement/trophy systems collect these, no idea about that one.)
Feels like this comment isn't needed on both Downloader and Download.
-[Unknown]
There was a problem hiding this comment.
This one only seems to only need simple text data (so far), and it's already being encoded for us by the rcheevos library - so I don't think bringing in MultipartFormDataEncoder here makes sense, since we're just forwarding already-encoded data. Simply adding the postMime parameter here was the easy way to go.
But yes, as you state in the other comment we're approaching DownloadDesc territory.
I'll clean up the comment.
| @@ -458,8 +458,8 @@ int Client::ReadResponseEntity(net::Buffer *readbuf, const std::vector<std::stri | |||
| return 0; | |||
| } | |||
|
|
|||
| Download::Download(const std::string &url, const Path &outfile) | |||
| : progress_(&cancelled_), url_(url), outfile_(outfile) { | |||
| Download::Download(RequestMethod method, const std::string &url, const std::string &postData, const std::string &postMime, const Path &outfile) | |||
There was a problem hiding this comment.
This is probably a case where a DownloadDesc would make more sense, given several of the args now are relevant only to a certain type.
-[Unknown]
Common/System/NativeApp.h
Outdated
| @@ -87,3 +87,6 @@ void NativeShutdownGraphics(); | |||
| void NativeShutdown(); | |||
|
|
|||
| void PostLoadConfig(); | |||
|
|
|||
| // The lambda runs on the next update. | |||
| void NativeRunOnMainThread(std::function<void()> func); | |||
There was a problem hiding this comment.
How have we defined main thread, given Vulkan/OpenGL/etc.? I guess this would be the UI thread (the CPU thread basically) snice it's called from NativeUpdate?
-[Unknown]
There was a problem hiding this comment.
It's the NativeUpdate thread, basically.
I thought I would use this for something that I didn't end up using it for, so I'm just gonna take it out from the PR for now.
Picked out some changes from #17589, nice to get stuff in separately for testing.
And will reduce the size of the retroachievements PR.