first draft of generic deck move system

[ssoulier]

I am clearly no there yet but at least unit test of Inferno Four and R2D2 are working.

I am not convinced by the value of the generic method to move card from deck vs having a couple of classes dedicated to the different scenarios we may encounter (top/bottom, top/discard, ...) . But this is very softly opinionated. I'll follow your advice on this @AMMayberry1 .
For example, I am wondering if we can't handle the discard destination in MoveCardSystem and then It should be easier to create a specific class for the Top/Discard case.

I have a problem with

export function lookMoveDeckCardsTopOrBottom<TContext extends AbilityContext = AbilityContext>(propertyFactory: PropsFactory<ILookMoveDeckCardsTopOrBottomProperties, TContext>) {
    return new LookMoveDeckCardsTopOrBottomSystem<TContext>(propertyFactory);
}

In gameSystemLibrary. It's not clear how to fix that between the base class and the children class

I have some difficulties creating a generic getEffectMessage for all the situations so I decided to not overwrite it in the base class. But maybe I should like in MoveCardSystem and implement a complex logic. I was a bit lazzy.

Finally, I am sure to understand how MoveCardSystem ensure we can choose the order of the cards we put on the top or bottom.

[AMMayberry1]

Re: getEffectMessage yeah just don't worry about it for now, we haven't been consistent about using that in general so we can come do cleanup on it later if we need to (not sure if we'll continue to use it the same way)

[AMMayberry1]

For the GameSystemLibrary question - not sure what the specific issue is, but it should be fine to create one function for the parent class and one for the child class. This is what we did for deckSearch() (parent) vs playMultipleCardsFromDeck() (child)

[AMMayberry1]

I think we need to make this a little more complex - unfortunately we can't just pass in 'discard' as the move destination, we need to actually trigger the DiscardSpecificCard system. Two reasons:

1. There might be special rules around discarding that are explicitly handled by that system (or will be in the future)
2. If there are triggered abilities that react to discard, then we need to correctly emit an OnCardDiscarded event instead of a generic OnCardMoved event

In addition, we want to make it possible to provide a string name for the button so we don't have to maintain a list of all the options in this class, that's overall difficult to maintain. Much better to let the caller declare it themselves, except potentially to handle some really common defaults (which is what the subclass idea is about).

What we have used in similar situations such as e.g. the SelectInterface use the IChoicesInterface which pairs a string name for the button with the associated action - see Ezra impl for an example.

[ssoulier]

unfortunately we can't just pass in 'discard' as the move destination, we need to actually trigger the DiscardSpecificCard system

This is what I am doing line 106, where I use DiscardFromDeckSystem. Does it answer your point about using the dedicated system for Discard?

[AMMayberry1]

Probably we should remove this method completely and move it entirely to the parent class. afaik the subclass has no reason to change the behavior of this

[AMMayberry1]

The way the ordering for top / bottom happens is that when the player clicks a button, we generate a move event for that card and push it onto a list. Then we resolve the move events in that order. So the cards are always moved in the order the player clicked the buttons.