search

triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.34k stars 397 forks source link

Next Compatible Release #1768

Closed ron-murhammer closed 6 years ago

ron-murhammer commented 7 years ago

So we have a few threads floating around on the next release and incompatibilities (#1735, #1715, etc). So I wanted to get a more concise list of issues we want to complete before releasing with the idea of the following release we could then break compatibility.

Remaining Features For Release:

simon33-2 commented 7 years ago

Doesn't it have to be 1.9.0.x? Changing that breaks the old jar interface further so shouldn't be done IMO.

RoiEXLab commented 7 years ago

@ron-murhammer Are we changing the versioning system before releasing a new stable release? If we do, we should probably update the code to be able to "recognize" any newer versions before releasing a new compatible stable. Other than that I don't have anything to add...

BTW. Removing unused branches on the repo is probably a good idea: Branches

DanVanAtta commented 7 years ago

Hmm, I'm afraid I may be starting to be a bit confused.

One note, the next release we mark as stable, must be 1.9.0.0. Recall that the current version most people are on is bugged if we do a version upgrade. With that said, I am interested in marking the latest release as stable ASAP, so we can get back into the habit of doing that after every few changes.

Following through from that, once we're ready for the non-backwards compat release, I thought we would then jump to a our new 3 number release numbering system (notably so we don't keep bringing up and redoing release numbering so many times)

@ron-murhammer / others: kindly confirm if that is the plan

ron-murhammer commented 7 years ago

@DanVanAtta Not following you on why its bugged? The idea is to just increment the last non-build number so we can drive most folks to upgrade as we haven't pushed the changes out for a while otherwise they never upgrade.

DanVanAtta commented 7 years ago

@ron-murhammer is the plan to set the next number to 1.9.0.1 when we are ready for non-backwards compat, and then work on the 2.0.build migration, and then release directly as 2.0.build? (which means 1.9.0.1 would never be officially released)

ron-murhammer commented 7 years ago

@DanVanAtta No. My thought was to release 1.9.0.1 as the last backwards compatible release before we start working on non-backwards compatible changes and revising the versioning.

So releases would go like this: 1.9.0.0 - current 1.9.0.1 - next compatible (soon) 2.0 - not compatible with new versioning

DanVanAtta commented 7 years ago

re: bugged

The latest release is all way back in january (https://github.com/triplea-game/triplea/releases/tag/1.9.0.0.3635). Recall when we tried to bump version, we had game clients lock up: https://github.com/triplea-game/triplea/issues/1493 The fix for that was pushed in: https://github.com/triplea-game/triplea/pull/1558, which unless I have my dates crossed, is not part of the current stable.

ron-murhammer commented 7 years ago

I thought the bug was resolved based on the issue being closed and the PR merged?

DanVanAtta commented 7 years ago

I see, in one sense there were two issues:

Hence the gap and different views.. :crying_cat_face: Happy to draw lessons - but, we do need to move forward..

DanVanAtta commented 7 years ago

I suggest as a plan (100% open to discussion):

ron-murhammer commented 7 years ago

@DanVanAtta Yeah, that works. I was primarily using this issue to track point 3.

simon33-2 commented 7 years ago

We can update the version number so long as we don't change the latest_version file.

Not following why we would do something different?

simon33-2 commented 7 years ago

BTW, We've already had a master called 1.9.0.1 so I think it should go to 1.9.0.2, or 1.9.0.build.

simon33-2 commented 7 years ago

Getting set of console errors with the latest build, 4684.

It seems that the issue is related to the first line of the xml on a lot of maps. "\<?xml version=1.0 ?>" works but "\<?xml version=1.0?>" doesn't. The difference being the space before the second question mark. Should this be solved with a bulk update or an engine change? I would say it needs to be fixed before marking the current release stable.

triplea.engine.version.bin:1.9 Could not parse:file:/home/simon/mapsrc/domination_1914_blood_and_steel/map/games/Domination_1914_Blood_And_Steel.xml error at line:1 column:1org.xml.sax.SAXParseException; systemId: jar:file:/home/simon/TripleA_1.9.0.0.4684/bin/triplea.jar!/games/strategy/engine/xml/; lineNumber: 1; columnNumber: 1; Premature end of file. at com.sun.org.apache.xerces.internal.parsers.DOMParser.parse(DOMParser.java:257) at com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderImpl.parse(DocumentBuilderImpl.java:339) at javax.xml.parsers.DocumentBuilder.parse(DocumentBuilder.java:150) at games.strategy.engine.data.GameParser.getDocument(GameParser.java:272) at games.strategy.engine.data.GameParser.parse(GameParser.java:81) at games.strategy.engine.framework.ui.NewGameChooserEntry.(NewGameChooserEntry.java:56) at games.strategy.engine.framework.ui.NewGameChooserModel.createEntry(NewGameChooserModel.java:195) at games.strategy.engine.framework.ui.NewGameChooserModel.addNewGameChooserEntry(NewGameChooserModel.java:167) at games.strategy.engine.framework.ui.NewGameChooserModel.populateFromDirectory(NewGameChooserModel.java:213) at games.strategy.engine.framework.ui.NewGameChooserModel.parseMapFiles(NewGameChooserModel.java:76) at games.strategy.engine.framework.ui.NewGameChooserModel.(NewGameChooserModel.java:43) at games.strategy.engine.framework.ui.NewGameChooser.getNewGameChooserModel(NewGameChooser.java:198) at games.strategy.engine.framework.startup.mc.GameSelectorModel.selectByName(GameSelectorModel.java:326) at games.strategy.engine.framework.startup.mc.GameSelectorModel.loadDefaultGame(GameSelectorModel.java:312) at games.strategy.engine.framework.startup.mc.GameSelectorModel.lambda$loadDefaultGame$100(GameSelectorModel.java:265) at java.lang.Thread.run(Thread.java:748)

ssoloff commented 7 years ago

@simon33-2 It looks like the latest game XML for that map is empty:

ssoloff@localhost ~/triplea/downloadedMaps/domination_1914_blood_and_steel-master/map/games 
$ ll
total 0
-rw-rw-r--. 1 ssoloff ssoloff 0 Jun  9 05:22 Domination_1914_Blood_And_Steel.xml

Repo says the same thing: 0 bytes.

simon33-2 commented 7 years ago

Doh! Looks like the bulk update stuffed up. I'll fix it.

simon33-2 commented 7 years ago

One minor issue - the right panel on "Start Local Game" sometimes doesn't refresh properly when leaving a game and loading it. I think it's to do with the %income change. Not sure exactly how to reproduce it.

simon33-2 commented 7 years ago

I believe #2080 resolves point #5 of the OP.

simon33-2 commented 7 years ago

Fair dinkum guys. There's been a number of improvements since build 3635 more than six months ago but we are still waiting for them to be included in the generally available release. What exactly are we waiting for? Derby Migration? If this is the problem, can I suggest we drop that change from the feature list?

DanVanAtta commented 7 years ago

What exactly are we waiting for?

Bug fixes and testing. All the features we were trying to get in are there.

ron-murhammer commented 7 years ago

@DanVanAtta @ssoloff @RoiEXLab I believe with merging #2209 that addresses all known regressions outside of the lobby admin database issues which already exist now #2204. IMO, we should be able to release soon unless other bugs are reported. Thoughts on timeline?

Also, for now should we restore setting the L&F to Substance Graphite? Otherwise I have a feeling we will get lots of negative feedback.

DanVanAtta commented 7 years ago

Thoughts on timeline?

We're now testing 6157: https://docs.google.com/spreadsheets/d/1CSZ4s7wBLcvgue_IHwl-CbjQd95qj8hhR8m89ngCzbU/edit?usp=sharing

IMO a few days for that, this weekend to late next week I would expect for us to mark something as a release. Then a few days after that I would recommend to update the lobby message.

Also, for now should we restore setting the L&F to Substance Graphite? Otherwise I have a feeling we will get lots of negative feedback.

That's a good suggestion

simon33-2 commented 7 years ago

I'm disappointed that the review of #1646 has been ticked off without merging #2080. It's a nice usability improvement for my money.

BTW, what's the version number going to be? 1.9.0.build (which I would endorse)?

simon33-2 commented 7 years ago

I agree with the suggestion to restore the default look and feel to Graphite. Looks much nicer to me.

ron-murhammer commented 7 years ago

@DanVanAtta @RoiEXLab @ssoloff I think we should be pretty much ready to release the next stable. Only thing left I think is to restore Graphite L&F default.

ssoloff commented 7 years ago

@ron-murhammer I'm hoping to submit a PR within the next 24 hours related to #2169. The only reason I would want to get it in before the next release is because it changes where the master password used for encrypting credentials is stored. That's a feature that will first appear in the next release, so changing it now won't break compatibility (except for our beta testers who use the latest pre-releases :smile:).

It's not a terribly big deal if it doesn't get in before the next release. We can either live with the break in compatibility (which simply means users must re-enter their saved PBEM/PBF credentials), or I just add extra code to migrate the master password from its old location to the new location.

ron-murhammer commented 7 years ago

@ssoloff That's fine, we can wait for that. Just want to get it out in the next few days as its been dragging on especially while we seem pretty stable.

RoiEXLab commented 7 years ago

Not that important, but just for the sake of consistency: I'd like to merge #2358 before releasing a new stable. The changes are just backwards compatible server-side, but we probably want to have the exact same version as stable and as lobby.

simon33-2 commented 7 years ago

Hallelujah!

Happy to wait for those two issues.

What about #1739? Shouldn't we get rid of the fourth version number?

simon33-2 commented 7 years ago

Logged PR #2394 for removing the 4th version number.

Note that latest_version should not be changed!

simon33-2 commented 7 years ago

Can I suggest that the removal of the old jar mechanism should be held over until the next incompatible release? I find that mechanism really useful.

DanVanAtta commented 7 years ago

my 2 cents, as soon as we have testing cleared, we should release: https://docs.google.com/spreadsheets/d/1CSZ4s7wBLcvgue_IHwl-CbjQd95qj8hhR8m89ngCzbU/edit?usp=sharing

I would also recommend everyone taking a stab at testing and add a column in that sheet and also test the latest version. It's pretty surprising how many usability issues and other bugs can be found.

We have been trying to work out the bugs for some time. Recall too the current version has an upgrade bug so we cannot change major versions until most people have upgraded. It will be difficult to do that as we will have to just ask people to upgrade.

RoiEXLab commented 7 years ago

@ron-murhammer @DanVanAtta @ssoloff I edited the old 1.9.0.1 milestone and used it as "tracking label" for issues that definetely need to be adressed before releasing a new stable. There are just 2 things I'd like to personally get done before a new stable: #2358 and one thing I noticed: The automatic update checking does not make use our "new stable workflow" e.g. using the GH API to determine the latest stable release. I'd like to replace the latest_version.properties File with a couple of requests to the Github API. This files does contain more than just the latest version, it includes a link to the changelog + the download link. While we probably can hardcode the download link here as well, we should be able to move the changelog link to another place or have a unified scheme for the links. (e.g. something like triplea-game.org/release_notes/#1.9.0.0.). Alternatively we should include the link in the github-release-description

prastle commented 7 years ago

(Gives a silent noob cheer in background ) :)

DanVanAtta commented 7 years ago

The automatic update checking does not make use our "new stable workflow" e.g. using the GH API to determine the latest stable release.

I'm not 100% sure we should do that. If we do, we can't have a 'soft-release'. Also, I really do expect us to start marking releases as stable on a weekly or bi-weekly basis. That would be too fast a cadence for the normal user to be upgrading. (Noting, 95% of all release versions will be backward compatible, I would expect non-backward compatible to be once or twice a year, and with current efforts to improve compatibility problems, then maybe almost never!)

RoiEXLab commented 7 years ago

@DanVanAtta If you worry about the message being displayed too often, we could of course implement a cooldown which makes TripleA not check for updates for the first month after a update

prastle commented 7 years ago

Somewhat off topic ( but quite about this ) is the simple point that in the old days everyone had one engine. A stable engine. We need everyone on the same one again. Jmho. It is very difficult to test glitches when George has 37, Sue has 38 and I have 41. Just my two cents worth. Either way you guys are getting there and an amazing job with all the new changes :)

DanVanAtta commented 7 years ago

I'm more worried about releases going out to every player and then us realizing pretty quickly there is a bad bug, and then downgrading the latest release. In that case everyone then has to downgrade, and we potentially have a number of games that are in limbo as well. The alternative I imagine is we mark the latest as stable, then ideally some small percentage would use that version and we would get more feedback before releasing to the entire community.

Month cool downs I think is also pretty arbitrary. I'm not sure we even need to notify players of the latest release if things are backwards compatible. When discussing the 3 point version system we reserved one number simply as a way for us to notify players of versions we would like them to upgrade to. Most updates should be really minor. Hence the month cool down would even be more arbitrary, not clear if it fits. The trade off is just really keeping the existing mechanism, I suspect we may be simply getting a bit too fancy by automating that (not entirely clear how much burden we are removing from what is just a simple file update)

DanVanAtta commented 7 years ago

We need everyone on the same one again.

We may never quite see that again, the pace of development is MUCH faster in github. Also I'm not sure I agree on that need. If versions are backward compatible, and if there are minor differences between versions, I don't quite see the problem if people are on different build numbers.

ron-murhammer commented 7 years ago

Yeah, my thought is the update prompt should only push users to a new major/minor not build version (if we go with 3 point version system). That way for the daily/weekly releases that are just updating the last version number, new downloaders and testers could grab them but not push everyone forward as we will probably end up having more bugs in these. Then when we look to do a major/minor version increment we do a bit more testing and look to drive people forward to that version.

DanVanAtta commented 7 years ago

I agree, but one comment on:

forward as we will probably end up having more bugs in these

WE really need to avoid that! If we are lax and allow 'unstable' versions then we'll be back in the mode of endless bug fixes and testing before releases. I really want to emphasize we need a process that is going to catch most of these issues. We have the testing group as a last line to make sure we don't repeat some of the previous horrible mistakes, but again, that is a last line and should not find any new bugs.

ron-murhammer commented 7 years ago

@DanVanAtta Agree but we also have to be realistic given the amount of automated unit tests and number of developers/testers we have. We do need to be better around keeping things stable and trying to push releases out more often. I think some of the major changes like the database migration, map download/XML changes, and user preference refactoring (while great changes) need to maybe be considered a little more carefully to avoid ending up with as many issues as we did.

Open to ideas around how we avoid unstable versions and keep things on a more regular schedule. It mostly comes down to individual developers making a better effort to test/verify their changes (preferably in an automated fashion) otherwise we shift back to putting too much burden on PR reviewers.

ron-murhammer commented 7 years ago

@ssoloff @RoiEXLab @DanVanAtta I think we resolved all issues needed to put out the next release. Would you agree? Can we update the download to the latest (1.9.0.7029) today?

ssoloff commented 7 years ago

Agreed. Copying @panther2 and @prastle just in case the test team has observed any blocking issues in the recent builds.

ssoloff commented 7 years ago

@ron-murhammer Actually, won't there have to be at least one more build? Does the version need to be bumped to 1.9.1? Or did @simon33-2's recent change effectively take care of that by moving the build number into the micro position? (Sorry, I'm still confused a bit by the current versioning rules.)

Regardless, I'm assuming that latest_version.properties still has to be updated after we've promoted a release.

RoiEXLab commented 7 years ago

@ssoloff The latest_version.properties file is just used to check for newer versions of TripleA (I still think the GH api should be used for that task, just like the website does. For the website to offer the new version the pre-release-checkbox just needs to be unchecked which makes deployment of a new stable very easy.

We should also update the lobby in the next week or so to fix a couple of db issues

simon33-2 commented 7 years ago

Does build 3627 contain the fix to the latest_version.properties check? Hopefully you remember what I'm talking about.

If not, we need to leave latest_version.properties at 1.9.0.0. I think we should probably leave it there for a while regardless.

simon33-2 commented 7 years ago

Regarding bumping to 1.9.1, no. We can't nor shouldn't do that. That would make it an incompatible release.

ssoloff commented 7 years ago

@simon33-2

Does build 3627 contain the fix to the latest_version.properties check? Hopefully you remember what I'm talking about.

Are you referring to #1558? If so, then no that's not in 3635. Good point, though. If we update latest_version.properties, then pretty much every current user is going to experience a hang. So maybe we should avoid updating latest_version.properties for this release?

simon33-2 commented 7 years ago

Oh, in fact, #1558 isn't in build 3635. There is no way we can bump latest_version.properties without causing all existing installations (besides pre-releases) to hang.