Refactor card-specific game state

[rspeer]

There should be a way for a card to store card-specific information in its own piece of the game state, instead of arbitrarily-named properties such as state.current.tacticians.

[bilts]

I've been looking at ways to accomplish this. Did you have anything in mind?

One difficulty I've noticed is caused by this design decision (from cards.coffee): "There is no need for classes because there are no separate instances. Each Copper is a reference to the same single Copper object, for example."

I'm curious why that decision was made and whether you'd be open to changing it. I can't imagine it saves that much memory. It seems like a lot of tricky code becomes unnecessary when cards are distinct (e.g. one-off Goons code), and this issue becomes much easier to fix.

[rspeer]

Replied to bilts in a Dominion game, but I'll post it here for the record too.

The one-off Goons code could be refactored, but it's not the fault of cards being singletons.

I designed cards as singletons because it seemed like there would be hard-to-find bugs if cards had state on them; the rules basically don't put state on individual cards except for where they are (which in Dominiate means what list they're in).

Equivalent cards are completely identical and indistinguishable when they're in your deck, and they're basically identical when they're in your hand too. Think of Donald's justification for why you can reveal the same Secret Chamber multiple times: because other people can't tell if it's the same SC or a different one anyway.

There are occasional exceptions like Tactician (one tactician in play can be different from another), but those are reasonably easily handled.

Now, I'll try to reply later in more detail about the general question, but basically, there should be more hooks in the game state where the cards say what to do. Just because cards aren't separate instances doesn't mean they can't have code on them; you can iterate through each card, for example, and ask "Does this card care that something is being bought?" Every Goons card will answer the same way. So then we can define that on the Goons object and not with special code in the game state.

[bilts]

First, I think that everything you need to know about a card should be self-contained. I think we're on the same page there. I should be able to find out precisely what Goons does just by looking at its card definition. That's not only useful for maintainability, but also for allowing users to define cards.

As for the instance/singleton issue, I understand what you're saying, and I think your points are valid. Perhaps a lot of it comes down to the way we think about code (OO vs procedural, perhaps). My strong suspicion is that singleton cards result in longer code that's harder to follow, combine, and test.

Let's keep considering Goons for a moment. A game may have 10 copies of Goons. At any point in time, a few may be in the supply, in play, in each player's draw, discard or hand, and/or in the trash. Depending on that piece of state, the cards behave differently from one another. If you played two Goons this turn and are in your buy phase, those two instances of Goons are the only ones you care about.

You can even tell cards in play apart from one another if that's important. With physical cards, you might play a duration on top of a KC on top of a KC, or play your treasures from left to right so you can count the value of your Banks.

In-game, you talked about the difference between what a card is vs where a card is (very insightful, btw). Thinking about it more, the location of a card seems like it should be a core property of a card instance.

So, enough with the abstract stuff, what does it buy you?

I think the code ends up becoming much simpler and clearer, and it maps much better to what's printed on the card. For instance, Goons says "+1 Buy, +$2. Each other player discards down to 3 cards in hand. While this is in play, when you buy a card, +1 VP."

Ideally, the code should look something like this (fake API, completely untested):

card "Goons"
  type: ['Action', 'Attack']
  cost: 6
  play: (game, player, turn) ->
    turn.coins += 2
    turn.buys += 1
    opponent.forceDiscard(handSize: 3) for opponent in player.opponents

    player.bind('buy', this, -> player.vpTokens += 1) unless player.bound('buy', this)

  cleanup: (game, player, turn) ->
    player.unbind('buy', this)

I think that reads a lot more like what's printed on the card. The bind/unbind thing is a little rough, but could be sorted out through helper methods. If you play 2 Goons, that's fine. Each one binds to the buy event and things just work. If you KC the Goons, you can just call play 3 times.

With singleton cards, the code gets a little more complex, but more importantly it starts looking less like what's printed on the card. You have to say something like "When someone buys a card, +1 VP per Goons in play." It becomes less obvious that a card is implemented correctly and harder to test.

Another idea here is that separate instances also lets you subscribe to events in a reasonable way. So you don't have to poll all cards for every game event in case one of them cares. You can directly notify the cards that care at any given moment.

Wow, pardon the novel.

Ultimately, this is all probably a minor thing that comes down to personal preferences, but I think separate instances buy a lot when it comes to defining and testing cards.

[bilts]

Ok, consider this me getting acquainted with the code. It seems as though Goons would look similar with or without singleton cards.

A few oddball cases get easier, but it seems as though the main thing that separate instances add is the event registration thing. It still feels cleaner to me, but less compelling.

Back to the issue at hand... moving specific card state out of the game state.

[bilts]

The above set of commits is factors out all of the card-specific state except for Young Witch (bane card) and Trade Route. I took the least invasive approach I could.

Young Witch should be straightforward, since I added a start-of-game hook (used by Tournament to set up prizes).

Trade Route is tricky since there's no on-buy hook.

The code I added for Tournament prizes should be useful for the Black Market market deck. Cards can set up and use a special supply with cards that aren't available through the normal supply.