Force a point when serializing a double game param

[gabrfarina]

This is just a bandaid solution to address some quirks in the way
game parameters are serialized and parsed.

See: #382
Unblocks: #376

I have added some tests to make sure the parser's behavior does not
change accidentally.

Should I rebase onto #381 once it lands?

[gabrfarina]

Some playthrough tests fail as a result of the new serialization code. I will look into it.

@lanctot What is the backwards compatibility story of OpenSpiel? Currently this patch would guarantee that anything that was successfully parsing (deserializing) before would continue doing so, and result in the same game. Is that good enough?

[lanctot]

You almost certainly need to regenerate the playthroughs, I would guess. BTW I think we might have used semicolon as a special token in serializing.. if you used that for your delimiter in your lists, could cause a problem. I have used hyphens to do string-represented-lists in some games (negotiation, I think).

Yes, that is good enough, for serialiaztion. Master branch is expected to move, and we are doing releases for people who want greater stability, and people can always fork if they want maximal stability. Of course this is all within reason, we don't arbitrarily break backward compatibility of the core API, even on master-- at least not without some amount of warning-- because that is unnecessarily disruptive, and in any case tend to do that only if it's absolutely necessary.

[gabrfarina]

Great, thanks for the clarifications. I have fixed the playthrough tests by simply amending the GameParameters line in the txt files to always include 10 digits after the point. I think that's better than regenerating the playthrough, because this we we get to see if this patch affects the playthrough files in other surprising ways that we were not accounting for.

About semicolons: that's unfortunate! Using dash seems like a dangerous idea for lists of doubles (like the ship values), where the dash could represent a minus in front of the double. At this point perhaps should I use spaces to separate the items in the lists?

EDIT I've committed the new playthrough files. Build + test is green locally, so I expect this will be ready to merge as soon as Travis finishes.

[lanctot]

If the tests passed then I am either misremembering, or the serialization code manually escapes the existing semicolons, so this is fine!

[gabrfarina]

Travis is green 🚀

What's your rebase policy? Should we just wait for #381 to land to master and then rebase? Or are you planning to roll them up together?

[lanctot]

Great, importing now. I can pul them both in, but I will need someone else to review it. I assume this one will be fairly quick, Battleship might be longer. Hoping to get them both into Monday's update but the internal reviews might only come on Monday too.

[gabrfarina]

Done. I will not cherrypick this last commit onto the battleship branch for now, as I guess I'll just end up rebasing battleship onto master when this PR lands (on Monday hopefully if I understood correctly?)

[lanctot]

Had to make a few edits because try / catch does not work as expected internally. But looking good. This means the master branch will have some local modifications from your Battleship PR, which you'll need to merge or rebase before we go ahead with Battleship.

[gabrfarina]

Perfect, thanks for the speedy review process, it's a pleasure to contribute to code bases this well managed :-)

The battleship branch is currently based on master. These fixes to the game parameters code were just cherry picked. I didn't want to rebase battleship onto this pr's branch while it still had outstanding code comments from you ---- I don't know what github would do if I were to push force after the rebase, and I didn't want to risk losing your comments /review.

When this lands I will simply rebase the battleship branch onto the head of master and drop the cherry picked commits.