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

Add git info to title screen & gameplay stats #4053

Merged
merged 8 commits into from
Aug 23, 2024

Conversation

Pepe20129
Copy link
Contributor

@Pepe20129 Pepe20129 commented Apr 12, 2024

The branch shows up if it the current commit doesn't have a tag (see here).
In the title screen, the commit hash will appear if the oot debug mode is enabled; in the gameplay stats, it'll appear if Show Debug Info is enabled.
image
image

Build Artifacts

@Archez
Copy link
Contributor

Archez commented Apr 12, 2024

I think rather than checking if the branch starts with develop we should instead check if the current commit has a tag ref, if it doesn't then it's essentially a non-release build.

This would make it so that even nightly builds or builds from the develop-rando branch would display the git details too. Especially since develop still shows the version as the "latest" named release.

@Pepe20129
Copy link
Contributor Author

I don't think that'd work as the builds get made before the commit is tagged.

@Archez
Copy link
Contributor

Archez commented Apr 12, 2024

I believe tagging in github causes another round of builds, which are the ones that get used for releases

There are two actions runs, one for the merge commit, and one for the tag
image

@Pepe20129
Copy link
Contributor Author

Pepe20129 commented Apr 12, 2024

I think rather than checking if the branch starts with develop we should instead check if the current commit has a tag ref, if it doesn't then it's essentially a non-release build.

Changed it.

CMakeLists.txt Outdated
@@ -9,6 +9,37 @@ project(Ship VERSION 8.0.5 LANGUAGES C CXX)
set(PROJECT_BUILD_NAME "MacReady Foxtrot" CACHE STRING "")
Copy link
Contributor

Choose a reason for hiding this comment

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

i've been hoping to be able to remove the hardcoded version number and version name, maybe it'd be possible to pull these from the tag too (i've generally made sure that tags have the named version as the description)

the question then becomes what do we do for builds that aren't releases - we'll still need to set a version number, maybe for the name we could just have "MacReady" for the named release branch and then "Nightly" or "Develop" for the main dev branch

in general i'd be very happy to see the extra info make it into the builds, i just want to make sure we think through all of the use cases we want to support

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the question then becomes what do we do for builds that aren't releases - we'll still need to set a version number, maybe for the name we could just have "MacReady" for the named release branch and then "Nightly" or "Develop" for the main dev branch

Do you mean for it to be just MacReady/Develop with no second part?
If so we would still need to have MacReady somewhere in there as I see no way of getting it from the branch reliably.

Copy link
Contributor

Choose a reason for hiding this comment

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

so i'm not quite sure what we'd want to set those values to when not on a tagged version, but i'm thinking we could just not show them on the logo screen when not on a tagged version

i'm still really hoping we can figure out a way to use the git tag description to set the version name for tagged versions instead of needing to hardcode it, it seems we can get that by using

git cat-file tag <tag_version> | sed '1,/^$/d'

but in my quick poking around i wasn't able to get that working in cmake

basically i'm hoping we can do

project(Ship VERSION "${GIT_COMMIT_TAG}" LANGUAGES C CXX)
set(PROJECT_BUILD_NAME "${GIT_TAG_DESCRIPTION}" CACHE STRING "")

when we have a tag, and then maybe just do

project(Ship VERSION 0.0.0 LANGUAGES C CXX)
set(PROJECT_BUILD_NAME "Unreleased" CACHE STRING "")

when we don't have a tag?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit hesitant on 0.0.0 since that version info is what gets stored into the game otrs and is used for enforcing the regenerate flow. I know what we have isn't great either since the develop branch wont complain about an otr from the current release.

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Approving as-is.

I know there was still conversation about potentially pulling out tag/release name info out of source entirely, but the original goal of this PR was to make tracing what commit a nightly build actually came from which would be good to merge in as testing for rando V3 increases. We can always open a separate issue to track the conversation about tag/release name in source.

@briaguya-ai
Copy link
Contributor

I know there was still conversation about potentially pulling out tag/release name info out of source entirely,

100% agree that can be a separate PR

could we make it so we only show the version number and name on tagged builds? i expect people would say "i'm on macready foxtrot develop rando" if we show both on the title screen when reporting from test builds - i think it'd be clearer to only show the branch name and commit sha instead

@Pepe20129
Copy link
Contributor Author

could we make it so we only show the version number and name on tagged builds? i expect people would say "i'm on macready foxtrot develop rando" if we show both on the title screen when reporting from test builds - i think it'd be clearer to only show the branch name and commit sha instead

I don't think that's a good idea as with the version and name there we can also quickly know if a build is severely outdated in the future.

@briaguya-ai
Copy link
Contributor

briaguya-ai commented Aug 23, 2024

quickly know if a build is severely outdated in the future

wouldn't the date do that?

@Pepe20129
Copy link
Contributor Author

I forgot about the date.

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

:shipit:

@Archez Archez merged commit e07fc59 into HarbourMasters:develop Aug 23, 2024
5 checks passed
@Pepe20129 Pepe20129 deleted the git_info branch August 23, 2024 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants