-
Notifications
You must be signed in to change notification settings - Fork 79
[GITHUB] Add weekly pre-releases workflow for Generals and GeneralsMD #929
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
base: main
Are you sure you want to change the base?
Conversation
What does this change produce then? |
If these are our 'official' pre-releases then they need to be properly versions. |
I can adjust for a semVer schema if it looks better. |
It will produce zip files weekly with the main executables for generals, worldbuilder and w3dview both for both games. |
It's not about looking better, it's about having traceability when somebody reports an issue or bug. |
d248576
to
b668cf2
Compare
@xezon @roossienb I’ve just updated the pull request to use semantic versioning. The version is now based on When the project is ready for a stable release, we just need to update the version file to I ran the workflow on my fork and everything went smoothly: ![]() ![]() |
In a discussion with @Mauller he told me that they did nightly builds in which they used the latest commit number as the build number in the application. This makes it easy to track back. |
It’s fine to change the logic to whatever is needed. But just need to decide what will be the pattern. All approaches have its own pros and cons. Using latest commit it’s a good approach but we can get more than one commit in a day, and this would be a problem to track anyway. What about to mix the approaches like |
I think most important is that the version is also shown in-game (so when you open the options menu, in the bottom). |
I totally agree with you. But I don’t have enough knowledge in C language. Can you guys help me to do that? |
I don't think it should have a proper version number tag, only official releases should get a version tag and otherwise should show the commit and if the tree was dirty when it was built. Vanilla Conquer uses a GitWatcher cmake module to populate the variables used to configure this file which is then referred to when configuring the version. |
09d1dd0
to
a801bfb
Compare
I think I’m getting close to add the build version to game. I believe in a few days I will be able to update this pr with the modifications. |
@xezon @roossienb @OmniBlade @tintinhamans I've update the first comment of this PR with the latest changes. |
GadgetStaticTextSetText( labelVersion, versionString ); | ||
UnicodeString fullVersion; | ||
fullVersion.format(L"%s - TheSuperHackers %s", versionString.str(), SUPERHACKERS_VERSION_WIDE); | ||
GadgetStaticTextSetText(labelVersion, fullVersion); |
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.
Could you make the indentation of all 3 lines match the removed line? Same for the Zero Hour file.
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 use spaces instead of tabs for indentation in IntelliJ, so it looks aligned to me. I’ve just updated it to replace the spaces with tabs again.
fc02c01
to
813798a
Compare
Please move the version change to a new Pull Request because it is not related to pre-releases workflow. This relates to task #1010 |
I don't think we have anyone active right now who can review this CI stuff. |
I work as a devops for many years, maybe I can help the project on CI reviews. I'm also have a few ideas for a more standard gitflow and release process |
Will work on that this weekend. |
@xezon @roossienb @OmniBlade @tintinhamans @Mauller moved the game logic changes to a new PR as requested: #1171 |
Do we need the text file? Can't it calculate based on the last tag in git for the change log and then just set the release tag to something like "weekly-dd:mm:yy"? That will nicely separate it from full release versions that can use semver when we ge to that point? |
How can I play the pr-release build with GenLauncher to play a mod? |
use my fork for now. |
I tried already pre-release build of your fork. I replaced generals.exe and modded.exe in working genlauncher directory, but the genlauncher didn't run a mod correctly. |
My build uses the exact same code from SuperHacker's repository. Please open an issue for further analysis. |
@OmniBlade can you review this? |
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.
Thank you for the images. The resulting Release overview looks pretty good. Perhaps we can name this "Preview Version" instead of "Pre-Release"? Basically we want to communicate that auto published versions are not hand picked final releases and can contain side effects.
@@ -0,0 +1 @@ | |||
0.9.1 |
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.
Why was this version value chosen?
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.
It's just a random value and can be set whatever we want to.
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 there needs to be some thought as to how these releases are differentiated from our "official" releases. As I've already commented, I think semver should be reserved for final release versions.
I also don't get what the vesion text files get us that can't be just stored in git itself as tags?
@@ -0,0 +1 @@ | |||
0.9.1 |
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 this versioning should be left out of the weekly builds completely personally.
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 agree. We can make the versioning enterily independent of the CI.
echo "changed=true" >> $GITHUB_OUTPUT | ||
fi | ||
|
||
calculate-version: |
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 benefit do we get creating these files to be moved around rather than just calculating the information and storing it in variables where the workflow requires it?
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 needed by build-toolchain and will be used in C++ counterpart here and here
I do think our previews and releases should only contain the vc6 + tools builds, no win32, no extras. Because the win32 is not meant to be used by players at this time. |
What about this approach?
This allows us to:
|
If the CMake can generate the version, then is it fine to use that instead? |
Although I don’t have experience in c/c++ coding, I can try to do it. What will be the version pattern? Just the git version? |
I can do that, because I already have all the code here to facilitate this. |
Great! Just to make sure we're on the same page, for this PR I need to:
Anything else? |
Yes that would be good. So for versioning I have prepared change #1219 that uses a CMake script to generate an additional git version file for C++ which looks like so: GeneralsGameCode/resources/gitinfo/gitinfo.cpp.in Lines 20 to 28 in 0f0352e
We can generate another version of this in CMake that the CI could read, in Json format or just Plain text: whatever works. Additionally we write game version information in GeneralsGameCode/GeneralsMD/Code/Main/CMakeLists.txt Lines 30 to 47 in 477be26
For in game versioning, I currently settled on:
Which in turn should look like so:
For VERSION_BUILDUSER we do not yet have logic in place to actually generated the string from CI. So that is something we would need to add (in this change). I think CI needs to:
Do you think you can work with this information? Do you need assistance? I can help with the CMake side of things. I think we need to finalize #1219 first. |
What this PR does?
This PR complements #1171
Creates a github action to create a weekly release with the latest code changes. It will be scheduled to every moday.
https://github.com/fbraz3/GeneralsGameCode/actions/runs/15689781515

It also can be mannualy triggered, and it have parameters to bypass the code changes check to force a new build

The release notes also have a changelog with all commits since the last release - for the first release it will be limited to the last 10 commits
