Triplea: Start working on a breaking release

[RoiEXLab]

With an increasing amount of workarounds to maintain compatibility, I'd like to suggest to go for a new breaking release 1.10.build

Prerequisites

• [x] We update the lobby and all the bots to the current stable release (or the build after the stable build due to a bug) and keep it running for a month. If we're not encountering any issues, the release can really be considered stable. Some bots are already on 1.9.0.0.7534. Note that the lobby requires it's db scheme to be changed.
• [ ] We create a branch "stable" which will continue to exist in the meantime, in order to be able to fix any P0 and P1 issues
• [ ] _BIG IF_: IF everything works out as expected, we should have a code signing certificate at the end of the month, so we _could_ release a signed binary as latest stable

Goals (although not final)

• [ ] Fix all checkstyle violations in serializable classes (and those used by the netcode) to get those out of the way, also the engine would no longer need to check if the vars are m_prefixed for game xmls.
  
  
  
  • [ ] Switch to @ssoloff's recently reverted savegame format #2612 drop the support for the current one, also drop support for file extensions svg, tsvg and tsvg.gz
  
  
  • [ ] Completely change the net code, probably the biggest task, this includes removing deprecated methods and passing Instant objects instead of Date objects, also we should switch to dedicated objects instead of passing Maps wrapping a couple lists in order to drastically improve readability. Also we should start working on a better "mute/ban" system, perhaps reputation based.
  
  
  • [ ] Split up attachment classes. Currently the Attachment classes are like _really messy_. Suggestion would be to turn for example TechAttachment to some sort of interface which is then implemented by the individual TechAttachments. this would simply split up the attachments into different classes to be much more readable.
  
  
  • [ ] Switch to a 3 number version system, once and for all (although I'd be fine with 4 digits as well although I'm pertty sure we'd never use the 3rd digit then)
  
  

Optional

• [ ] Perhaps we should convert the gamexml DOM parser to a SAXParser, not really tied to compatibility unless we want to change the required XML Scheme
• [ ] Come up with a good solution for Map downloading, not really tied to compatibility either unless we want to change the map xml scheme, currently the version defined in the triplea_maps.yaml file is pretty much useless. The game engine just downloads a new version from the latest master of the repo, so we could either just use the current hash and insert it into the file whenever a map gets updated, or we go completely crazy and implement some sort of package manager which allows us to map map versions to a hash and a compatible triplea version string, preferably using semver and a backing DB instead of a simple yaml file. (Requires an issue on it's own though)

Your suggestions: What's your opinion on this?

[ron-murhammer]

@RoiEXLab Agree. The only issue I'd like us to try and address before rolling out the latest stable is #2600. As there does seem to be some bug related to multi-hitpoint units. If we can address that then update the website download and let it sit for say a week. Then we can roll it out to lobby/bots and begin work on incompatible release.

[ssoloff]

We create a branch "stable" which will continue to exist in the meantime, in order to be able to fix any P0 and P1 issues

I'm highly in favor of a separate release branch, regardless of what we call it (FWW, Git Flow uses the master/develop convention for stable/current). I'd hate to see a P0 issue come out after we made some incompatible changes to master.

And, yes, we need to get a handle on #2600 before moving forward with an incompatible release. I plan on working on that today.

[RoiEXLab]

Ok, lets freeze this until #2600 is resolved then!

[simon33-2]

Small thing: 1.10? Why not 2.0 or just 2?

[simon33-2]

Also shouldn't breaking changes be merged into a different branch than master or am I missing something. Didn't merging #2612 break that rule?

[ssoloff]

Also shouldn't breaking changes be merged into a different branch than master or am I missing something. Didn't merging #2612 break that rule?

2612 simply removed an incomplete beta feature. There were no breaking changes (at least not intentionally).

[RoiEXLab]

@simon33-2

Small thing: 1.10? Why not 2.0 or just 2?

Good question this depends on us completely.
From the semantic version definition:

MAJOR version when you make incompatible API changes

Which we are technically going to make, one could argue that a major increment of the version number should indicate a complete or big partial rewrite, but on the other Hand @ssoloff's savegame format is going to be a huge improvement.

Also shouldn't breaking changes be merged into a different branch than master or am I missing something. Didn't merging #2612 break that rule?

@ssoloff's savegame format was still experimental you could use it if you wanted to, but you didn't had to and you really shouldn't have)
If you used it, those changes are indeed breaking, but in that case it shouldn't be too hard to load in a tsvgx file into triplea and save it as tsvg

[ssoloff]

@ssoloff's savegame format was still experimental you could use it if you wanted to, but you didn't had to and you really shouldn't have)

I can make it even easier... You couldn't have used it. :smile: It would always error out during a load due to the circular reference problem.

[prastle]

Atm 24 hrs later and thanks to you @RoiEXLab and @ssoloff for helping with the bots. 0 issues so far in the bots running .7534

Thanks guys for all the hard work.

[RoiEXLab]

2600 has been fixed.

I'll leave it up to you when to create a second branch (I'm fine with develop as I'm fine with stable), as I'm pretty busy recently.

[ssoloff]

Throwing a GitFlow reference out for review to see if this is something we'd want to adopt (possibly minus release branches). IIRC, @DanVanAtta had some reservations about this type of branching model in the past.

[ron-murhammer]

@ssoloff Yeah, I don't think we need full gitflow. I think just one branch for next incompatible release and one branch for compatible changes and fixes. One of those 2 should be master, I don't really care which one.

[ssoloff]

@ron-murhammer Right. The only thing everyone needs to agree on then is that stable fixes should be merged into both the stable and current branches. Should that responsibility be on the fix author? That is, the author would open two PRs from their fix branch: one each against the stable and current branches?

I suppose the current branch could have diverged so much from the stable branch that the fix branch wouldn't merge cleanly with the current branch. It may not even make sense anymore in the current branch due to architectural changes, etc. In those cases, I guess we'd have to decide on a PR-by-PR basis whether it makes sense to (somehow) merge it into the current branch.

[ron-murhammer]

@ssoloff Yeah, my initial thought is do fixes to compatible and incompatible branches. Everything else to only incompatible to minimize having to merge into both frequently.

[RoiEXLab]

@ron-murhammer @ssoloff I think we should prefer merging any stable fixes into the stable branch and merge the stable branch into the develop branch afterwards if possible.
Another thing that we need to keep in mind is the release artifacts I don't think we should push a gh release after every commit, although building artifacts in general are of course useful for regular users.

[RoiEXLab]

See #2625

[DanVanAtta]

We create a branch "stable" which will continue to exist in the meantime, in order to be able to fix any P0 and P1 issues

I'm a no on this. We discussed this when going to the single feature branch model. We decided we would do 'hotfixes' by checking out the last released tag, do a one-off branch, release that branch, then throw it away.

[DanVanAtta]

We update the lobby and all the bots to the current stable release (or the build after the stable build due to a bug) and keep it running for a month. If we're not encountering any issues, the release can really be considered stable. Some bots are already on 1.9.0.0.7534. Note that the lobby requires it's db scheme to be changed.

I am really concerned by this. We created the testing group so that releases could be 'stable'. I've been working on this project for two years to get rid of the whole 'stable' 'unstable' concept. If merge code to master that is not 'stable', we are saying that one: bugs are okay, and that two: it's okay for the admins and users to find all those released bugs and to then be on the hook for fixing them. We really got off track by having broken code for a few months and had trouble getting all those problems fixed. My hope is that we get tighter on this, start releasing more often, QA each pull request better, and generally do a better job releasing.

[DanVanAtta]

@RoiEXLab a missing major goal would be to get save game proxy in place. That would have this next incompatible as the last release that can't natively parse backward compatible save game formats.

[RoiEXLab]

@DanVanAtta I understand your concerns on having a "stable" "unstable" model.
The Point is, we probably want a period, where we can make lots of incompatible changes to completely overhaul the internal code structure.
I'd assume after we did a lot of incompatible changes we'd change our system to work with a single branch again.
The idea of having 2 branches was just in case we discover a severe bug to reliably be able to fix it without having to finish with the incompatible changes.

[RoiEXLab]

@DanVanAtta And I completely agree on the savegame proxy change, this is actually the second point on my list assuming I wasn't clear enough.

[DanVanAtta]

The idea of having 2 branches was just in case we discover a severe bug to reliably be able to fix it without having to finish with the incompatible changes.

With severe bug we can create a branch from 1.9.0.0.3635, push it, tag it, and the tag should get released. We can then make any further fixes on top of that tag. Let's make sure that is possible, as that has been the plan for some years when we switched to Github.

[ssoloff]

With severe bug we can create a branch from 1.9.0.0.3635, push it, tag it, and the tag should get released.

Not quite. Currently, _.travis.yml_ restricts releases to only the master branch and tags are never released (this seems to have been added to avoid an infinite loop in the build system due to how we push tags from Travis back to GitHub). As is, we'd have to modify _.travis.yml_ on every one-off branch to permit a release.

I actually tried to fix that "releasing tags results in infinite build loop" issue several months ago by pushing tags back to GitHub using the recommended idiomatic Travis mechanism. I remember attaching a patch to some related issue. Let me search for it, and I'll link back here.

[DanVanAtta]

As is, we'd have to modify .travis.yml on every one-off branch to permit a release.

Sounds acceptable, we expect such releases to be one-off.

---

For our goals of what to have in the incompatible release, I think our top goal should be compatibility projects.

[ssoloff]

This post contains the patch I mentioned above where I modified our build to do a release to GitHub for only tags instead of a specific branch or branches.

[RoiEXLab]

@ssoloff @ron-murhammer @DanVanAtta
Lobby migration went fine so far... No problems (I'd know of) were reported since then, so we could start creating a separate branch to start pushing incompatible changes.

@ssoloff I like your proposal, according to the semver convention we are still allowed (See 10.) to add some kind of "build number"/commit hash as part of the version string, as long as it's seperated by a plus sign and dot-seperated.
Note that a build identifier doesn't have any affect when comparing two versions per semver definition, but in this case we probably want to increment the patch number on every build (resetting it on every minor increment), and increase the minor _or_ patch number on every "stable version".

In any case we should ensure that the latest stable TripleA client should be able to "handle" a new version structure even when we change it in such a drastic way, so users are prompted to download the new version once it's available.

[RoiEXLab]

Given that we should "fix" the game engine behaviour which tells the users once a new game engine version is out.
We really need users with a new engine version to log in in order to get the DB migrated to BCrypt, there are still too much unmigrated accounts

[RoiEXLab]

Closing in favour of #2827