diff --git a/Mage.Common/src/main/java/mage/view/CardView.java b/Mage.Common/src/main/java/mage/view/CardView.java index d4ba85ce637..7386f810296 100644 --- a/Mage.Common/src/main/java/mage/view/CardView.java +++ b/Mage.Common/src/main/java/mage/view/CardView.java @@ -993,7 +993,6 @@ public class CardView extends SimpleCardView { && a.getManaCostStr().equals(b.getManaCostStr()) && a.getRules().equals(b.getRules()) && Objects.equals(a.getRarity(), b.getRarity()) - && a.getFrameStyle() == b.getFrameStyle() && Objects.equals(a.getCounters(), b.getCounters()) && a.isFaceDown() == b.isFaceDown())) { @@ -1004,6 +1003,7 @@ public class CardView extends SimpleCardView { && Objects.equals(a.getCardNumber(), b.getCardNumber()) && Objects.equals(a.getImageNumber(), b.getImageNumber()) && Objects.equals(a.getImageFileName(), b.getImageFileName()) + && Objects.equals(a.getUsesVariousArt(), b.getUsesVariousArt()) )) { return false; } diff --git a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/SimulatedPlayer2.java b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/SimulatedPlayer2.java index 858d2c2e0d2..d02b446c38f 100644 --- a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/SimulatedPlayer2.java +++ b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/SimulatedPlayer2.java @@ -35,6 +35,7 @@ public final class SimulatedPlayer2 extends ComputerPlayer { private static final boolean AI_SIMULATE_ALL_BAD_AND_GOOD_TARGETS = false; // TODO: enable and do performance test (it's increase calculations by x2, but is it useful?) + // warning, simulated player do not restore own data by game rollback private final boolean isSimulatedPlayer; private transient ConcurrentLinkedQueue allActions; // all possible abilities to play (copies with already selected targets) private final Player originalPlayer; // copy of the original player, source of choices/results in tests @@ -54,6 +55,17 @@ public final class SimulatedPlayer2 extends ComputerPlayer { this.originalPlayer = player.originalPlayer.copy(); } + @Override + public void restore(Player player) { + // simulated player can be created from any player type + if (!originalPlayer.getClass().equals(player.getClass())) { + throw new IllegalArgumentException("Wrong code usage: simulated player must use same player class all the time. Need " + + originalPlayer.getClass().getSimpleName() + ", but try to restore " + player.getClass().getSimpleName()); + } + + super.restore(player.getRealPlayer()); + } + @Override public SimulatedPlayer2 copy() { return new SimulatedPlayer2(this); diff --git a/Mage.Tests/src/test/java/org/mage/test/multiplayer/BloodchiefAscensionTest.java b/Mage.Tests/src/test/java/org/mage/test/multiplayer/BloodchiefAscensionTest.java index 6227f3fed13..eb63770e62d 100644 --- a/Mage.Tests/src/test/java/org/mage/test/multiplayer/BloodchiefAscensionTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/multiplayer/BloodchiefAscensionTest.java @@ -1,4 +1,3 @@ - package org.mage.test.multiplayer; import mage.constants.PhaseStep; @@ -9,13 +8,12 @@ import org.junit.Test; import org.mage.test.serverside.base.CardTestMultiPlayerBase; /** - * - * @author LevelX2 + * @author LevelX2, JayDi85 */ public class BloodchiefAscensionTest extends CardTestMultiPlayerBase { @Test - public void testBloodchiefAscensionAllPlayers() { + public void test_BloodchiefAscension_AllAlive() { // Enchantment // At the beginning of each end step, if an opponent lost 2 or more life this turn, you may put a quest counter on Bloodchief Ascension. (Damage causes loss of life.) // Whenever a card is put into an opponent's graveyard from anywhere, if Bloodchief Ascension has three or more quest counters on it, you may have that player lose 2 life. If you do, you gain 2 life. @@ -30,19 +28,41 @@ public class BloodchiefAscensionTest extends CardTestMultiPlayerBase { addCard(Zone.BATTLEFIELD, playerB, "Mountain", 3); addCard(Zone.HAND, playerB, "Fireball"); + // Player order: A -> D -> C -> B + runCode("before", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> { + // make sure range used + Assert.assertTrue(playerA.hasPlayerInRange(playerA.getId())); + Assert.assertTrue(playerA.hasPlayerInRange(playerD.getId())); + Assert.assertFalse(playerA.hasPlayerInRange(playerC.getId())); + Assert.assertTrue(playerA.hasPlayerInRange(playerB.getId())); + }); + + // turn 1 + // damage itself, no quest counters gain castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Fireball", playerA); setChoice(playerA, "X=2"); + setChoice(playerA, true); // put quest counter on end of turn 2 + // turn 2 + checkPermanentCounters("after turn 1", 2, PhaseStep.UPKEEP, playerA, "Bloodchief Ascension", CounterType.QUEST, 0); + // damage opponent, +1 quest counter castSpell(2, PhaseStep.PRECOMBAT_MAIN, playerD, "Fireball", playerD); setChoice(playerD, "X=2"); + // turn 3 + checkPermanentCounters("after turn 2", 3, PhaseStep.UPKEEP, playerA, "Bloodchief Ascension", CounterType.QUEST, 0 + 1); + // damage out of range player, no quest counter castSpell(3, PhaseStep.PRECOMBAT_MAIN, playerC, "Fireball", playerC); setChoice(playerC, "X=2"); + // turn 4 + checkPermanentCounters("after turn 3", 4, PhaseStep.UPKEEP, playerA, "Bloodchief Ascension", CounterType.QUEST, 0 + 1 + 0); + // damage opponent, +1 quest counter castSpell(4, PhaseStep.PRECOMBAT_MAIN, playerB, "Fireball", playerB); setChoice(playerB, "X=2"); + setChoice(playerA, true); // put quest counter on end of turn 4 - // Player order: A -> D -> C -> B + setStrictChooseMode(true); setStopAt(4, PhaseStep.END_TURN); execute(); @@ -65,7 +85,7 @@ public class BloodchiefAscensionTest extends CardTestMultiPlayerBase { * play. I took lethal damage on my turn, but they didn't get a counter on * Bloodchief Ascension at my end step. I think they should, even though I * had left the game from dying, because of: - * + *

* 800.4g. If a player leaves the game during their turn, that turn * continues to its completion without an active player. If the active * player would receive priority, instead the next player in turn order @@ -73,7 +93,7 @@ public class BloodchiefAscensionTest extends CardTestMultiPlayerBase { * or step ends, whichever is appropriate. */ @Test - public void testBloodchiefAscension() { + public void test_BloodchiefAscension_DieBeforeEndTurn() { // Enchantment // At the beginning of each end step, if an opponent lost 2 or more life this turn, you may put a quest counter on Bloodchief Ascension. (Damage causes loss of life.) // Whenever a card is put into an opponent's graveyard from anywhere, if Bloodchief Ascension has three or more quest counters on it, you may have that player lose 2 life. If you do, you gain 2 life. @@ -87,19 +107,42 @@ public class BloodchiefAscensionTest extends CardTestMultiPlayerBase { addCard(Zone.BATTLEFIELD, playerB, "Mountain", 21); addCard(Zone.HAND, playerB, "Fireball"); + // Player order: A -> D -> C -> B + runCode("before", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> { + // make sure range used + Assert.assertTrue(playerA.hasPlayerInRange(playerA.getId())); + Assert.assertTrue(playerA.hasPlayerInRange(playerD.getId())); + Assert.assertFalse(playerA.hasPlayerInRange(playerC.getId())); + Assert.assertTrue(playerA.hasPlayerInRange(playerB.getId())); + + }); + + // turn 1 + // damage itself, no quest counters gain castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Fireball", playerA); setChoice(playerA, "X=2"); + setChoice(playerA, true); // put quest counter on end of turn 2 + // turn 2 + checkPermanentCounters("after turn 1", 2, PhaseStep.UPKEEP, playerA, "Bloodchief Ascension", CounterType.QUEST, 0); + // damage opponent, +1 quest counter castSpell(2, PhaseStep.PRECOMBAT_MAIN, playerD, "Fireball", playerD); setChoice(playerD, "X=2"); + // turn 3 + checkPermanentCounters("after turn 2", 3, PhaseStep.UPKEEP, playerA, "Bloodchief Ascension", CounterType.QUEST, 0 + 1); + // damage out of range player, no quest counter castSpell(3, PhaseStep.PRECOMBAT_MAIN, playerC, "Fireball", playerC); setChoice(playerC, "X=2"); + // turn 4 + checkPermanentCounters("after turn 3", 4, PhaseStep.UPKEEP, playerA, "Bloodchief Ascension", CounterType.QUEST, 0 + 1 + 0); + // opponent kills itself, turn continue to the end and must give +1 counters castSpell(4, PhaseStep.PRECOMBAT_MAIN, playerB, "Fireball", playerB); setChoice(playerB, "X=20"); + setChoice(playerA, true); // put quest counter on end of turn 4 - // Player order: A -> D -> C -> B + setStrictChooseMode(true); setStopAt(4, PhaseStep.END_TURN); execute(); diff --git a/Mage.Tests/src/test/java/org/mage/test/multiplayer/PlayersListAndOrderTest.java b/Mage.Tests/src/test/java/org/mage/test/multiplayer/PlayersListAndOrderTest.java new file mode 100644 index 00000000000..0e4d6bed1f1 --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/multiplayer/PlayersListAndOrderTest.java @@ -0,0 +1,209 @@ +package org.mage.test.multiplayer; + +import mage.constants.MultiplayerAttackOption; +import mage.constants.PhaseStep; +import mage.constants.RangeOfInfluence; +import mage.game.FreeForAll; +import mage.game.Game; +import mage.game.GameException; +import mage.game.mulligan.MulliganType; +import mage.players.Player; +import mage.players.PlayerList; +import org.junit.Assert; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestMultiPlayerBase; + +import java.io.FileNotFoundException; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +/** + * @author JayDi85 + */ +public class PlayersListAndOrderTest extends CardTestMultiPlayerBase { + + // problem: some player's list can return random/unstable order (HashSet or HashMap instead linked list) + // so make sure all relates players list return stable/linked collection, not random + // also make sure some lists return in turn order + // TODO: add reverse turn order support from Aeon Engine + private List needByAddedAll = new ArrayList<>(); + private List needByApnapAllFromA = new ArrayList<>(); + private List needByApnapAllFromB = new ArrayList<>(); + private List needByApnapOpponentsFromA = new ArrayList<>(); + private List needByApnapOpponentsFromB = new ArrayList<>(); + + @Override + protected Game createNewGameAndPlayers() throws GameException, FileNotFoundException { + // Start Life = 2 + Game game = new FreeForAll(MultiplayerAttackOption.MULTIPLE, RangeOfInfluence.ALL, MulliganType.GAME_DEFAULT.getMulligan(0), 2, 7); + // Player order: A -> D -> C -> B + playerA = createPlayer(game, "PlayerA"); + playerB = createPlayer(game, "PlayerB"); + playerC = createPlayer(game, "PlayerC"); + playerD = createPlayer(game, "PlayerD"); + + needByAddedAll.addAll(Arrays.asList( + playerA.getId(), + playerB.getId(), + playerC.getId(), + playerD.getId() + )); + needByApnapAllFromA.addAll(Arrays.asList( + playerA.getId(), + playerD.getId(), + playerC.getId(), + playerB.getId() + )); + needByApnapAllFromB.addAll(Arrays.asList( + playerB.getId(), + playerA.getId(), + playerD.getId(), + playerC.getId() + )); + needByApnapOpponentsFromA.addAll(Arrays.asList( + playerD.getId(), + playerC.getId(), + playerB.getId() + )); + needByApnapOpponentsFromB.addAll(Arrays.asList( + playerA.getId(), + playerD.getId(), + playerC.getId() + )); + + return game; + } + + private void assertPlayersListFromCollection(String info, List need, Collection source) { + List byArray = new ArrayList<>(source); + assertPlayersList("toArray - " + info, need, byArray); + + // warning, do not optimize code here by IDE - it must use stream/iterator, not toArray (toArray called in addAll or constructor); + List byIterator = new ArrayList<>(source).stream().collect(Collectors.toList()); + assertPlayersList("iterator - " + info, need, byIterator); + } + + private void assertPlayersList(String info, List need, List current) { + int maxCheckCount = 100; + IntStream.rangeClosed(1, maxCheckCount).forEach(i -> { + if (!need.equals(current)) { + List needPlayers = need.stream().map(id -> currentGame.getPlayer(id)).collect(Collectors.toList()); + List currentPlayers = current.stream().map(id -> currentGame.getPlayer(id)).collect(Collectors.toList()); + Assert.fail(info + "\n" + "need: " + needPlayers + "\n" + "find: " + currentPlayers); + } + }); + } + + @Test + public void test_Game_GetPlayers() { + // game.getPlayers() - added order, for inner game engine only + + runCode("check", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> { + List byValues = currentGame.getPlayers().values() + .stream() + .map(Player::getId) + .collect(Collectors.toList()); + assertPlayersList("game.getPlayers().values() - must return in added order", needByAddedAll, byValues); + + List byKeys = new ArrayList<>(currentGame.getPlayers().keySet()); + assertPlayersList("game.getPlayers().keySet() - must return in added order", needByAddedAll, byKeys); + }); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + } + + @Test + public void test_Game_GetPlayerList() { + // game.getPlayerList() - APNAP order, for cards usage + + PlayerList list = new PlayerList(); + list.add(playerA.getId()); + list.add(playerB.getId()); + list.add(playerC.getId()); + list.add(playerD.getId()); + list.setCurrent(playerA.getId()); + List current = new ArrayList<>(list); + assertPlayersList("for fast debug", needByApnapAllFromA, current); + + runCode("check", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> { + assertPlayersListFromCollection( + "game.getPlayerList() - must return in APNAP order", + needByApnapAllFromA, + currentGame.getPlayerList() + ); + }); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + } + + @Test + public void test_Game_GetOpponents() { + // game.getOpponents() - APNAP order, for cards usage + + runCode("check", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> { + assertPlayersListFromCollection( + "game.getOpponents(from player A) - must return in APNAP order", + needByApnapOpponentsFromA, + currentGame.getOpponents(playerA.getId()) + ); + assertPlayersListFromCollection( + "game.getOpponents(from player B) - must return in APNAP order", + needByApnapOpponentsFromB, + currentGame.getOpponents(playerB.getId()) + ); + }); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + } + + @Test + public void test_GameState_GetPlayerList() { + // game.getState().getPlayerList() - APNAP order, for cards usage + + runCode("check", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> { + assertPlayersListFromCollection( + "game.getState().getPlayerList(from current player) - must return in APNAP order", + needByApnapAllFromA, + currentGame.getState().getPlayerList() + ); + assertPlayersListFromCollection( + "game.getState().getPlayerList(from player B) - must return in APNAP order", + needByApnapAllFromB, + currentGame.getState().getPlayerList(playerB.getId()) + ); + }); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + } + + @Test + public void test_GameState_GetPlayersInRange() { + // game.getState().getPlayersInRange(player, game) - APNAP order, for cards usage + + runCode("check", 1, PhaseStep.PRECOMBAT_MAIN, playerA, (info, player, game) -> { + assertPlayersListFromCollection( + "game.getState().getPlayersInRange(from player A) - must return in APNAP order", + needByApnapAllFromA, + currentGame.getState().getPlayersInRange(playerA.getId(), currentGame) + ); + assertPlayersListFromCollection( + "game.getState().getPlayersInRange(from player B) - must return in APNAP order", + needByApnapAllFromB, + currentGame.getState().getPlayersInRange(playerB.getId(), currentGame) + ); + }); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + } +} diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index 7b88af5c249..986b497bc7d 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -54,7 +54,6 @@ import mage.util.MultiAmountMessage; import mage.util.RandomUtil; import org.apache.log4j.Logger; import org.junit.Assert; -import org.junit.Ignore; import java.io.Serializable; import java.util.*; @@ -69,7 +68,6 @@ import static org.mage.test.serverside.base.impl.CardTestPlayerAPIImpl.*; * * @author BetaSteward_at_googlemail.com, Simown, JayDi85 */ -@Ignore public class TestPlayer implements Player { private static final Logger LOGGER = Logger.getLogger(TestPlayer.class); @@ -90,6 +88,8 @@ public class TestPlayer implements Player { private int maxCallsWithoutAction = 400; private int foundNoAction = 0; + // warning, test player do not restore own data by game rollback + // full playable AI, TODO: can be deleted? private boolean AIPlayer; // AI simulates a real game, e.g. ignores strict mode and play command/priority, see aiXXX commands @@ -2600,7 +2600,7 @@ public class TestPlayer implements Player { targetCardZonesChecked.add(Zone.GRAVEYARD); TargetCard targetFull = (TargetCard) target.getOriginalTarget(); - List needPlayers = game.getState().getPlayersInRange(this.getId(), game).toList(); + List needPlayers = new ArrayList<>(game.getState().getPlayersInRange(this.getId(), game)); // fix for opponent graveyard if (target.getOriginalTarget() instanceof TargetCardInOpponentsGraveyard) { // current player remove @@ -2947,8 +2947,12 @@ public class TestPlayer implements Player { @Override public void restore(Player player) { - // no rollback for test player meta data (modesSet, actions, choices, targets, aliases) - computerPlayer.restore(player); + if (!(player instanceof TestPlayer)) { + throw new IllegalArgumentException("Wrong code usage: can't restore from player class " + player.getClass().getName()); + } + + // no rollback for test player metadata (modesSet, actions, choices, targets, aliases, etc) + computerPlayer.restore(player.getRealPlayer()); } @Override @@ -2984,8 +2988,8 @@ public class TestPlayer implements Player { } @Override - public Set getInRange() { - return computerPlayer.getInRange(); + public boolean hasPlayerInRange(UUID checkingPlayerId) { + return computerPlayer.hasPlayerInRange(checkingPlayerId); } @Override @@ -4619,4 +4623,9 @@ public class TestPlayer implements Player { //Assert.fail("Wrong choice command: " + choice); LOGGER.warn("Wrong choice command: " + choice); } + + @Override + public Player getRealPlayer() { + return this.computerPlayer; + } } diff --git a/Mage/src/main/java/mage/game/Game.java b/Mage/src/main/java/mage/game/Game.java index 05c231a4997..2c2d713c7ae 100644 --- a/Mage/src/main/java/mage/game/Game.java +++ b/Mage/src/main/java/mage/game/Game.java @@ -158,31 +158,42 @@ public interface Game extends MageItem, Serializable, Copyable { Player getPlayerOrPlaneswalkerController(UUID playerId); + /** + * Static players list from start of the game. Use it to find player by ID. + */ Players getPlayers(); + /** + * Static players list from start of the game. Use it to interate by starting turn order. + * WARNING, it's ignore range and leaved players, so use it game engine only + */ PlayerList getPlayerList(); + /** + * Returns opponents list in range for the given playerId. Use it to interate by starting turn order. + * Warning, it will return dead players until end of turn. + */ default Set getOpponents(UUID playerId) { return getOpponents(playerId, false); } /** - * Returns a Set of opponents in range for the given playerId + * Returns opponents list in range for the given playerId. Use it to interate by starting turn order. + * Warning, it will return dead players until end of turn. * - * @param playerId - * @param excludeDeadPlayers Determines if players who have lost are excluded from the list - * @return + * @param excludeDeadPlayers exclude dead player immediately without waiting range update on next turn */ default Set getOpponents(UUID playerId, boolean excludeDeadPlayers) { Player player = getPlayer(playerId); if (player == null) { - return new HashSet<>(); + return new LinkedHashSet<>(); } - return player.getInRange().stream() + return this.getPlayerList().stream() .filter(opponentId -> !opponentId.equals(playerId)) + .filter(player::hasPlayerInRange) .filter(opponentId -> !excludeDeadPlayers || !getPlayer(opponentId).hasLost()) - .collect(Collectors.toSet()); + .collect(Collectors.toCollection(LinkedHashSet::new)); } default boolean isActivePlayer(UUID playerId) { diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 6b9e0481490..6ea8dc969b1 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -3183,8 +3183,6 @@ public abstract class GameImpl implements Game { /** * Return a list of all players ignoring the range of visible players - * - * @return */ @Override public PlayerList getPlayerList() { diff --git a/Mage/src/main/java/mage/game/GameState.java b/Mage/src/main/java/mage/game/GameState.java index ec7e564239f..ae70b465bac 100644 --- a/Mage/src/main/java/mage/game/GameState.java +++ b/Mage/src/main/java/mage/game/GameState.java @@ -62,8 +62,8 @@ public class GameState implements Serializable, Copyable { // warning, do not use another keys with same starting text cause copy code search and clean all related values public static final String COPIED_CARD_KEY = "CopiedCard"; - private final Players players; - private final PlayerList playerList; + private final Players players; // full players by ID (static list, table added order) + private final PlayerList playerList; // full players (static list, turn order e.g. apnap) private UUID choosingPlayerId; // player that makes a choice at game start // revealed cards >, will be reset if all players pass priority @@ -719,8 +719,6 @@ public class GameState implements Serializable, Copyable { /** * Returns a list of all players of the game ignoring range or if a player * has lost or left the game. - * - * @return playerList */ public PlayerList getPlayerList() { return playerList; @@ -761,8 +759,9 @@ public class GameState implements Serializable, Copyable { PlayerList newPlayerList = new PlayerList(); Player currentPlayer = game.getPlayer(playerId); if (currentPlayer != null) { + // must fill PlayerList by table added order (same as main game) for (Player player : players.values()) { - if (player.isInGame() && currentPlayer.getInRange().contains(player.getId())) { + if (player.isInGame() && currentPlayer.hasPlayerInRange(player.getId())) { newPlayerList.add(player.getId()); } } diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index 5bbf056c2de..5ea3418c940 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -296,7 +296,11 @@ public interface Player extends MageItem, Copyable { ManaPool getManaPool(); - Set getInRange(); + /** + * Is checking player in range of current player. + * Warning, range list updates on start of the turn due rules. + */ + boolean hasPlayerInRange(UUID checkingPlayerId); boolean isTopCardRevealed(); @@ -1250,4 +1254,10 @@ public interface Player extends MageItem, Copyable { } public UserData getControllingPlayersUserData(Game game); + + /** + * Some player implementations (TestPlayer, Simulated) uses facade structure with hidden player object, + * so that's method helps to find real player that used by a game (in most use cases it's a PlayerImpl) + */ + Player getRealPlayer(); } diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index cfcbffae5ea..65ba60279aa 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -146,7 +146,7 @@ public abstract class PlayerImpl implements Player, Serializable { protected boolean idleTimeout; protected RangeOfInfluence range; - protected Set inRange = new HashSet<>(); // players list in current range of influence (updates each turn) + protected Set inRange = new HashSet<>(); // players list in current range of influence (updates each turn due rules) protected boolean isTestMode = false; protected boolean canGainLife = true; @@ -309,6 +309,10 @@ public abstract class PlayerImpl implements Player, Serializable { */ @Override public void restore(Player player) { + if (!(player instanceof PlayerImpl)) { + throw new IllegalArgumentException("Wrong code usage: can't restore from player class " + player.getClass().getName()); + } + this.name = player.getName(); this.human = player.isHuman(); this.life = player.getLife(); @@ -357,7 +361,7 @@ public abstract class PlayerImpl implements Player, Serializable { this.attachments.addAll(player.getAttachments()); this.inRange.clear(); - this.inRange.addAll(player.getInRange()); + this.inRange.addAll(((PlayerImpl) player).inRange); this.payLifeCostLevel = player.getPayLifeCostLevel(); this.sacrificeCostFilter = player.getSacrificeCostFilter() != null ? player.getSacrificeCostFilter().copy() : null; @@ -579,16 +583,16 @@ public abstract class PlayerImpl implements Player, Serializable { } @Override - public Set getInRange() { + public boolean hasPlayerInRange(UUID checkingPlayerId) { if (inRange.isEmpty()) { // runtime check: inRange filled on beginTurn, but unit tests adds cards by cheat engine before game starting, // so inRange will be empty and some ETB effects can be broken (example: Spark Double puts direct to battlefield). // Cheat engine already have a workaround, so that error must not be visible in normal situation. // TODO: that's error possible on GameView call before real game start (too laggy players on starting???) - throw new IllegalStateException("Wrong code usage (game is not started, but you call getInRange in some effects)."); + throw new IllegalStateException("Wrong code usage (game is not started, but you call hasPlayerInRange in some effects)."); } - return inRange; + return inRange.contains(checkingPlayerId); } @Override @@ -5130,7 +5134,7 @@ public abstract class PlayerImpl implements Player, Serializable { public boolean hasOpponent(UUID playerToCheckId, Game game) { return !this.getId().equals(playerToCheckId) && game.isOpponent(this, playerToCheckId) - && getInRange().contains(playerToCheckId); + && hasPlayerInRange(playerToCheckId); } @Override @@ -5470,4 +5474,9 @@ public abstract class PlayerImpl implements Player, Serializable { public String toString() { return getName() + " (" + super.getClass().getSimpleName() + ")"; } + + @Override + public Player getRealPlayer() { + return this; + } } diff --git a/Mage/src/main/java/mage/players/PlayerList.java b/Mage/src/main/java/mage/players/PlayerList.java index f96d39470a4..139167e50e5 100644 --- a/Mage/src/main/java/mage/players/PlayerList.java +++ b/Mage/src/main/java/mage/players/PlayerList.java @@ -27,7 +27,7 @@ public class PlayerList extends CircularList { UUID currentPlayerBefore = this.get(); UUID nextPlayerId = game.isTurnOrderReversed() ? super.getPrevious() : super.getNext(); do { - if (basePlayer.getInRange().contains(nextPlayerId)) { + if (basePlayer.hasPlayerInRange(nextPlayerId)) { return game.getPlayer(nextPlayerId); } nextPlayerId = game.isTurnOrderReversed() ? super.getPrevious() : super.getNext(); diff --git a/Mage/src/main/java/mage/util/CircularList.java b/Mage/src/main/java/mage/util/CircularList.java index d6bd6753390..5fb6a09c6d6 100644 --- a/Mage/src/main/java/mage/util/CircularList.java +++ b/Mage/src/main/java/mage/util/CircularList.java @@ -1,20 +1,13 @@ - package mage.util; import java.io.Serializable; -import java.util.ArrayList; -import java.util.Collection; -import java.util.ConcurrentModificationException; -import java.util.Iterator; -import java.util.List; -import java.util.ListIterator; +import java.util.*; import java.util.concurrent.locks.ReentrantLock; /** - * a thread-safe circular list + * Component: a thread-safe circular list, used for players list * - * @author BetaSteward_at_googlemail.com - * @param + * @author BetaSteward_at_googlemail.com, JayDi85 */ public class CircularList implements List, Iterable, Serializable { //TODO: might have to make E extend Copyable @@ -42,17 +35,17 @@ public class CircularList implements List, Iterable, Serializable { } /** - * Inserts an element into the current position - * - * @param e - * @return + * Insert new element at the current position (make new element as current) */ @Override - public boolean add(E e) { - list.add(this.index, e); + public boolean add(E element) { + add(this.index, element); return true; } + /** + * Insert new element at selected position (keep old element as current) + */ @Override public void add(int index, E element) { lock.lock(); @@ -65,35 +58,36 @@ public class CircularList implements List, Iterable, Serializable { } /** + * Set current position to an element * - * @param e the element to set as current - * @return true if element e exists and index was set + * @return false on unknown element */ - public boolean setCurrent(E e) { - if (list.contains(e)) { - this.index = list.indexOf(e); + public boolean setCurrent(E element) { + if (list.contains(element)) { + this.index = list.indexOf(element); return true; } return false; } /** - * Retrieves the element at the current position - * - * @return + * Find current element */ public E get() { + return get(this.index); + } + + /** + * Find element at searching position + */ + @Override + public E get(int index) { if (list.size() > this.index) { return list.get(this.index); } return null; } - @Override - public E get(int index) { - return list.get(index); - } - /** * Returns the next element in the list. Will loop around to the beginning * of the list if the current element is the last. @@ -117,18 +111,23 @@ public class CircularList implements List, Iterable, Serializable { /** * Removes the current element from the list * - * @return true is the item was successfully removed + * @return true on successfully removed */ public boolean remove() { return this.remove(get()); } + /** + * Removes element from searching position + * + * @return true on successfully removed + */ @Override public E remove(int index) { lock.lock(); try { E ret = list.remove(index); - checkPointer(); + fixInvalidPointer(); modCount++; return ret; } finally { @@ -141,7 +140,7 @@ public class CircularList implements List, Iterable, Serializable { lock.lock(); try { boolean ret = list.remove(o); - checkPointer(); + fixInvalidPointer(); modCount++; return ret; } finally { @@ -152,50 +151,49 @@ public class CircularList implements List, Iterable, Serializable { private int incrementPointer() { lock.lock(); try { - index = incrementListPointer(index); + index = findNextListPointer(index); return index; } finally { lock.unlock(); } } - private int incrementListPointer(int index) { - index++; - if (index >= list.size()) { - index = 0; + private int findNextListPointer(int currentIndex) { + currentIndex++; + if (currentIndex >= list.size()) { + currentIndex = 0; } - return index; + return currentIndex; } private int decrementPointer() { lock.lock(); try { - index = decrementListPointer(index); + index = findPrevListPointer(index); return index; } finally { lock.unlock(); } } - private int decrementListPointer(int index) { - index--; - if (index < 0) { - index = list.size() - 1; + private int findPrevListPointer(int currentIndex) { + currentIndex--; + if (currentIndex < 0) { + currentIndex = list.size() - 1; } - return index; + return currentIndex; } /** * This method should only be called from a locked method thus it is not * necessary to lock from this method */ - private int checkPointer() { + private void fixInvalidPointer() { if (index > list.size()) { index = list.size() - 1; } else if (index < 0) { index = 0; } - return index; } @Override @@ -215,16 +213,22 @@ public class CircularList implements List, Iterable, Serializable { @Override public Object[] toArray() { - return list.toArray(); + return toArray(new UUID[0]); } @Override public T[] toArray(T[] a) { - return list.toArray(a); - } + T[] res = list.toArray(a); - public List toList() { - return list; + // sort due default order + Iterator iter = this.iterator(); + int insertIndex = 0; + while (iter.hasNext()) { + res[insertIndex] = (T) iter.next(); + insertIndex++; + } + + return res; } @Override @@ -254,7 +258,7 @@ public class CircularList implements List, Iterable, Serializable { try { boolean ret = list.removeAll(c); modCount++; - checkPointer(); + fixInvalidPointer(); return ret; } finally { lock.unlock(); @@ -267,7 +271,7 @@ public class CircularList implements List, Iterable, Serializable { try { boolean ret = list.retainAll(c); modCount++; - checkPointer(); + fixInvalidPointer(); return ret; } finally { lock.unlock(); @@ -286,6 +290,10 @@ public class CircularList implements List, Iterable, Serializable { } } + public E set(E element) { + return this.set(this.index, element); + } + @Override public E set(int index, E element) { lock.lock(); @@ -297,10 +305,6 @@ public class CircularList implements List, Iterable, Serializable { } } - public E set(E element) { - return this.set(this.index, element); - } - @Override public int indexOf(Object o) { return list.indexOf(o); @@ -361,7 +365,7 @@ public class CircularList implements List, Iterable, Serializable { throw new ConcurrentModificationException(); } E data = (E) list.get(cursor); - cursor = incrementListPointer(cursor); + cursor = findNextListPointer(cursor); hasMoved = true; return data; } @@ -409,7 +413,7 @@ public class CircularList implements List, Iterable, Serializable { throw new ConcurrentModificationException(); } E data = (E) list.get(cursor); - cursor = incrementListPointer(cursor); + cursor = findNextListPointer(cursor); hasMoved = true; return data; } @@ -430,7 +434,7 @@ public class CircularList implements List, Iterable, Serializable { if (curModCount != modCount) { throw new ConcurrentModificationException(); } - cursor = decrementListPointer(cursor); + cursor = findPrevListPointer(cursor); hasMoved = true; return (E) list.get(cursor); } @@ -438,7 +442,7 @@ public class CircularList implements List, Iterable, Serializable { @Override public int nextIndex() { if (this.hasNext()) { - return incrementListPointer(cursor); + return findNextListPointer(cursor); } return list.size(); } @@ -446,7 +450,7 @@ public class CircularList implements List, Iterable, Serializable { @Override public int previousIndex() { if (this.hasPrevious()) { - return decrementListPointer(cursor); + return findPrevListPointer(cursor); } return -1; }