Closed rzulian closed 4 years ago
I've added the tests for three players, and also for the assert conditions.
I've added the range
to simplify the bid process. I hope I didn't screw up the free actions in the viewer ;)
Regarding the setup
variable. In your case you have also to add the list of the original factions, because you have to keep track of the not bid factions. I don't think is really much different from this implementation, but I can do it if needed
Nice thing with the range variable :)
I downloaded the code on my end.
A few things:
yarn lint --fix
. It should fix a lot of errors (I can't run yarn test
because of the lint errors)Youhave to spend more than ${bid}
); <- space missing between "You" and "have"Two more things:
In a lot of places, this.players.map()...
is replaced by this.setup.map()...
. For me this is a problem because it means that the turn order for the first turn is determined by the auction:
p1 faction itars
p2 faction geodens
p1 bid geodens 1
p2 bid itars 1
With this, with how the code is made, it means that p2 will start the turn and put the first building. Is this normal?
With 4 players, with how the code is written, the turn order changes during auction, which is complicated
For example:
p1 faction itars
p2 faction geodens
p3 faction xenos
p4 faction ivits
p1 bid itars 1
p2 bid geodens 1
p3 bid geodens 2
p4 bid itars 2
With this, we would expect that p1
is next to auction, but with the code it's p2
. For example, if you add p1 bid itars 3
it will throw an error, because it expects player 2.
Those two last things could be fixed if we base ourself on this.players
for the turn order. It would also remove a lot of one-line changes, and possibly make the engine compatible with exisiting games on the same version (instead of having to create a new version).
For the turn order during faction, you could do something like this (and remove the use of tempTurnOrder
in the auction phase completely):
[Phase.SetupAuction](move: string) {
this.loadTurnMoves(move, {processFirst: true});
const player = [...range(this.currentPlayer + 1, this.players.length), ...range(0, this.currentPlayer)].find(player => !this.setup.some(item => item.player === player));
if (player !== undefined) {
this.currentPlayer = player;
} else {
this.endSetupAuctionPhase();
}
}
Thanks for the good feedback!
I've corrected the code to support the initial turnOrder
during the auction, which is aligned with the BIG rules.
You are correct. The turn order will be determined by the auction. The logic behind is that value of the faction can be more or less based on the position, so you are bidding for that faction on that starting position.
In the tournaments the option can be that the factions are randomly selected and can be the same for all tables.
@rzulian I'm ok for the merge.
Regarding the linting, a lot of errors aren't picked up by travis but are locally on my computer. I'll do the fixes after the merge.
We should probably create a page on the site (yay CMS!) explaining the auction mechanism, and that the turn order is determined by the order the factions were first chosen.
I've added a new property to handle pos, faction, player, bid and a new option
auction
I didn't complete the assert in theCommand.Bid