Closed jkomoros closed 7 years ago
Actually thinking about this more, it's weird to have non-trivial logic in a wrapper instead of in the object itself.
Also, Connect() is not in the core boardgame layer. The fact that e.g. memory and bolt don't require a Connect() is a feature, not a bug, because it allows testing in those contexts. We do already exercise the non-trivial behavior (stuff like what caused this bug) in the mysql integration test, which is fine. So this is WAI?
Just fixing the CurrentPlayerINdex() home point is probably sufficient then.
I believe these are all now fixed.
This broke likely around c6ac49bce0fb7bc16057e668c3357b3ab75052f8
First tracked in #516, some commits have landed against that issue.
The issue was that moves.CurrentPlayer started being executed during manager.SetUp, but it uses games.CurrentPlayerIndex(), which hits CurrentState which hits storage, which doesn't exist. :-/
Tests didn't catch it--it was only when running the server that I noticed an odd error that was preventing it from starting.
0ea579ca98de9c273268f0cc5a5cd7c4d60221c3 is a temporary fix that has mysql storage layer return errors if not corrected, but the problem is more deep.
go test ./...
fails with the issue to ensure future tests will find the issue. In particular, ensure that every example has a test to check that the game can be set up without errors (at the very least)