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

Suggest to update only if the latest release is available for the current platform #26930

Merged
merged 3 commits into from Feb 10, 2024

Conversation

Susko3
Copy link
Member

@Susko3 Susko3 commented Feb 1, 2024

We've had few releases lately where mobile isn't working / is crashing, so the mobile release was pulled. Unfortunately, the game would still send an update notification, leading to many people opening issues about the update not being available for their platform.

This PR changes SimpleUpdateManager to only show the update notification if the latest release actually has an update for the current platform (i.e. it wasn't pulled).

For this to work properly, the files need to removed from the release. This is mostly relevant for iOS, as the osu.iOS.ipa is available to download even if the update has not been pushed to testflight.

@bdach
Copy link
Collaborator

bdach commented Feb 10, 2024

Tested this by downloading the json of the github response to disk, spinning up a local http server via

$ python3 -m http.server

and then

diff --git a/osu.Game/Updater/SimpleUpdateManager.cs b/osu.Game/Updater/SimpleUpdateManager.cs
index 0f9d5b929f..68f5a50f14 100644
--- a/osu.Game/Updater/SimpleUpdateManager.cs
+++ b/osu.Game/Updater/SimpleUpdateManager.cs
@@ -36,7 +36,10 @@ protected override async Task<bool> PerformUpdateCheck()
         {
             try
             {
-                var releases = new OsuJsonWebRequest<GitHubRelease>("https://api.github.com/repos/ppy/osu/releases/latest");
+                var releases = new OsuJsonWebRequest<GitHubRelease>("http://localhost:8000/latest.json")
+                {
+                    AllowInsecureRequests = true
+                };
 
                 await releases.PerformAsync().ConfigureAwait(false);
 
diff --git a/osu.Game/Updater/UpdateManager.cs b/osu.Game/Updater/UpdateManager.cs
index 190748137a..05cc48560d 100644
--- a/osu.Game/Updater/UpdateManager.cs
+++ b/osu.Game/Updater/UpdateManager.cs
@@ -22,7 +22,7 @@ public partial class UpdateManager : CompositeDrawable
         /// <summary>
         /// Whether this UpdateManager should be or is capable of checking for updates.
         /// </summary>
-        public bool CanCheckForUpdate => game.IsDeployedBuild &&
+        public bool CanCheckForUpdate => //game.IsDeployedBuild &&
                                          // only implementations will actually check for updates.
                                          GetType() != typeof(UpdateManager);
 

Seems to work as advertised in general. I can get behind the idea at least.

// iOS releases are available via testflight. this link seems to work well enough for now.
// see https://stackoverflow.com/a/32960501
return "itms-beta://beta.itunes.apple.com/v1/app/1447765923";
if (release.Assets?.Exists(f => f.Name.EndsWith(".ipa", StringComparison.Ordinal)) == true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure about relying on the existence of the .ipa. It's more likely to go away in the future than to stay.

See: #23218 (comment)

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Hmm, I think it's okay to rely on this for the time being. At least it gives us a way to manage availability.

@bdach bdach requested a review from peppy February 10, 2024 08:10
Copy link
Sponsor Member

@peppy peppy left a comment

Choose a reason for hiding this comment

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

Let's get this in. Heavily leaning into spaceman's testing of this.

@peppy peppy merged commit 32bfd87 into ppy:master Feb 10, 2024
13 of 17 checks passed
@Susko3 Susko3 deleted the suggest-update-only-if-available branch February 10, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants