-
-
Notifications
You must be signed in to change notification settings - Fork 21.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
Add automatic checking for engine updates #75916
Add automatic checking for engine updates #75916
Conversation
Remember to add a SCons option to allow disabling update checks at build-time, as the Flatpak (and other Linux distribution packages) must not display in-app update notifications by design. It's expected that these packaging systems handle update notifications on their own instead. |
This is looking pretty cool, but i think the check for updates should be only in the project manager and not in the editor as it can be distracting as users could enable it by accident there. Also maybe an option to replace the current version with the newer one so you don't have to uninstall a old version you don't want to use for a more stable one. |
The popup appears on first launch and then only when there is an update available. Other than that the dialog will check for new versions in the background and you won't see it very often.
This is not that simple, because you can't replace an executable that is already running (at least on Windows). The implementation would be a bit involved. For now only version checking will be available; actual update can be added later. |
I think also for privacy reasons the connection to the Internet should be a by default disabled option and/or selectable in the project manager. |
The dialog that appears is the same both for Project Manager and editor. Before making any HTTP requests it will ask you if you want to enable it (unlike AssetLib btw). The setting is disabled by default. |
81fbdd4
to
9a454e0
Compare
9a454e0
to
b9758b6
Compare
e3a2673
to
ed31d9b
Compare
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.
Not sure how much traffic this causes, but I would probably work on a document just listing the interesting versions to minimize it and possibly simplify this PR a bit.
I like being notified about updates. I do not like to be interrupted by dialogs. I think a passive icon should show up in the top right corner (or on the bottom, next to the display of the current version) when there's a new update. |
I agree with Xix, even if I realize that this pop-up has been developed. The mockup is not perfect, but the idea is here. Anyway, it's still a great feature that is really good, even in a pop-up. |
ed31d9b
to
b275197
Compare
I pushed a WIP update. It does more or less what I described in the above comment: If the user is in offline mode, updates are disabled: Also this mostly works with official versions. In custom builds it can fail if it can't determine newer version: I still have a cleanup to do and I plan I plan to add an editor setting. The setting would have 4 values: "Disable Update Checks", "Check Newest Unstable", "Check Newest Stable" and "Check Newest Patch". Also the update checker is only in project manager right now. I will probably add it to EditorNode next to the version label, but it'st going to behave a bit differently to not take space. EDIT: |
The new button looks good, but I'd make the "Current version up-to-date" text gray like the offline mode text so it doesn't take as much attention from the rest of the dialog. Only "update available" and "failed to check for updates" should really stand out from the rest. |
b275197
to
e94e21c
Compare
c9e92e6
to
1275213
Compare
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.
Tested locally, it works as expected.
However, when running a self-compiled development build, you'll always see this at the bottom of the project manager:
The red color can make people think it's an error, leading to false bug reports as update checking for these builds isn't supported by design.
To handle this particular situation (unnumbered dev
/beta
/rc
builds), I suggest displaying a message in gray of the form:
Running a development build.
btw I just had an interesting idea. Say we merge this for some dev version. Since "Newest Stable" is the default setting, when a beta appears, users will still see that there is no update, which might lead to bug reports. What if the default was different for stable versions and unstable versions? This way if you have unstable version, you will get updates until stable, then in stable you will only get info about next stable, unless you again go to unstable build. Does that make sense? |
Stable versions should only inform you about other stable versions by default, while unstable versions should inform you about unstable versions as well (while including stable versions). |
1275213
to
c37ce8b
Compare
By the way, I think the network request should also utilize |
c37ce8b
to
704c311
Compare
704c311
to
e50c969
Compare
Tested briefly, looks good to me! Small nitpicks:
|
That dialog is removed. |
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.
Quick code and doc review.
editor/gui/editor_bottom_panel.cpp
Outdated
#ifdef ALLOW_AUTO_UPDATE | ||
EngineUpdateLabel *eul = memnew(EngineUpdateLabel); | ||
eul->set_v_size_flags(Control::SIZE_SHRINK_CENTER); | ||
eul->enable_compact_mode(); | ||
bottom_hbox->add_child(eul); | ||
#endif |
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.
I wonder if we even need it in the editor. I think having it in the project manager might be good enough.
If a user chooses to stay on an older version for one reason or another, but still wants to keep track of the latest, it makes sense to me to see the update message in the PM but not to have it nag you constantly during development in the editor.
We're also constrained in width in the editor bottom panel, this message being constantly there will quickly become a problem on smaller screens.
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.
This option is for users who don't use Project Manager. But I guess not using it at all is not that common 🤔
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.
Yeah I thought about that too. It's true alternative project managers like godots
are rising in popularity and likely imply skipping the default project manager. But those alternative PMs also do their own update checking logic, so it might not be much of a problem.
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.
What I mean is launching project directly, like directly from the folder or using e.g. VS Code. Personally I open projects with VS Code most of the time.
e50c969
to
49e69fa
Compare
Updated.
I didn't add the update setting to quick settings dialog though. After #89788 I realized the dialog needs some major refactoring, but refrained from doing it, because we won't add that many settings anyway (it's supposed to be small). Now I don't feel like touching it at all. |
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.
Looks great to me!
Thanks! |
if (p_result != OK) { | ||
_set_status(UpdateStatus::ERROR); | ||
_set_message(vformat(TTR("Failed to check for updates. Error: %d."), p_result), theme_cache.error_color); | ||
return; | ||
} | ||
|
||
if (p_response_code != 200) { | ||
_set_status(UpdateStatus::ERROR); | ||
_set_message(vformat(TTR("Failed to check for updates. Response code: %d."), p_response_code), theme_cache.error_color); | ||
return; | ||
} |
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 thought about this, but I wonder if we should future proof in case we ever refactor the website and no longer have https://raw.githubusercontent.com/godotengine/godot-website/master/_data/versions.yml
Maybe we should just print verbose these messages, and not show anything in the PM when requests fail?
So people can keep using e.g. Godot 4.3-stable without being nagged if in a year the website has been ported to a new framework and this file no longer exists.
Otherwise, we need to make sure the file we use for these checks will always be online (but that's hard to future proof).
WDYT @Calinou @coppolaemilio?
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.
This is a valid concern because we might lose control of githubusercontent.com. However it can be debated we can also lose control of godotengine.org.
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.
The update checker also makes many assumptions about the file's structure.
I think the most reliable way to handle it would be adding a dedicated page on the main website, which would provide a raw text dump of version data (it would be hidden/not linked in any page; only for the purpose of accessing it by the engine).
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.
Yeah I was thinking the same.
We currently add .json files to https://godotengine.org/mirrorlist/ (e.g. https://godotengine.org/mirrorlist/4.2.1.stable.json) for each release, but it's not the most convenient to parse.
It should be fairly easy to add a build step for the website to generate e.g. https://godotengine.org/mirrorlist/versions.json that would include all versions with their respective blog post.
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.
Yes, we shouldn't use this file. I would go with our website. We can serve a page like versions.yml in a domain that we control so that if we change servers or things like that we still have the same response from the same address.
This could be versions.godotengine.org/yml
or godotengine.org/versions.yml
. Anything like that will be better.
I can make the changes needed for getting that address up.
Edit: not sure why github didn't show me last messages when I replied, but we could also go with what akien proposes of https://godotengine.org/mirrorlist/versions.json
and/or https://godotengine.org/mirrorlist/versions.yml
as well.
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.
BTW when we implement that, the description I amended in https://github.com/godotengine/godot/pull/75916/files#diff-52f0c165415d19abd2d5586c8a168169a8d39716be86688a9117125409b421f0R878 should be changed to remove explicit mention of GitHub.
Closes godotengine/godot-proposals#1132
The PR adds update checks in a form of a label on the bottom of Project Manager and editor. See #75916 (comment) for more images.
The update checker will check for newer version and tell you when one is available. Clicking the button will open the download page for this specific version. The checker can check updates in 3 modes: "Newest Unstable", which checks for newest official version, "Newest Stable", which checks for newest stable version and "Newest Patch", which checks for the newest patch of the current minor version. "Newest Stable" is the default for stable versions, "Newest Unstable" is default for preview versions.
Note that updates are disabled by default due to network mode being offline by default. There is also a setting to disable updates separately from network mode.
For version checking I'm parsing the version YAML from Godot website. The editor will make many assumptions about structure of the file (such as it's sorted from newest to oldest version), so hopefully it won't change 😅
You can test the PR by modifying
version.py
file. Compiling withallow_automatic_update=false
scons option will disable the dialog completely.