simutrans/Simutrans-extended repository

[Mariculous]

This is a follow-up of To the Simutrans github organisation

It seems like James is currently on a hiatus and he's the only one able to accept commits to his repository, which is currently the official repository of simutrans extended.

To circumvent this issue for now and in the future, we (Well namely Freddy and me) would like to maintain a repository owned by the simutrans organisation.

This repository is ready now. You can find it at https://github.com/simutrans/simutrans-extended
Anyone involved in simutrans-extended developement is welcomed to open pull-requests to that repository.
Besides PRs to that repository, I will keep the master in sync with James repository, so it won't diverge.

Further, anyone involved in sumtrans-extended developement is welcomed to join the simutrans-extended team on github.
Team members can push changes to feature branches directy.
Pushes to master are disabled.
Instead, a PR is required.
Any team member can review and accept PRs.
To join the team, please contact sdog.

So far, this repository also introduces nightly builds on github.
From tomorrow on, each night at 2 am UTC, the latest successful commit to master will will be released as Nightly.
You can find the nightly release here: https://github.com/simutrans/simutrans-extended/releases/tag/Nightly
Besides the advantage of not relying on a sigle person, github builds will further move the computational load away from the BB server and the Nightly tag will make it easier to self-compile builds compatible with all servers that run the latest nightly.

Feel free to test the builds.

Known issues:
- The windows binaries generated by the CI seem to link dynamically to libraries usually not installed on windows, thus windows builds are broken.
- The nightly deploy does not check if anything has actually changed, it will simply deploy the latest successful commit each night even if that exact commit is already deployed.

Future plans:
Once the above issues are solved, the following points will be considered:
- Deploy a Simutrans-Extended-Complete package
- Deploy files in a way that Nightly Updater V2.jar could use and provide a patched nightly updater that will pull from that source.
- Deploy nightlies to steam directly from the Nightly workflow.

[jamespetts]

My apologies for not having had a great deal of time to deal with Simutrans related things recently: I have been rather preoccupied with other matters. Thank you to Freahk for working on this in the meantime.

I should not want the Bridgewater-Brunel nightly builds or server to fall behind useful bug fixes or other innovations. I note that your version of the "master" branch has incorporated PJ Mack's pier system, which looks to be a very interesting development which I have not had a chance to review and test yet (and I note that there is a Ranran UI patch also awaiting testing). Can I ask whether the incorporation of the pier system in your master branch means that you have tested this system and that you believe that it is ready for deployment?

[freddyhayward]

I'm alarmed by the inclusion of PJMack's pier patch in the master branch. The role of team members other than James should primarily be maintenance, i.e. bug fixes. Team members should only merge major changes with James's permission, or, if this is not possible, after extensive discussion with other team members.

[Mariculous]

My apologies for not having had a great deal of time to deal with Simutrans related things recently

Thank you for spending a great deal of time into simutrans for so many years!

Can I ask whether the incorporation of the pier system in your master branch means that you have tested this system and that you believe that it is ready for deployment?

Yes, I have tested that feature and discussed it in the related thread https://forum.simutrans.com/index.php/topic,21164.0.html
There had been a few issues with the feature, which have been fixed aftwerwards.
The feedback given on this feature was generally positive, although there has not been much feedback, so I have incorporated this afther the raised issues were solved.

Anyways, we can surely talk about incorporation policies.
Currently, to merge a feature, there has to be a PR, which has to be reviewed and approved by at least one team member.
I have now increased this to 2 team members.
This ensures that a discussion will be started if there are issues noticed when reviewing the PR.
Technically, repository admins can bypass this requirement. They should only do so if either of the following is true:
a) it is an urgent bugfix, for example serious crashes, desyncs or savegame corruption in the current nightly release.
b) the commit is not about the game itself but of administrative means to the github repository. For example adjusting the CI run script.
c) you are James

Is that fine to you?

and I note that there is a Ranran UI patch also awaiting testing

I did not notice this patch yet. I might start a review on this.

Further, when moving a repository, links to the old location will redirect to the new location.
Thus, I'd welcome James to install his repository in place. Moving mine over was rather meant to be a paceholder, so we have a repository to work on in the meantime.

[freddyhayward]

Yes, I have tested that feature and discussed it in the related thread https://forum.simutrans.com/index.php/topic,21164.0.html
There had been a few issues with the feature, which have been fixed aftwerwards.
The feedback given on this feature was generally positive, although there has not been much feedback, so I have incorporated this afther the raised issues were solved.

Anyways, we can surely talk about incorporation policies.
Currently, to merge a feature, there has to be a PR, which has to be reviewed and approved by at least one team member.
I have now increased this to 2 team members.
This ensures that a discussion will be started if there are issues noticed when reviewing the PR.
Technically, repository admins can bypass this requirement. They should only do so if either of the following is true:
a) it is an urgent bugfix, for example serious crashes, desyncs or savegame corruption in the current nightly release.
b) the commit is not about the game itself but of administrative means to the github repository. For example adjusting the CI run script.
c) you are James

Is that fine to you?

In the case of PJMack's pier system, the scale of the update should have warranted a much longer discussion time. And there should certainly be some warning that it is going to be merged soon in the absence of more feedback. I briefly tried it a few days ago and segfaulted. I don't know whether this has been fixed but I didn't have the time to report the crash.

[Mariculous]

I have now updated the incorporation policies with feedback of Freddy.
Think of it as a draft, but I gues that's something everyone actively involved in developement can agree on.

• Members of simutrans/simutrans-extended can accept pull requests, if at least two members reviewed and approved the PR.
  This is technically secured.
• 
  Depending on the scale of the changes, an appropriate time in between the PR and incorporation shall be warranted and a warning shall be given if the feature is going to be merged soon.
  
  
  
  • 
    Minor bugfixes do not necessarily require a discussion, unless an issue was noticed in the review process.
    Minor bugfixes are fixes that do not introduce code restructuring nor affect the gameplay beyond the fix of the bug.
    Bugfixes do always require a comprehensible description of the bug they fix.
    
  • 
    Medium changes require a discussion.
    This category includes all changes not covered by the other cetgories. For example non minor bugfixes or minor features.
    A discussion with a positive conclusion is required.
    When this discussion has a positive conclusion, the changes can be merged in after a sensible amount, so further people have achance to rewiew the changes.
    If there is no such discusion after a sensible amount of time, a warning that this feature might be incorporated soon shall be given.
    If there is no objection within a sensible time after this warning, conclusion can be assumed and the the PR can be merged.
    Medium changes do always require a brief description on what they implement and how to use it, to allow proper reviews.
    
  • 
    Major changes require particular caution.
    All changes that change the savegame version or have a major impact on gameplay or balancing are considered major changes.
    A discussion at least involving the majority of people who actively developed simutrans-extended within the last month is required.
    When this discussion has a positive conclusion, a warning shall be placed that this feature might be incorporated soon.
    If there are no concerns after that warning within a few days, the feature can be merged.
    Major changes do always require a comprehensible description of the concept they implement and how to use them to allow proper reviews.
    
• 
  Repository admins can bypass the technical restriction.
  They should only do so if either of the following is true:
  
  
  
  • 
    It is an urgent bugfix.
    For example there are crashes, desyncs or savegame corruptions in the nightly release.
    
  • 
    The commit is not about the game itself but of administrative means to the github repository.
    For example adjusting the CI run script.
    
  • 
    You are James.
    For example, if you are James.
    

[prissi]

I wonder how such a system can work, because usually I rarely include patches without a few (or more) changes. In your system, this would require another pull request and so on?

[Mariculous]

I am not sure if I understand the concern correctly.
Notice that PRs are not tied to a specific commit, but to a branch.
Thus, when an author considers a feature to be ready, he should push the changes to github and create a PR.
In the review process, issues can be raised. The author can fix these issues and push the changes to the branch and the PR will keep track of the changes.

Is that what you were concerned about?

[jamespetts]

Thank you for responding. First of all, ideally, this repository ought not to get too out of sync with my repository, so we probably do need perhaps more testing work before merging in major patches. The peirs feature looks very interesting indeed, and if I understand correctly from a brief overview what it entails, is something that I have wanted to be included since at least 2014, but such a fundamental change does have the potential to break things, so quite detailed testing is likely to be necessary. My time is quite constrained at present - I will need to set aside a considerable time for testing.

However, the more testing that can be done by the community in the meantime, the less exhaustive that my testing will need to be, so there is potentially great use for a repository of this nature, as it can be used to test features by people other than those who can compile the code themselves that are not integrated into the master branch. Perhaps the Stevenson-Seimens server could be set up to run code from this branch to allow automated testing of builds in an online environment? Certainly testing that the network game stays in sync when this feature is used is one of the important things that will need to be checked.

[Mariculous]

However, the more testing that can be done by the community in the meantime, the less exhaustive that my testing will need to be, so there is potentially great use for a repository of this nature, as it can be used to test features by people other than those who can compile the code themselves that are not integrated into the master branch. Perhaps the Stevenson-Seimens server could be set up to run code from this branch to allow automated testing of builds in an online environment? Certainly testing that the network game stays in sync when this feature is used is one of the important things that will need to be checked.

The original intention of this repository is a central place to allow feature incorporation in a way ready for the main repository, so the work of different people can be incorporated whilst you are busy, reducing the risk of diverging feature branches leading to nastiy merge conflicts.
I am aware that the incorpoartion of the pier system did not meet these requirements.

In the second place, there is a nightly build system using github actions on this repository, which is still work-in-progress.
Once finished and tested in server games, it is It is envisaged to be incorporated into the official repository, so nightly builds can be moved into the cloud, relieving ressources on the B-B server.
I have not yet found a pure github solution to offer builds in a way compatible to the the nightly updater. Github pages might be a solution but I am not yet sure about this.

Setting up Stephensom-Siemens to use the github nightly builds for testing reasons seems to be a good idea to me.

[jamespetts]

Excellent, thank you for your response - this is certainly a good idea to allow for community co-ordinated pre-release testing and merging of patches: that would certainly reduce the administrative burden on me. Do not worry about the incorporation of the pier system not meeting the intention; it is easy to make mistakes when first setting up a new system as it is difficult to think through all the remote consequences of various decisions all at once.

Because of the pier system situation, until that has been merged into the master branch following testing, I do not think that I can use this repository for its originally intended function, but it would definitely be good to see further testing of the pier system on this branch. One very useful piece of testing might be to check whether a map with extensive deployment of piers can be played over a network and stay in sync with the server.

Thank you again for your work on this.

[Mariculous]

I am wondering whether it might be a good idea in this case to rebase this repositories master to remove the pier system for now.
That way, it can return to a state which fits the original intention quickly.

About the pier system, I might setup experimental-feature branches with live builds (i.e. those built by the CI action), based on latest master of this repository.

[jamespetts]

There may be much to be said for rebasing these without the pier system for the present, but also maintaining a pier system branch for the testing of that system, as it may be helpful to have a way for players who are not able or do not have the spare time to compile from sources to test this project and give feedback on it.

[Mariculous]

Both, the simutrans/simutrans-extended and simutrans/simutrans-pak128.britain-ex are now up-to-date excluding the pier system.
You should be able to (fast-forward) pull these.

The pier system is available on the feature-pier_system branch now. I did not yet set up nightlies for these. For now you can use live builds of these branches:
https://github.com/simutrans/simutrans-pak128.britain-ex/actions?query=branch%3Afeature-pier_system
https://github.com/simutrans/simutrans-extended/actions?query=branch%3Afeature-pier_system

[jamespetts]

Can I check whether there are any commits on this branch not on the master branch on the Bridgewater-Brunel server that ought to be?

[Mariculous]

Oh, I must have overseen this.

I'm not sure about the pier system branch. PMJack might know better than me about it.
All changes rom master seem to be pulled. Recently, I just fixed some recently introduced issues in the nightly builds.
Please don't incorporate these yet please, I'll create a branch for this.

[jamespetts]

Oh, I must have overseen this.

I'm not sure about the pier system branch. PMJack might know better than me about it.
All changes rom master seem to be pulled. Recently, I just fixed some recently introduced issues in the nightly builds.
Please don't incorporate these yet please, I'll create a branch for this.

Splendid, thank you. Please notify me when this is ready.