Refactor GameCommand Lists with Key into Multimaps

[Hanmac]

Cases like this:

forge/forge-game/src/main/java/forge/game/zone/MagicStack.java

Lines 995 to 1016 in 31d38f6

	public final void addCastCommand(final String valid, final GameCommand c) {
	if (commandList.containsKey(valid)) {
	commandList.get(valid).add(0, c);
	} else {
	commandList.put(valid, Lists.newArrayList(c));
	}
	}
	private void executeCastCommand(final Card cast) {
	for (Entry<String, List<GameCommand>> ev : commandList.entrySet()) {
	if (cast.getType().hasStringType(ev.getKey())) {
	execute(ev.getValue());
	}
	}
	}
	private static void execute(final List<GameCommand> c) {
	final int length = c.size();
	for (int i = 0; i < length; i++) {
	c.remove(0).run();
	}
	}

• First: why are the GameCommands are added in the front? That doesn't make much sense. (see later point)
• Second: why does execute removes them like that? Wouldn't it be better to iterate over the whole list, and then clean up all entries for the given key? (that might be problematic with new GameCommands added in the front? See above)

another example are player specific Phase Commands:

forge/forge-game/src/main/java/forge/game/phase/Phase.java

Lines 50 to 52 in 31d38f6

	private final HashMap<Player, ArrayList<GameCommand>> untilMap = new HashMap<>();
	private final HashMap<Player, ArrayList<GameCommand>> untilEndMap = new HashMap<>();
	private final HashMap<Player, ArrayList<GameCommand>> registerMap = new HashMap<>();

[tool4ever]

How about we just refactor the single card that uses it (Soul Sculptor) into just using effect?

[Hanmac]

How about we just refactor the single card that uses it (Soul Sculptor) into just using effect?

Yeah, I was thinking about that too, and for the Soul Sculptor part, I would be happy with an Effect too

I was just thinking that you would complain about "not AI friendly" 😅

[tool4ever]

ok, I'll refactor it later
the card is marked RemoveDeck already anyway :P

[Hanmac]

@tool4ever what about this one?

forge/forge-game/src/main/java/forge/game/ability/SpellAbilityEffect.java

Lines 1069 to 1078 in fce2158

	public static GameCommand exileEffectCommand(final Game game, final Card effect) {
	return new GameCommand() {
	private static final long serialVersionUID = 1L;
	@Override
	public void run() {
	game.getAction().exileEffect(effect);
	}
	};
	}

It can be refactored to () -> game.getAction().exileEffect(effect)
but maybe the whole exileEffectCommand function can be removed like this?

[tool4ever]

yea I think you can remove it completely (same for the Interface)
I also found no reason for serializing them