From 0134f15d18fc57affc8da7369d5d2b0deb021ce4 Mon Sep 17 00:00:00 2001 From: Pacien TRAN-GIRARD Date: Mon, 25 Apr 2016 14:41:58 +0200 Subject: New code review and refactoring --- src/ch/epfl/xblast/server/painter/BlockImage.java | 6 +- .../epfl/xblast/server/painter/BoardPainter.java | 25 ++++--- .../xblast/server/painter/ExplosionPainter.java | 32 ++++---- .../epfl/xblast/server/painter/PlayerPainter.java | 86 ++++++++++++++++------ 4 files changed, 102 insertions(+), 47 deletions(-) (limited to 'src') diff --git a/src/ch/epfl/xblast/server/painter/BlockImage.java b/src/ch/epfl/xblast/server/painter/BlockImage.java index d5fa593..85a44be 100644 --- a/src/ch/epfl/xblast/server/painter/BlockImage.java +++ b/src/ch/epfl/xblast/server/painter/BlockImage.java @@ -1,7 +1,10 @@ package ch.epfl.xblast.server.painter; /** - * @author Timothée FLOURE (257420 + * Block images enumeration. + * + * @author Timothée FLOURE (257420) + * @author Pacien TRAN-GIRARD (261948) */ public enum BlockImage { @@ -39,4 +42,5 @@ public enum BlockImage { * Bomb Range Bonus. */ BONUS_RANGE + } diff --git a/src/ch/epfl/xblast/server/painter/BoardPainter.java b/src/ch/epfl/xblast/server/painter/BoardPainter.java index 69af129..8378cce 100644 --- a/src/ch/epfl/xblast/server/painter/BoardPainter.java +++ b/src/ch/epfl/xblast/server/painter/BoardPainter.java @@ -1,12 +1,18 @@ package ch.epfl.xblast.server.painter; -import ch.epfl.xblast.*; -import ch.epfl.xblast.server.*; +import ch.epfl.xblast.Cell; +import ch.epfl.xblast.Direction; +import ch.epfl.xblast.server.Block; +import ch.epfl.xblast.server.Board; +import java.util.Collections; import java.util.Map; /** + * Board painter. + * * @author Timothée FLOURE (257420) + * @author Pacien TRAN-GIRARD (261948) */ public final class BoardPainter { @@ -16,11 +22,11 @@ public final class BoardPainter { /** * Instantiates a new BoardPainter. * - * @param blocksMap map linking block to the images + * @param blocksMap map linking block to the images * @param shadowedFreeBlock a "shadowed" block image */ - public BoardPainter(Map blocksMap, BlockImage shadowedFreeBlock ) { - this.blocksMap = blocksMap; + public BoardPainter(Map blocksMap, BlockImage shadowedFreeBlock) { + this.blocksMap = Collections.unmodifiableMap(blocksMap); this.shadowedFreeBlock = shadowedFreeBlock; } @@ -28,13 +34,14 @@ public final class BoardPainter { * Returns the bits sequence of the block related to the given Cell. * * @param board given board - * @param cell given cell - * @return the bits sequence related to the image of the given cell + * @param cell given cell + * @return the bit sequence related to the image of the given cell */ public byte byteForCell(Board board, Cell cell) { if (board.blockAt(cell).isFree() && board.blockAt(cell.neighbor(Direction.W)).castsShadow()) return (byte) shadowedFreeBlock.ordinal(); - - return (byte) this.blocksMap.get(board.blockAt(cell)).ordinal(); + else + return (byte) this.blocksMap.get(board.blockAt(cell)).ordinal(); } + } diff --git a/src/ch/epfl/xblast/server/painter/ExplosionPainter.java b/src/ch/epfl/xblast/server/painter/ExplosionPainter.java index 63181e9..756d3c4 100644 --- a/src/ch/epfl/xblast/server/painter/ExplosionPainter.java +++ b/src/ch/epfl/xblast/server/painter/ExplosionPainter.java @@ -1,11 +1,15 @@ package ch.epfl.xblast.server.painter; -import ch.epfl.xblast.server.*; +import ch.epfl.xblast.server.Bomb; /** + * Explosion painter. + * * @author Timothée FLOURE (257420) + * @author Pacien TRAN-GIRARD (261948) */ public final class ExplosionPainter { + private static final byte BLACK_BOMB_IMAGE = 20; private static final byte WHITE_BOMB_IMAGE = 21; @@ -15,41 +19,37 @@ public final class ExplosionPainter { private static final byte WEST_EXPLOSION = 0b0001; /** - * Return the image corresponding to a bomb depending on its fuse length. + * Returns the image corresponding to a bomb depending on its fuse length. * * @param bomb given bomb * @return the byte related to the image of the bomb (black or white) */ - public static byte byteForBomb(Bomb bomb) - { - if (Integer.bitCount(bomb.fuseLength()) == 1 ) { - return WHITE_BOMB_IMAGE; - } else { - return BLACK_BOMB_IMAGE; - } + public static byte byteForBomb(Bomb bomb) { + return Integer.bitCount(bomb.fuseLength()) == 1 ? WHITE_BOMB_IMAGE : BLACK_BOMB_IMAGE; } /** * Return the image corresponding to the explosion on a cell given the explosions of its neighbors. * * @param north is an explosion on the north cell - * @param east is an explosion on the east cell + * @param east is an explosion on the east cell * @param south is an explosion on the south cell - * @param west is an explosion on the west cell + * @param west is an explosion on the west cell * @return the byte corresponding to the related image */ public static byte byteForBlast(boolean north, boolean east, boolean south, boolean west) { - byte correspondingByte = 0b0; + byte correspondingByte = 0b0000; if (north) - correspondingByte += NORTH_EXPLOSION; + correspondingByte |= NORTH_EXPLOSION; if (east) - correspondingByte += EAST_EXPLOSION; + correspondingByte |= EAST_EXPLOSION; if (south) - correspondingByte += SOUTH_EXPLOSION; + correspondingByte |= SOUTH_EXPLOSION; if (west) - correspondingByte += WEST_EXPLOSION; + correspondingByte |= WEST_EXPLOSION; return correspondingByte; } + } diff --git a/src/ch/epfl/xblast/server/painter/PlayerPainter.java b/src/ch/epfl/xblast/server/painter/PlayerPainter.java index 4f28554..043cb50 100644 --- a/src/ch/epfl/xblast/server/painter/PlayerPainter.java +++ b/src/ch/epfl/xblast/server/painter/PlayerPainter.java @@ -1,44 +1,88 @@ package ch.epfl.xblast.server.painter; -import ch.epfl.xblast.server.*; +import ch.epfl.xblast.Direction; +import ch.epfl.xblast.PlayerID; +import ch.epfl.xblast.server.Player; /** + * Player painter. + * * @author Timothée FLOURE (257420) + * @author Pacien TRAN-GIRARD (261948) */ public final class PlayerPainter { - private static final byte DEAD_PLAYER = 16; + + private static final byte DEAD_PLAYER_IMAGE_ID = 16; private static final byte DYING_IMAGE_ID = 12; private static final byte LAST_DYING_IMAGE_ID = 13; - private static final byte PLAYER_MULTIPLIER = 20; + + private static final int PLAYER_MULTIPLIER = 20; + private static final int DIRECTION_MULTIPLIER = 3; + + private static final int ANIMATION_TICK_LOOP = 4; + + /** + * Computes and returns the animation frame byte for the given tick. + * + * @param tick the tick + * @return the frame byte + */ + private static byte byteForFrame(int tick) { + int cycleTick = tick % ANIMATION_TICK_LOOP; + return (byte) (cycleTick % 2 == 0 ? 0 : cycleTick / 2 + 1); + } + + /** + * Returns the byte for the given Direction. + * + * @param d the Direction + * @return the directional byte + */ + private static byte byteForDirection(Direction d) { + return (byte) (d.ordinal() * DIRECTION_MULTIPLIER); + } + + /** + * Returns the player image byte for the given PlayerID. + * + * @param pid the PlayerID + * @return the image byte for the player + */ + private static byte byteForPlayerID(PlayerID pid) { + return (byte) (pid.ordinal() * PLAYER_MULTIPLIER); + } + + /** + * Returns the image ID for the dying state according to the number of remaining lives. + * + * @param lives the number of remaining lives + * @return the dying image ID + */ + private static byte byteForDyingState(int lives) { + return lives == 0 ? LAST_DYING_IMAGE_ID : DYING_IMAGE_ID; + } /** * Returns the byte related to the image corresponding to the actual state of the given player. * * @param player the given player - * @param tick the actual tick of the game + * @param tick the actual tick of the game * @return the byte related to the image of the the actual state of the player */ public static byte byteForPlayer(Player player, int tick) { - if (!player.isAlive()) - return DEAD_PLAYER; + switch (player.lifeState().state()) { + case DEAD: + return DEAD_PLAYER_IMAGE_ID; - byte correspondingByte = (byte) (player.id().ordinal() * PLAYER_MULTIPLIER); + case DYING: + return (byte) (byteForPlayerID(player.id()) + + byteForDyingState(player.lives())); - if (player.lifeState().state() == Player.LifeState.State.DYING) - if (player.lives() > 0) - return (byte) (correspondingByte + DYING_IMAGE_ID); - else - return (byte) (correspondingByte + LAST_DYING_IMAGE_ID ); - - correspondingByte += player.direction().ordinal() * 3; - - switch (tick % 4) { // Magic Numbers !!! - case 1: - return (byte) (correspondingByte + 1); - case 3: - return (byte) (correspondingByte + 2); default: - return (byte) (correspondingByte + 0); + return (byte) (byteForPlayerID(player.id()) + + byteForDirection(player.direction()) + + byteForFrame(tick)); } } + } -- cgit v1.2.3 From a73178cc0c313b675d370672c6bd2a5b8d18d817 Mon Sep 17 00:00:00 2001 From: Pacien TRAN-GIRARD Date: Mon, 25 Apr 2016 14:57:21 +0200 Subject: Reinforce immutability --- src/ch/epfl/xblast/Lists.java | 23 ++++++++++++++++++++++ src/ch/epfl/xblast/SubCell.java | 2 +- src/ch/epfl/xblast/server/Board.java | 5 ++--- src/ch/epfl/xblast/server/Bomb.java | 8 ++++---- src/ch/epfl/xblast/server/GameState.java | 10 +++++----- .../epfl/xblast/server/painter/BoardPainter.java | 3 ++- 6 files changed, 37 insertions(+), 14 deletions(-) (limited to 'src') diff --git a/src/ch/epfl/xblast/Lists.java b/src/ch/epfl/xblast/Lists.java index 22d9d06..ef17f05 100644 --- a/src/ch/epfl/xblast/Lists.java +++ b/src/ch/epfl/xblast/Lists.java @@ -12,6 +12,29 @@ import java.util.stream.Stream; */ public final class Lists { + /** + * Returns an immutable copy of a list. + * + * @param l a list + * @param the element type + * @return the immutable list + */ + public static List immutableList(List l) { + return Collections.unmodifiableList(new ArrayList<>(l)); + } + + /** + * Returns an immutable copy of a map. + * + * @param m a mab + * @param the key type + * @param the value type + * @return the immutable copy + */ + public static Map immutableMap(Map m) { + return Collections.unmodifiableMap(new HashMap<>(m)); + } + /** * Returns a symmetric version of the list, without repeating the last element of the input list. * For instance, mirrored([kay]) will return [kayak]. diff --git a/src/ch/epfl/xblast/SubCell.java b/src/ch/epfl/xblast/SubCell.java index 51cbbb4..4278fab 100644 --- a/src/ch/epfl/xblast/SubCell.java +++ b/src/ch/epfl/xblast/SubCell.java @@ -31,7 +31,7 @@ public final class SubCell { /** * The coordinates of the SubCell. */ - private int x, y; + private final int x, y; /** * Instantiates a new SubCell with the given coordinates. diff --git a/src/ch/epfl/xblast/server/Board.java b/src/ch/epfl/xblast/server/Board.java index a18422a..4519781 100644 --- a/src/ch/epfl/xblast/server/Board.java +++ b/src/ch/epfl/xblast/server/Board.java @@ -4,7 +4,6 @@ import ch.epfl.cs108.Sq; import ch.epfl.xblast.Cell; import ch.epfl.xblast.Lists; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; @@ -41,7 +40,7 @@ public final class Board { /** * List containing all the blocks of the board. */ - private List> blocks; + private final List> blocks; /** * Instantiates a new Board with the given sequence of blocks. @@ -53,7 +52,7 @@ public final class Board { if (blocks == null || blocks.size() != BLOCKS_LIST_SIZE) throw new IllegalArgumentException(); - this.blocks = new ArrayList<>(blocks); + this.blocks = Lists.immutableList(blocks); } /** diff --git a/src/ch/epfl/xblast/server/Bomb.java b/src/ch/epfl/xblast/server/Bomb.java index 535573f..57c271f 100644 --- a/src/ch/epfl/xblast/server/Bomb.java +++ b/src/ch/epfl/xblast/server/Bomb.java @@ -28,22 +28,22 @@ public final class Bomb { /** * Owner of the Bomb. */ - private PlayerID ownerId; + private final PlayerID ownerId; /** * Position of the Bomb. */ - private Cell position; + private final Cell position; /** * Fuse of the Bomb. */ - private Sq fuseLengths; + private final Sq fuseLengths; /** * Range of the Bomb. */ - private int range; + private final int range; /** * Instantiates a new Bomb. diff --git a/src/ch/epfl/xblast/server/GameState.java b/src/ch/epfl/xblast/server/GameState.java index 44fd5d1..abd07b3 100644 --- a/src/ch/epfl/xblast/server/GameState.java +++ b/src/ch/epfl/xblast/server/GameState.java @@ -52,7 +52,7 @@ public final class GameState { * @param players list of the players */ public GameState(Board board, List players) { - this(0, board, players, new ArrayList<>(), new ArrayList<>(), new ArrayList<>()); + this(0, board, players, Collections.emptyList(), Collections.emptyList(), Collections.emptyList()); } /** @@ -72,11 +72,11 @@ public final class GameState { this.board = Objects.requireNonNull(board); if (players.size() != PlayerID.values().length) throw new IllegalArgumentException(); - this.players = new ArrayList<>(players); + this.players = Lists.immutableList(players); - this.bombs = new ArrayList<>(Objects.requireNonNull(bombs)); - this.explosions = new ArrayList<>(Objects.requireNonNull(explosions)); - this.blasts = new ArrayList<>(Objects.requireNonNull(blasts)); + this.bombs = Lists.immutableList(Objects.requireNonNull(bombs)); + this.explosions = Lists.immutableList(Objects.requireNonNull(explosions)); + this.blasts = Lists.immutableList(Objects.requireNonNull(blasts)); } /** diff --git a/src/ch/epfl/xblast/server/painter/BoardPainter.java b/src/ch/epfl/xblast/server/painter/BoardPainter.java index 8378cce..34d0974 100644 --- a/src/ch/epfl/xblast/server/painter/BoardPainter.java +++ b/src/ch/epfl/xblast/server/painter/BoardPainter.java @@ -2,6 +2,7 @@ package ch.epfl.xblast.server.painter; import ch.epfl.xblast.Cell; import ch.epfl.xblast.Direction; +import ch.epfl.xblast.Lists; import ch.epfl.xblast.server.Block; import ch.epfl.xblast.server.Board; @@ -26,7 +27,7 @@ public final class BoardPainter { * @param shadowedFreeBlock a "shadowed" block image */ public BoardPainter(Map blocksMap, BlockImage shadowedFreeBlock) { - this.blocksMap = Collections.unmodifiableMap(blocksMap); + this.blocksMap = Lists.immutableMap(blocksMap); this.shadowedFreeBlock = shadowedFreeBlock; } -- cgit v1.2.3