Consider if merged-stack infrastructure should be server-side

[jkomoros]

Currently boardgame-card-stack is what merges them together. But that logic could go on server side as a computed property kind of thing. For example, Blackjack has EffectiveHand.

[jkomoros]

This is the source of the error in #520 . It's finicky to do it well, especially with sanitized values.

[jkomoros]

Would want these MergedStacks to be able to be output as computed properties (that might just work now that we can output interface{})

[jkomoros]

• [x] Constructors (deck is same, then for overlap which verify size, that both are fixed for overlap)
• [x] Tests
• [x] merged.Ids doesn't handle sanitization of Len correctly, because it should be overrideIDs
• [x] When a state is checked to be valid (when we do the playerindex check), also check that the merged stacks don't error (as in, the stacks they're based on are the same size for overlap @mode). All stacks get a valid() error. By default return nil, but mergedStacks actually do error checking there
• [x] We also do all of the playerIndex/stack.valid() checks when a new game state is first created, as well as on the exampleState check in NewGameManager.
• [x] MutableStacks get an importFrom(other Stack) that basically copies in all of the data from that other object into its own self.
• [x] Enums, Timers get an importFrom (although ugh that means that Enum.importFrom has to be public... )
• [x] copyReader uses the various importFrom for all interface types instead of copy
• [x] copyReader fails gracefully when the property for stacks and enums isn't mutable
• [x] state.copy() generates a new state object from exampleState and then copies over values into it.
• [x] Rationalize all of the state copy/creation methods as described in the comment below into the one canonical newState flow.
• [x] state.copy() has a bug, right? it won't work for timers and stacks that are in dynamic component indexes, becuase it doesn't actually enumerate them. (might be moot after refactoring) nah, dynamicCompnentIndexes were set in that main loop when they were created)
• [x] unspecial case enum.Val in autoreader
• [x] autoreader is OK with non-mutable stacks, too
• [x] NewGameManager will remind you to use MutableStacks if there's an empty Stack in exampleState.
• [x] ErrPropertyImmutable is now used for any mutability mismatch, so its name is wrong
• [x] add ConfigureStack, ConfigureEnum, ConfigureTimer, support in autoreader, re generate all readers all example games
• [x] Actually use computed property form for Memory to test
• [x] is state null that's being passed to c.ID()?
• [x] REad in config for concatenate and overlap struct tags autoinflation for stacks
• [x] Actually do something for fields that have the concatenate or overlap struct tags
• [x] test concatenate/overlap struct tags (will require defaultReader to actually do reasonable things for interface properties that are immutable, and return real values for MutableProp())
• [x] newReaderValidator should barf if concatenate/overlap are included on a MutableStack, if stack/sizedstack are included on an immutable stack. (newReaderValidator should take a propertyReader AND an optional propertyReadSetter, and use the latter only for the PropMutable check)
• [x] AutoInflate or newReaderValidator (one of the two) need to know when props are mutable or not so they don't error when inflating.
• [x] make sure that UnmarshalJSON for merged stacks drops it on the floor, and MarshalJSON does output useful things for mergedstacks
• [x] Package doc (including updating README with new sections, and the patterns for merged stacks, and updated code snippets)
• [x] What happens if you have a mergedstack that relies on another merged stack in the same object? How can you make sure the dependency graph is done in the right order?
• [x] can we get rid of setReaderStatePtr, since we changed how copies happen?
• [x] doesn't state.copy drop secretMovecount ont he floor? Is this a long-standing bug? Perhaps that's the source of that bug noticed clientside awhile ago about secretMovecount.
• [x] Allow any arbitrary number of stacks in a given merge list (1...n)
• [x] Use for all example games
• [x] get rid of blackjack.player.EffectiveHand
• [x] get rid of stack.inflate() and just have deck() do memoization
• [x] Use for all games in games repo
• [x] can interface types get rid of copy() and mutableCopy()?
• [x] Remove from boardgame-component-stack

[jkomoros]

Retunring an error from a merged stack constructor seems like it's required. (At the very least for overlapped stacks, where we have to make sure their size is the same).

but it seems like a big pain to do especially because in most cases the error will be dropped on the floor.

It also seems like merged stacks are a very common idiom for differentially-private information. Should the concept be baked in more deeply, and have SubStates have a merged stack (name, name, type) configuration that can be checked at NewGameManager time? And then changes in e.g. setSize for a stack that is linked to another stack in merged stack in overlap mode would also resize the other stack.

We could have them be a property on a SubState:

They'd be special in that in the datamodel they'd only encode the primary, secondary, and merge style.

Conceptually they'd be created in your Constructor methods:

func (g *gameDelegate) GameStateConstructor() {
  deck := g.Manager().Chest().Deck("cards")
  primary := deck.NewStack()
  secondary := deck.NewStack()

  //merged just maintains refs to those two literal stacks. The primary and secondary have a pointer to overlapStacks they're a part of, so if their size changes they can let the other one know. 
  merged := NewOverlappedStack(primary, secondary)

  return &gameState{
    primary: primary,
    secondary: secondary,
    merged: merged,
  }  

}

But of course struct based tags could be used to do that automatically:

type gameState struct {
  boardgame.BaseSubState
  HiddenHand boardgame.MutableStack `stack:"cards" sanitize:"order"`
  RevealedHand boadgame.MutableStack `stack:"cards"`
  Hand boardgame.Stack `concatenate:"HiddenHand,RevealedHand"`
}

Note that this means that all of the tooling would have to support Stack (non-mutable) in SubStates. Look at how we do enum.Val to see how hard that is to support.

In JSON output we'd output them just like a normal stack--just with duplicates. When Unmarshalling from JSON for merged stacks we'd just drop on the floor because our state is fully represented by the other stacks we're based on.

This set-up only allows easy-to-define merged stacks that are members of the same SubState object (becuase the mergedStack must be non-nil when returned from the constructor). If that doesn't fulfill your need, then you can still create a MergedStack as a computed property.

[jkomoros]

The pattern for enum.Val that we used in autoreader doesn't work for the GameState structs because it only works if you're only generating a reader. The PropertyReadSetter interface assumes that all interface types are mutable.

That implies that we need a new MergedStack type so it can be represented correctly in the various interfaces.

Alternatively we could support immutable interface types in SubStates, but I'd want it to be consistent across all of them, and the use cases for others just aren't there. In copying we'd try to get a Mutable version, which would fail, and so we'd need a ConfigureStack on PropertyReadSetConfigurer for setting these in that case.

Note that Copying merged stacks is challenging because they'd have a copy to the original stacks, not the stacks in the new state. :-/

[jkomoros]

Actually, we don't need the primary/secondary stacks to know about each other--it's just that when the state is about to be saved we'd call Valid(), which in addition to checking whether player indexes are valid, would also verify that merged stacks are valid. In fact, that might be the operative way we check for errors on merged stacks. We create one always--it's not until the Valid() check that we look to see if all of the invariants are met, so we can report them correctly. Would need to make sure that NewGameManager does that Valid() check on states in the initial set up.

This is simpler because stacks that are part of a merged stack don't have to know it in order to inform other related stacks when their size changes, and removes spooky action at a distance.

[jkomoros]

MergedStacks are really interesting, because in theory if the underlying stack objects in other SubStates never physically changed their identity, then MergedStacks are initalized and configured in the constructor and never need to change.

However, today physical stacks DO change after being constructed, because ConfigureMutableStack just physically sets the stack, and that's called in Copy. We could have MutableStacks have a Configure(other Stack) that just adopts the config of the other stack. (That then is weird because it goes against the MutableStack Configuration need, because you could do it manually by just doing readSetter.MutableStack('name').Configure(other), even if you didn't have a ReadSetConfigurer. Wait, we also use ConfigureMutableStack to auto-inflate, not just to copy.

You'd represent merged stacks simply as boardgame.Stack. You'd get them via PropertyReader.StackProp(). But MutableStackProp would fail because they aren't a stack. And ReadSetConfigurer would grow a ConfigureStackProp(propName string, stack Stack). In auto inflation we'd create a merged stack of the right type and set it in with ConfigureStackProp. \

We could also have similar behavior for enum.Val (it wouldn't make sense to include for enum.Timer, because an immutable timer is nonsensical)

Then we'd just have to figure out a way for copying SubStates to work for the MergedStack--because only the auto-inflation knows how to create the stack to copy, and MergedStack.Copy() won't work because it needs a physical pointer to the stacks it merges, and that's not passed in there. And the Reader.Copy() in readerutil shouldn't have to know about MergedStacks ideally.

[jkomoros]

Whoa, state.copy doesn't use gameManager.gameStateConstructor, which does all of the auto-inflation magic. It just creates literal empty objects and then copies over items one by one. Because we use the autoInflation when we load up a refried state, and for the very first state in a game, it copies over correctly.

One thing we could do is change how we do copy to also use the inflation constructors. We'd get an empty object, copy over any things that could get MutableCopies, and then run autoinflation to fill in the rest.

Conceptually then constructors go like this:

1. Get a container value from delegate.GameStateConstructor. This might have lots, or nothing, filled in for interface types.
2. If we have an example to copy over from, copy over as much MutableStuff as we can get. Skip things that e.g. are a Stack but no MutableStack for now. For new items, or loading from disk, we skip this step.
3. Use autoinflation to fill in anything that's still not filled in. This is the point at which merged stacks can run, because we won't be copying over things again.

We should then use this flow for all copies and creates by rationalizing the stateFromRecord, state.Copy, gameManager.SetUpValidators, manager.exampleState, game.starterState, and game.SetUp.

Wait, this doesn't actually work, because autoinflation conceptually should just be convenience sugar for what you can do yourself in GameStateConstructor(). Which implies we need another way for merged stacks to get a ref to their actual stacks. :-/

Just add an importFrom(other Stack) private method to MutableStacks. And then only use it within copyReader.

state.copy() doesn't create a new state, but rather generates an empty state with enough players and then imports all of the information.

[jkomoros]

It's nice that currently if you accidentally use boardgame.Stack in your state struct, it will fail. If you do a Stack and it's empty (that is not initalized by overlap or concatenate struct tags) then we should have a descriptive error message reminding you that you generally want MutableStack.