-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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 a "version" project setting and use it in new export presets #35555
Add a "version" project setting and use it in new export presets #35555
Conversation
27a03c6
to
76f8929
Compare
76f8929
to
fa0de0e
Compare
@Calinou Is this still desired? If so, this needs to be rebased against the latest master branch. |
fa0de0e
to
0e247ff
Compare
Rebased. In the Windows export code, I also added version conversion logic required for rcedit to work if your version string does not follow the format |
0e247ff
to
0729050
Compare
doc/classes/ProjectSettings.xml
Outdated
@@ -228,6 +228,10 @@ | |||
<member name="application/config/use_custom_user_dir" type="bool" setter="" getter="" default="false"> | |||
If [code]true[/code], the project will save user data to its own user directory (see [member application/config/custom_user_dir_name]). This setting is only effective on desktop platforms. A name must be set in the [member application/config/custom_user_dir_name] setting for this to take effect. If [code]false[/code], the project will save user data to [code](OS user data directory)/Godot/app_userdata/(project name)[/code]. | |||
</member> | |||
<member name="application/config/version" type="String" setter="" getter="" default=""1.0.0""> | |||
The project's human-readable version identifier. This should always be set to a non-empty string, as some exporters rely on this value being defined. | |||
[b]Note:[/b] When exporting for iOS, this version number must be incremented every time a new version is submitted to the App Store, even if the previous version was rejected by Apple. For instance, version [code]1.2.3[/code] can be incremented to [code]1.2.4[/code], [code]1.3.0[/code] or [code]2.0.0[/code]. |
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.
That's not entirely correct.
If submitted application gets rejected it's enough to change/increase CFBundleVersion
. This value doesn't get displayed to user on AppStore, CFBundleShortVersionString
does.
In most cases CFBundleVersion
is used as build number, so displaying application version like CFBundleShortVersionString (CFBundleVersion)
results in something like 1.0.0 (2520)
. And on TestFlight builds are grouped by CFBundleShortVersionString
value.
} else if (lines[i].find("$version") != -1) { | ||
strnew += lines[i].replace("$version", p_preset->get("application/version")) + "\n"; | ||
strnew += lines[i].replace("$version", GLOBAL_GET("application/config/version")) + "\n"; |
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 think only $short_version
(CFBundleShortVersionString
) should be replaced with application/config/version
.
$version
(CFBundleVersion
) which is internal application version would be better to stay as export setting.
In some cases if CFBundleShortVersionString
get's changed (which it should after every release) there might be no reason to change CFBundleVersion
so it can stay the same. But it should be changed in case of AppStore rejection, so the version displayed in AppStore does not get increased without a reason.
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.
Seems like the ideal case here is to have version
kept in the export setting, while having an empty value default to the short_version
one.
This could be useful to check if other network peers are running the same project version for compatibility purposes as well. |
Kind of reminds me of godotengine/godot-proposals#228 As for easy retrieving of the version string, this is doable with a plugin, as I commented here: godotengine/godot-proposals#372 (comment) |
@@ -1649,7 +1649,6 @@ void EditorExportPlatformAndroid::get_export_options(List<ExportOption> *r_optio | |||
r_options->push_back(ExportOption(PropertyInfo(Variant::BOOL, "one_click_deploy/clear_previous_install"), false)); | |||
|
|||
r_options->push_back(ExportOption(PropertyInfo(Variant::INT, "version/code", PROPERTY_HINT_RANGE, "1,4096,1,or_greater"), 1)); | |||
r_options->push_back(ExportOption(PropertyInfo(Variant::STRING, "version/name"), "1.0")); |
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.
@Calinou I'm wondering if it would be better to keep this property and have it default to application/config/version
.
The advantage of keeping is that it allows you to customize the version string per export preset (for example, you could have one Android export preset that support VR and another one that doesn't for the same game, and this would allow to update their version name accordingly).
Needs a rebase and to address the feedback, but otherwise we should include this for 4.0! |
0729050
to
557bcc0
Compare
I've revamped this PR in a way that preserves the ability to override versions on a per-preset basis. See OP for an updated description. Note that I haven't tested these changes yet. |
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 changes look good!
Documentation needs fixing the order :) |
557bcc0
to
101c911
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.
Seems fine.
101c911
to
f033962
Compare
f033962
to
eb16ffe
Compare
This makes it easy to retrieve the project version at runtime for display purposes, while simplifying the export preset configuration. You can now leave the version empty unless you need to override it on a per-preset basis. Since export presets save the values of default values to the `export_presets.cfg` file, this change only affects export presets created after this commit was merged.
eb16ffe
to
ad4480b
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.
Seems OK.
I'm slightly concerned about the UX since we now have a mostly mandatory project setting which is empty by default, but which can be overridden in presets - but the placeholders advise to leave it empty, without clarifying whether there is a valid fallback for them to use.
Let's see how it goes, we could add more validation code if needed.
Thanks! |
I thought I had set its default to |
This makes it easy to retrieve the project version at runtime for display purposes, while simplifying the export preset configuration. You can now leave the version empty unless you need to override it on a per-preset basis.
Since export presets save the values of default values to the
export_presets.cfg
file, this change only affects export presets created after this commit was merged.
The Android, iOS and macOS versions requirement have also been documented. See this Stack Overflow question.
August 2021 patch