-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Refactor & modularize code in Updater.cs #6670
base: master
Are you sure you want to change the base?
Conversation
Cache httpClient and remove ConstructHttpClient()
Replace `WebClient` with `HttpClient`, plus updating DoUpdateWithSingleThreadWorker and DoUpdateWithMultipleThreads
This is just for testing the update functionality!
Since Memory<byte> is initialized with a fixed size, `.Slice(0, readSize)` and `.Slice(0, bytesRead)` are necessary to ensure that only the part of the buffer that contains data is written to the file stream. So IDE0057 is not appropriate in this context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a quick review on code side. I think you should reduce the amount of sub-files because a lot of them only contains one method, you should store everything in classes instead.
private static long _buildSize; | ||
private const int ConnectionCount = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static long _buildSize; | |
private const int ConnectionCount = 4; | |
private const int ConnectionCount = 4; | |
private static long _buildSize; |
int completedRequests = 0; | ||
int[] progressPercentage = new int[ConnectionCount]; | ||
List<byte[]> chunkDataList = new List<byte[]>(new byte[ConnectionCount][]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Buffer.BlockCopy(chunk, 0, data, (int)position, chunk.Length); | ||
position += chunk.Length; | ||
} | ||
return data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return data; | |
return data; |
{ | ||
internal static partial class Updater | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await ContentDialogHelper.CreateWarningDialog( | ||
LocaleManager.Instance[LocaleKeys.DialogUpdaterConvertFailedMessage], | ||
LocaleManager.Instance[LocaleKeys.DialogUpdaterCancelUpdateMessage]); | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return null; | |
return null; |
LocaleManager.Instance[LocaleKeys.DialogUpdaterConvertFailedGithubMessage], | ||
LocaleManager.Instance[LocaleKeys.DialogUpdaterCancelUpdateMessage]); | ||
_running = false; | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return false; | |
return false; |
if (asset.Name.StartsWith("ryujinx") && asset.Name.EndsWith(_platformExt) && asset.State == "uploaded") | ||
{ | ||
_buildUrl = asset.BrowserDownloadUrl; | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return true; | |
return true; |
} | ||
|
||
_running = false; | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return false; | |
return false; |
await ContentDialogHelper.CreateErrorDialog( | ||
LocaleManager.Instance[LocaleKeys.DialogUpdaterFailedToGetVersionMessage]); | ||
_running = false; | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return false; | |
return false; |
{ | ||
request.Headers.Range = range; | ||
} | ||
return await _httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return await _httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); | |
return await _httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead); |
WebClient
toHttpClient
async
andawait
instead ofResult
Please test by updating the app when prompted to make sure there's no regression. I only tested on Windows, but you can test again to make sure.
Also please check if I do something wrong or miss something 🙏🙏.