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.35k stars 399 forks source link

Next Incompatible Release Planning (v1.10) #2827

Closed ron-murhammer closed 5 years ago

ron-murhammer commented 6 years ago

So once we cut the next stable (hopefully in the next few days), I'd like to turn attention to what items folks would like to see in the next incompatible release. I think it would be good to get a check list together so we can have a sense of what people want to focus on. My initial thought is target a 3 month period and see where that gets us.

RoiEXLab commented 6 years ago

Should #2614 be closed in favour of this?

ron-murhammer commented 6 years ago

@RoiEXLab Maybe best to summarize #2614 based on the conversation there and add to the initial post here? Given that the thread is 2 months old, probably better to get a fresh start :)

RoiEXLab commented 6 years ago

@ron-murhammer Most of the discussion was if we fork off a "stable" branch we could submit hotfixes to, in case a P1 or P0 issue is encountered

RoiEXLab commented 6 years ago

@ron-murhammer I updated your list with a minor entry

RoiEXLab commented 6 years ago

@ron-murhammer Another task we should do after we decided to make incompatible changes: If it's possible, we should consider bumping https://github.com/triplea-game/triplea/blob/master/latest_version.properties to the latest version String if that's possible. I just checked the lobby DB, and it seems there are still 1000 people without a migrated bcrypt password. Once we're going to remove the support for those passwords, we're going to need to remove the accounts as well... We should probably add this to the lobby info text in case we can't bump this version string.

ssoloff commented 6 years ago

Regarding this task:

Submit Instants over the network instead of using Date

This was part of the bigger issue of how RMI methods are enumerated. One thing we should look into is making the method "sorting" a little more forgiving so it doesn't break compatibility to add new remote methods 99% of the time. That will go a long way to ease maintenance until a new network protocol is in place.

We might also want to consider running a parallel lobby during this time that is compatible with the new method sorting for pre-release users. IIRC, @DanVanAtta set up a second lobby instance for testing someplace; don't remember what happened to that.

ssoloff commented 6 years ago

This may be partially covered by some of the above tasks, but what about removing as many deprecated methods/types as possible?

ssoloff commented 6 years ago

What about changing the task "Rename all m_* variables" to "Fix all Checkstyle naming violations"? That will cover packages and types (in addition to fields) that we haven't been able to rename due to save game compatibility.

ron-murhammer commented 6 years ago

@ssoloff Feel free to update it. Though we need to be careful with package names as some are referenced in the map XML.

ssoloff commented 6 years ago

@ron-murhammer Good point. I updated it to read "Fix all Checkstyle naming violations (minus those that would break map XML compatibility)".

DanVanAtta commented 6 years ago

Incompatible releases are really disruptive in TripleA as games can often be played out over months. It's a pain for a number of other reasons, I'll skip the details though.

What can we do from an architecture point of view to reduce the need for incompatible releases? I think those items should be some of our primary objectives/goals. Making incompatible releases more rare will help everyone. At the same time the compatibility problem is one of the more brittle aspects in the tripleA code base - would be a great benefit to develop to have confidence that any given update would not break network, or break save games.

RoiEXLab commented 6 years ago

@DanVanAtta .@ssoloff Should still have his savegame proxy branch somewhere around, so that reduces the amount of incompatible savegames there. While we're already there, we could also use this chance to refactor the attachment classes and use interfaces instead of reflection. I believe an important goal should be to fix the netcode which is causing the most compatibility issues at this point, the pseudo RMI does completely break when adding new methods to it.

DanVanAtta commented 6 years ago

@RoiEXLab IIRC the attachment classes are now looked up by name instead of reflection: https://github.com/triplea-game/triplea/blob/master/src/main/java/games/strategy/engine/data/gameparser/XmlGameElementMapper.java

@ssoloff also IIRC correctly, the proxy framework we tried to use had a fatal circular dependency. Would that be a problem with a more classic proxy done as a one-time effort? I do remember that approach allowed for GameData references to be removed.

Regardless of whether we use serialization proxy or not - I'm starting to be pretty convinced that save game compatibility should be the goal of next incompatible release. That would give us a lot of flexibility going forward and make it much less burdensome when we require players to update.

ssoloff commented 6 years ago

IIRC correctly, the proxy framework we tried to use had a fatal circular dependency

@DanVanAtta Yes. The first attempt was nice because it was basically a drop-in replacement that didn't require many changes to existing code. But it ultimately failed because there is a fundamental restriction in the Java serialization framework that prevents circular references between proxies.

The second attempt abandoned the proxies as we typically think of them and went with a more memento-based approach. That required a lot of plumbing changes in existing code to be able to reconstruct an object graph. However, there were still some circular reference issues, but they appeared to be resolvable via some refactoring. Unfortunately, the refactoring itself would have introduced compatibility issues, and that's when I decided to abandon the effort.

After going through this twice, I'm thinking that taking a "let's introduce save game compatibility" tack a third time may well fail again. There needs to be some fundamental redesign in the engine. I think using the problems we discover while trying to isolate the persistence functionality should drive changes in the engine. However, my gut's telling me that may take longer than one release cycle.

And I didn't even get to the issues I expected to run into when I tried to proxy the IExecutable stuff... :smile:

RoiEXLab commented 6 years ago

@DanVanAtta

@RoiEXLab IIRC the attachment classes are now looked up by name instead of reflection

Yes that's correct, but I mainly meant the developer site of things. On those attachment objects we are currently calling getters and setters of certain fields via reflection. Ideally we'd have a map for each attachment type containing objects implementing some sort of attachmentproperty interface that defines a setter and a getter method (and perhaps some overloads for convenience) as values, with their respective names as keys. Ideally the package name for the attachment types as specified in map xmls wouldn't be necessary either. There shouldn't be multiple attachment types with the sane class name anyways

RoiEXLab commented 6 years ago

Not necessarily a breaking change, but it would be useful to change the bots to try to reconnect to the lobby in exponential intervals, I.e. Retry in 10 seconds, retry in 100 seconds, thousand, tenthousand etc. Or simply double the value each time.

DanVanAtta commented 6 years ago

On those attachment objects we are currently calling getters and setters of certain fields via reflection.

Can that be updated without a breaking change?

There needs to be some fundamental redesign in the engine. I think using the problems we discover while trying to isolate the persistence functionality should drive changes in the engine. However, my gut's telling me that may take longer than one release cycle.

We may be doomed if we must always solve this in one release cycle but at the same time cannot. IMO it's worthwhile looking at this in more detail. For the moment, updating variable names and releasing one new future does not yet seem super valuable for breaking everyone's save games. If we stop breaking save games, most of the incompatible pain goes away, players only need to download an updated client and pick up any games they had going.

RoiEXLab commented 6 years ago

@DanVanAtta

Can that be updated without a breaking change?

Might be possible, but this wouldn't allow us to do the very much needed refactoring.

DanVanAtta commented 6 years ago

@RoiEXLab @ssoloff @ron-murhammer this is an important topic, we need to get to consensus on this.

Breaking save game compatibility is a major deal, in the past participation has dropped after breaking changes (confusion on lobby, fact that most players remain in old, the mutiple versions to manage).

Old jars was meant to address this, but:

TripleA is a bit unique in that save games can be played out over a few months. If we removed this problem, we could do incompatible releases with much less impact. It's not ideal, but it's okay if the game engine forces an update and then a few minutes later be playing. We could do incompatible releasees every few months, maybe every month. But it's another issue to manage, as a user save game to engine version compatibility over a few months, potentially losing a few games when the old lobby goes away.

This is why I think it makes sense to solve this problem, then incompatible releases becomes much easier down the road and we can make many other changes without needing to wait for the next incompatible release cycle (and again impacting players with older save games that they can no longer easily play)

It's a challenge, but it would mean we would have a goal to fix this problem, perhaps we can try to find something that would work and split it up between us. At the same time queuing up a few useful features to take advantage of the incompatible release would make sense as well. What is everyone else thoughts, is save game compat just not feasible to solve? Will we be locked in this cycle of impacting users over weeks, maybe longer when we release new non-compatible versions (which is why we do not do it very often)?

simon33-2 commented 6 years ago

My wish list for a breaking release would be to include:

I cannot see how anything in the OP is more important than the first one at least and probably the second one too. FWIW.

prastle commented 6 years ago

@DanVanAtta Everything you said above is basically true. I do not think you will ever solve this easily. But considering we have peeps playing from a to z engines it would probably be best to have at least have one breaking release every 6 months at the rate of your releases. Just my humble opinion. This at least keeps everyone on the same rough engine page. Which allows easier bug diagnosis. Just my two cents.

ron-murhammer commented 6 years ago

So I think its time to begin working on an incompatible release now that we seem stable on 1.9.0.0.12226. And by that I mean I'm planning to break save game and lobby backwards compatibility (but preferably not map XML backwards compatibility unless we feel its justified).

Let's try to get a list of items together in the first post that we'd like to tackle. Ideally, we shoot for a 3-6 month development period depending on how many things we'd like to tackle.

ssoloff commented 6 years ago

I know we've talked about this before, but, in light of what happened with infrastructure, what's the plan for pushing hotfixes to the current stable during the next 3-6 months? Are we going to create a dedicated branch for 1.9?

ron-murhammer commented 6 years ago

Ideally, we don't need any :)

In reality, we'll create a branch off of 12226 and make any hotfixes. Then figure out how to deploy those to the bots.

prastle commented 6 years ago

YUP!

prastle commented 6 years ago

Yes I will get yelled at but as a side note (and off topic) If easy hosting was ever developed we would never need bots... ;)

ssoloff commented 6 years ago

Then figure out how to deploy those to the bots.

While it's fresh in my mind... We still might need to create a dedicated branch at that point so we can enable releases from it. Right now, only builds from master produce a release. .travis.yml would have to be updated to include the additional branch.

RoiEXLab commented 6 years ago

Not sure if it has been noted related to this topic before, but ideally we'd get rid of the MD5Crypt dependency altogether. Lobby accounts that are still not migrated are probably "dead" and would be removed from the db. I don't know the numbers, but it would be nice to know how many would be removed in advance. The next steps would include features that rely on MD5Crypt for some sort of encryption. Those should be eliminated as well and replaced with a proper encryption algorithm.

Also we want to drop a version number and work on fixing the auto update notification mechanism

prastle commented 6 years ago

If they have not migrated yet I doubt they ever will. I agree that they are "dead".

DanVanAtta commented 6 years ago

Is the first-post list current @ron-murhammer ?

It seems like we may still need to do two things:

  1. confirm what to include for a next non-compatible
  2. confirm branching strategy

I can be persuaded otherwise, though it seems like we solve many problems now and mostly forever when we fix compatibility constraints. It seems like our focus should be there. Those three areas are:

ron-murhammer commented 6 years ago

Yeah, its current. I think those are good areas to try and tackle but they do require probably significant effort so would need someone to dedicate cycles to. Feel free update the first post with your thoughts.

DanVanAtta commented 6 years ago

Thanks. With luck networking compatibility updates are far less impactful than they once were.

For save-game compatibility changes, are these the only two items that would break that?

  • Fix all Checkstyle naming violations like m_* (minus those that would break map XML compatibility)
  • Move types so marked to new packages that couldn't be previously moved due to save game and/or network compatibility

I'm thinking it'd be most worthwhile for us to pick one of those three areas of non-compatibility and try to resolve one on major upgrades. Networking I think has a path forward and progress is being made. That mostly leaves save-game + map parsing.

Very much devil's advocate, if we don't tackle a more major initiative, will dev coding conveniences be worthwhile to the community/user-base? I'm thinking of the many notable examples where save-games have a very long shelf-life. In terms of highest impact, my thinking is we should tackle save-game compatibility.

prastle commented 6 years ago

I think we just do a 2016. Keep two lobbys up. (opps we are) and give them 6 months warning save game be dammed!

DanVanAtta commented 6 years ago

@ssoloff we could go whole hog on serialization proxy, non-backward compatible. That would keep the updates straight forward, we'd drop a lot of GameData references, and that would give us a migration path for replacing java serialization with something else. I could see the alternative to wait on that and just directly migrate to a replacement as well. I'm curious if you think this is the right opportunity to resolve the save game compatibility problem.

DanVanAtta commented 6 years ago

(asking @ssoloff directly given the extensive work in this area)

ssoloff commented 6 years ago

@DanVanAtta Sure. If we're breaking compatibility, revisiting serialization proxy makes sense.

DanVanAtta commented 5 years ago

Since we've broken compatibility, it looks like we are on our way. Unless there is a need to re-open, any new topics should probably get their own issue and reference this one as needed.

DanVanAtta commented 5 years ago

Re-opening, this is a very good planning thread for v1.10 and there are even unresolved topics (this was closed prematurely, my apologies there).

DanVanAtta commented 5 years ago

@ron-murhammer I think this kind of topic is a good fit for Github 'project' tracking. I set up a shell project to get started: https://github.com/triplea-game/triplea/projects/6

I think this issue is perfect for discussing what should land in the 'todo' column and for general 1.10 release planning.

The benefit to the project is that other issues and PR's can be related without needing to remember this issue number or adding a comment to relate them. I do not think it's great practice for us to require edits of comments to see latest status. In one-off cases it works, or if really hacking at the forums, long-term and naturally the person that writes a comment, owns it. Same thing if an issue is created generally. @ron-murhammer it's up to you if you want to keep a check list here or use the project.

Now that we are pretty far into the non-compatible release, it seems we should finish discussing what we intend to be included. To add to the list, a few things to discuss or note:

On my radar has been match challenges (https://github.com/triplea-game/triplea/projects/3). To get there we need an http server, a pattern for interfacing with it, and lots of DB work as we start to do a lot more persistence. In that context, error-reporting has been good for proving a way to do client-server communication, moderator toolbox which I've started recently is intended to break in the DB layer to be able to scale for more complexity.

prastle commented 5 years ago

Above all points have my vote @DanVanAtta just my noob two cents Pras

DanVanAtta commented 5 years ago

Latest release planning is in this thread: https://github.com/triplea-game/triplea/issues/5025