Terraforming-mars: Valley Trust provides four prelude cards instead of three

[dabewi]

Hi,

Valley Trust seems to be buggy again. It provides 4 prelude cards and not only 3 as it should be:

https://abload.de/img/img_0839rtk6d.png

This error was already fixed (see https://github.com/bafolts/terraforming-mars/issues/215) but it seems that I'm not allowed to reopen that issue, therefore this new one.

Cheers!

[bafolts]

I am able to recreate this on a 3 player game with player 1 as Valley Trust.

[bafolts]

I am also able to recreate this on a 2 player game with player 1 as Valley Trust. It doesn't seem to reproduce on a 1 player game.

[bafolts]

The problem here is that valley trust calls to our internal player playCard method which increments actions taken this turn and then calls again to take action. This is then putting valley trust back into play and allowing the user to select a card again. The fix here should be to implement a play card method that valley trust can use which doesn't increment the internal state of the player while playing the card.

[bafolts]

Where there is smoke there is usually fire. The underlying issue here of a call to takeAction indirectly calling another takeAction will be a problem more than only here. To fix this issue fully it may finally be time to refactor takeAction. I see at least two other places that will end up with a similar issue:

https://github.com/bafolts/terraforming-mars/blob/master/src/Game.ts#L907
https://github.com/bafolts/terraforming-mars/blob/master/src/cards/promo/Philares.ts#L88

Depending on what initiates these actions we could end up calling into takeAction when we were already waiting for an action which will then lead us calling into takeAction twice. Not sure of solution yet but I have a grasp on the scope of the problem.

[vincentneko]

I've tested Valley Trust in a 2 player game, this is now running as expected.
@bafolts thanks for the takeActionrework, this is great !