Converting cards for multiplayer

[moomarc]

I've started converting the cards to make them multiplayer compatible. As I go through I'll add the RemMultiplayer flag to cards that need further work. To start things off I've changed Player.Opponent so that it only returns actual opponents, and added Player.Other to return all other players. (No longer will the AI kill off it's allies when it should be restricted to opponents)

I've covered cards from A-D that use the phrase "each opponent" and so far there's just two that I couldn't convert yet (if short memory serves). One just needs a new or modified count method which I should be able to handle when I have time. The other is a fairly common scenario that will take more work (and likely from someone more competent than me). The problem is those instances of "for each <player> do something" when a counting method is used relative to the player (such as "Each player loses life equal to the number of cards in his/her hand" - don't think that actually exists but its the best example I could come up with offhand).

Anyway, just thought I'd start this thread as a single place to post any related observations or questions I (or anyone else) have. If you see anything you feel should be handled differently, let me know.

[moomarc]

Hmm, I thought I might as well get some basic count methods done to make life easier as I go along, but I'm struggling to get this working so hopefully someone will be able to spot what I've done wrong. My test subject is Descendant of Kiyomaro and for the hand size comparison I've got the following:
Card script snippet:

Code: Select all

S:Mode$ Continuous | Affected$ Card.Self | AddPower$ 1 | AddToughness$ 2 | AddTrigger$ TriggerGainLife | AddSVar$ TrigGainLife | CheckSVar$ X | SVarCompare$ GTY | Description$ As long as you have more cards in hand than each opponent,...
SVar:X:Count$InYourHand
SVar:Y:PlayerCountOpponents$HighestCardsInHand

AbilityFactory.calculateAmount snippet (I've checked and calculateAmount is definitely called by StaticAbility.checkConditions:CheckSVar so I know the problem isn't that):

Code: Select all

ln:542      } else if (calcX[0].startsWith("PlayerCount")) {
                final String hType = calcX[0].substring(11);
                final ArrayList<Player> players = new ArrayList<Player>();
                if (hType.equals("Players") || hType.equals("")) {
                    players.addAll(Singletons.getModel().getGame().getPlayers());
                    return CardFactoryUtil.playerXCount(players, calcX[1], card) * multiplier;
                } else if (hType.equals("Opponents")) {
                    players.addAll(card.getController().getOpponents());
                    return CardFactoryUtil.playerXCount(players, calcX[1], card) * multiplier;
                }
            }

Lastly, in CardFactoryUtil.playerXCount:

Code: Select all

        if (l[0].startsWith("Highest")) {
            for (final Player player : players) {
                players.clear();
                players.add(player);
                final int current = CardFactoryUtil.playerXCount(players, s.replace("Highest", ""), source);
                if (current > n) {
                    n = current;
                }
            }

            return CardFactoryUtil.doXMath(n, m, source);
        }

EDIT: Nevermind, I sorted it out. Clearing the var players seems to have been causing the issue. Now it just creates a new ArrayList and all seems to work fine. On with the conversions

[moomarc]

Just a small observation while converting more cards. While changing Lobber Crew I thought I'd change to DamageAll:ValidPlayers$Player.Opponent instead of DealDamage:Defined$Player.Opponent. When I tested it though, it seems that the AI won't activate the crew's ability at all when set up with DamageAll, even if I'm at 1 life with 0 creatures. It activates them without qualms though when I reverted back to DealDamage.

[moomarc]

UPDATE: With Molten Psyche I've opened the door for converting other cards that need a value calculated per player. Essentially in each effect resolve that needs it, a small section needs to be added that calls playerXCount directly for that player if the parameter 'LocalCount' is present. The actual Count$ part of the string is stripped seeing as it doesn't pass through calculateAmount first (I figured it would be easier to call playerXCount directly rather than add a player param to the calculateAmount method and each call to it, and I couldn't think of a scenario where you would need a local count and one of the calculateAmount values other thasn Count).

[friarsol]

UPDATE: With Molten Psyche I've opened the door for converting other cards that need a value calculated per player. Essentially in each effect resolve that needs it, a small section needs to be added that calls playerXCount directly for that player if the parameter 'LocalCount' is present. The actual Count$ part of the string is stripped seeing as it doesn't pass through calculateAmount first (I figured it would be easier to call playerXCount directly rather than add a player param to the calculateAmount method and each call to it, and I couldn't think of a scenario where you would need a local count and one of the calculateAmount values other thasn Count).

Does RepeatEach not work here instead of adding a few different SVars and function calls?

Repeat Per Player
Shuffle Cards into Library (and remember them)
Remembered Player draws that many cards
If Remembered Player is an opponent conditionally takes damage.

[moomarc]

Should work. I was just convinced for some reason that RepeatEach didn't work for players. I'll look into it when I get to Forge again. Thanks for the advice!

[friarsol]

Should work. I was just convinced for some reason that RepeatEach didn't work for players. I'll look into it when I get to Forge again. Thanks for the advice!

I converted Winds of Change so you can have an example, since no cards were actually using RepeatPlayers. Molten Psyche should be pretty simple with that as a baseline.

[moomarc]

Should work. I was just convinced for some reason that RepeatEach didn't work for players. I'll look into it when I get to Forge again. Thanks for the advice!

I converted Winds of Change so you can have an example, since no cards were actually using RepeatPlayers. Molten Psyche should be pretty simple with that as a baseline.

Thanks Sol! Finally converted those LocalCount methods in favor of RepeatEach. Hopefully over the next few days I'll have the time to convert another big batch of cards instead of the occasional one or two.

[moomarc]

There seems to be some problem with triggers for additional opponents in multiplayer games. Current test scenario is Black Vise which I converted to be multiplayer compatible. I give each of my two opponents a Black Vise and they choose Human obviously. So far so good. Then my upkeep starts and the stack freezes with the triggered ability on the stack once. Cards can be added to the stack via dev mode's 'Add to play', but they can't be resolved either and no other actions can be performed (aside from conceding/restarting etc, so the game isn't frozen, just the stack).

So I broke it down further and tried with just one opponent at a time. When I give just the first opponent the Vise, the trigger goes off correctly on my upkeep. If I give the only the second opponent a Vise, either by Donating the current Vise or by giving a new on and destroying the old one; at the beginning of my upkeep nothing happens. The game proceeds as usual until another spell is added to the stack at which time it freezes as per the first scenario. The same is true for all opponents other than the first.

So I don't know if it's a priority issue somewhere or something in triggers, but hopefully this will help someone trace the problem.

EDIT: I also added a printline (System.out.println("Chosen = " + source.getChosenPlayer() + "; This player = " + this);) in Player.hasProperty:Chosen that outputs the chosen player and current player for comparison and the console prints "Chosen = Human; This player = Human" as expected. It does mean that the trigger line in the script is being read properly and there's no issue with the player comparison, so I'm guessing that the priority is getting messed up somewhere.

[Sloth]

It does mean that the trigger line in the script is being read properly and there's no issue with the player comparison, so I'm guessing that the priority is getting messed up somewhere.

I'm on vacation at the moment, so i can't help you a lot.

The multiplayer freezing bug has been around since multiplayer exists. Priority getting passed around (maybe back and forth between two AI's) sounds like a good lead.

[moomarc]

It does mean that the trigger line in the script is being read properly and there's no issue with the player comparison, so I'm guessing that the priority is getting messed up somewhere.

I'm on vacation at the moment, so i can't help you a lot.

The multiplayer freezing bug has been around since multiplayer exists. Priority getting passed around (maybe back and forth between two AI's) sounds like a good lead.

Mostly I was hoping that it would be a good lead for someone with more advanced skills than I have seeing as I was finally able to find a case that is easy to track how far it was getting and I was able to test what happens when consistently. But if you can point me to where the priority is passed then I might just have a look anyway (always good to understand more parts of the code).

[moomarc]

Finished with all the cards that use the phase "each opponent" (aside from Kinship cards and Angelic Arbiter). I've also converted a few count methods and random cards along the way. Next up: Each player.

[moomarc]

Just plodding along when I saw something wierd. Lines 45-51 in AbilityFactory.SacrificeAllEffect looks like it should come out as it seems to want to convert "X" in the ValidCards string to an integer. I'm guessing this is left over from when the abilities were split into separate classes, as I can't think of any situation in which X should need to be calculated in SacrificeAll (and besides, There's not a single instance of capital X anywhere in a script following SacrificeAll). I'm removing for now so if anyone can see what it was meant to do that I missed, let me know.

[friarsol]

I think the problem with triggers in Multiplayer matches may be because of:

MagicStack.java | Open

public final void chooseOrderOfSimultaneousStackEntryAll() {
final Player playerTurn = game.getPhaseHandler().getPlayerTurn();

this.chooseOrderOfSimultaneousStackEntry(playerTurn);

if (playerTurn != null) {
this.chooseOrderOfSimultaneousStackEntry(playerTurn.getOpponent());
}
}

[moomarc]

Thanks Sol! Definitely seems like it could be the problem. I had checked for getOpponent in triggers without luck when I first tried to look into it. Wasn't sure where else to search though. Which player should be referenced here? Is it the next player in turn order? If so I think I'd created a method to return the next active player when I added the 'until your next turn' stuff for effects. Otherwise if it should be each player, should be easy enough to loop through getGame.getPlayers. Hopefully I'll have some time tomorrow to look into it.

Does this get called even when there's only one trigger though?