From 102e5f142eb5e8eb0c9633acf9c4b036189f438a Mon Sep 17 00:00:00 2001 From: Wizjany Date: Mon, 26 Sep 2011 04:38:37 -0400 Subject: [PATCH] Fix data cycling the right way. Merged TomyLobo's test changes in. --- .../com/sk89q/worldedit/data/BlockData.java | 54 ++++++++----- .../sk89q/worldedit/data/BlockDataTest.java | 75 ++++++++++++++++++- 2 files changed, 109 insertions(+), 20 deletions(-) diff --git a/src/main/java/com/sk89q/worldedit/data/BlockData.java b/src/main/java/com/sk89q/worldedit/data/BlockData.java index 5ccb16310..6e0e5448a 100644 --- a/src/main/java/com/sk89q/worldedit/data/BlockData.java +++ b/src/main/java/com/sk89q/worldedit/data/BlockData.java @@ -583,19 +583,24 @@ public final class BlockData { * @return the new data value for the block */ public static int cycle(int type, int data, int increment) { + if (increment != -1 && increment != 1) { + throw new IllegalArgumentException("Increment must be 1 or -1."); + } + int store; switch (type) { case BlockID.LOG: case BlockID.LONG_GRASS: case BlockID.STONE_BRICK: case BlockID.SILVERFISH_BLOCK: - return (data + increment) % 3; + if (data > 2) return -1; + return mod((data + increment), 3); case BlockID.TORCH: case BlockID.REDSTONE_TORCH_ON: case BlockID.REDSTONE_TORCH_OFF: if (data < 1 || data > 4) return -1; - return (data - 1 + increment) % 4 + 1; + return mod((data - 1 + increment), 4) + 1; case BlockID.WOODEN_STAIRS: case BlockID.COBBLESTONE_STAIRS: @@ -603,31 +608,37 @@ public final class BlockData { case BlockID.STONE_BRICK_STAIRS: case BlockID.PUMPKIN: case BlockID.JACKOLANTERN: - return (data + increment) % 4; + if (data > 3) return -1; + return mod((data + increment), 4); case BlockID.STEP: case BlockID.DOUBLE_STEP: case BlockID.CAKE_BLOCK: - return (data + increment) % 6; + if (data > 5) return -1; + return mod((data + increment), 6); case BlockID.CROPS: case BlockID.PUMPKIN_STEM: case BlockID.MELON_STEM: - return (data + increment) % 7; + if (data > 6) return -1; + return mod((data + increment), 7); case BlockID.SOIL: - case BlockID.SNOW: - return (data + increment) % 9; + if (data > 8) return -1; + return mod((data + increment), 9); case BlockID.RED_MUSHROOM_CAP: case BlockID.BROWN_MUSHROOM_CAP: - return (data + increment) % 11; + if (data > 10) return -1; + return mod((data + increment), 11); case BlockID.CACTUS: case BlockID.REED: case BlockID.SIGN_POST: case BlockID.VINE: - return (data + increment) % 16; + case BlockID.SNOW: + if (data > 15) return -1; + return mod((data + increment), 16); case BlockID.FURNACE: case BlockID.BURNING_FURNACE: @@ -635,31 +646,32 @@ public final class BlockData { case BlockID.WALL_SIGN: case BlockID.LADDER: case BlockID.CHEST: - return (data - 2 + increment) % 4 + 2; + if (data < 2 || data > 5) return -1; + return mod((data - 2 + increment), 4) + 2; case BlockID.REDSTONE_REPEATER_OFF: case BlockID.REDSTONE_REPEATER_ON: case BlockID.TRAP_DOOR: case BlockID.FENCE_GATE: case BlockID.LEAVES: + if (data > 7) return -1; store = data & ~0x3; - return ((data & 0x3) + increment) % 4 | store; + return mod(((data & 0x3) + increment), 4) | store; case BlockID.MINECART_TRACKS: if (data < 6 || data > 9) return -1; - return (data - 6 + increment) % 4 + 6; + return mod((data - 6 + increment), 4) + 6; case BlockID.SAPLING: + if ((data & 0x3) == 3 || data > 15) return -1; store = data & ~0x3; - return ((data & 0x3) + increment) % 3 | store; + return mod(((data & 0x3) + increment), 3) | store; case BlockID.CLOTH: - if (increment > 0) { + if (increment == 1) { data = nextClothColor(data); - } else if (increment < 0) { + } else if (increment == -1) { data = prevClothColor(data); - } else { - return -1; // shouldn't have a 0 increment anyway } return data; @@ -668,6 +680,14 @@ public final class BlockData { } } + /** + * Better modulo, not just remainder. + */ + private static int mod(int x, int y) { + int res = x % y; + return res < 0 ? res + y : res; + } + /** * Returns the data value for the next color of cloth in the rainbow. This * should not be used if you want to just increment the data value. diff --git a/src/test/java/com/sk89q/worldedit/data/BlockDataTest.java b/src/test/java/com/sk89q/worldedit/data/BlockDataTest.java index a5004911a..566f0cef6 100644 --- a/src/test/java/com/sk89q/worldedit/data/BlockDataTest.java +++ b/src/test/java/com/sk89q/worldedit/data/BlockDataTest.java @@ -19,6 +19,8 @@ package com.sk89q.worldedit.data; +import java.util.TreeSet; + import org.junit.*; import com.sk89q.worldedit.CuboidClipboard.FlipDirection; @@ -26,6 +28,9 @@ import com.sk89q.worldedit.blocks.BlockID; import static org.junit.Assert.*; +/** + * @author TomyLobo + */ public class BlockDataTest { @Test public void testRotateFlip() { @@ -33,23 +38,36 @@ public class BlockDataTest { for (int data = 0; data < 16; ++data) { final String message = type+"/"+data; + //Test r90(r-90(x))==x assertEquals(message, data, BlockData.rotate90(type, BlockData.rotate90Reverse(type, data))); + //Test r-90(r90(x))==x assertEquals(message, data, BlockData.rotate90Reverse(type, BlockData.rotate90(type, data))); - int flipped = BlockData.flip(type, BlockData.flip(type, data, FlipDirection.WEST_EAST), FlipDirection.NORTH_SOUTH); + final int flipped = BlockData.flip(type, BlockData.flip(type, data, FlipDirection.WEST_EAST), FlipDirection.NORTH_SOUTH); + //Test r90(r90(x))==flipNS(flipWE(x)) assertEquals(message, flipped, BlockData.rotate90(type, BlockData.rotate90(type, data))); + //Test r-90(r-90(x))==flipNS(flipWE(x)) assertEquals(message, flipped, BlockData.rotate90Reverse(type, BlockData.rotate90Reverse(type, data))); + //Test flipNS(flipNS(x))==x assertEquals(message, data, BlockData.flip(type, BlockData.flip(type, data, FlipDirection.NORTH_SOUTH), FlipDirection.NORTH_SOUTH)); + //Test flipWE(flipWE(x))==x assertEquals(message, data, BlockData.flip(type, BlockData.flip(type, data, FlipDirection.WEST_EAST), FlipDirection.WEST_EAST)); + //Test flipUD(flipUD(x))==x assertEquals(message, data, BlockData.flip(type, BlockData.flip(type, data, FlipDirection.UP_DOWN), FlipDirection.UP_DOWN)); + + //Test r90(r90(r90(r90(x))))==x + assertEquals(message, data, BlockData.rotate90(type, BlockData.rotate90(type, BlockData.rotate90(type, BlockData.rotate90(type, data))))); + //Test r-90(r-90(r-90(r-90(x))))==x + assertEquals(message, data, BlockData.rotate90Reverse(type, BlockData.rotate90Reverse(type, BlockData.rotate90Reverse(type, BlockData.rotate90Reverse(type, data))))); } } } @Test public void testCycle() { + // Test monotony for (int type = 0; type < 256; ++type) { if (type == BlockID.CLOTH) continue; @@ -57,14 +75,65 @@ public class BlockDataTest { for (int data = 0; data < 16; ++data) { final String message = type+"/"+data; - int cycled = BlockData.cycle(type, data, 1); + final int cycled = BlockData.cycle(type, data, 1); if (cycled <= data) { continue; } - assertEquals(message, data+1, cycled); + assertEquals(message, data+1, cycled); + } + } + + // Test cyclicity + final TreeSet datasTemplate = new TreeSet(); + for (int data = 0; data < 16; ++data) { + datasTemplate.add(data); + } + + // Forwards... + for (int type = 0; type < 256; ++type) { + @SuppressWarnings("unchecked") + final TreeSet datas = (TreeSet) datasTemplate.clone(); + while (!datas.isEmpty()) { + final int start = datas.pollFirst(); + String message = type+"/"+start; + int current = start; + boolean first = true; + while (true) { + current = BlockData.cycle(type, current, 1); + if (first && current == -1) break; + first = false; + message += "->"+current; + assertTrue(message, current >= 0); + assertTrue(message, current < 16); + if (current == start) break; + assertTrue(message, datas.remove(current)); + } + } + } + + // ...and backwards + for (int type = 0; type < 256; ++type) { + @SuppressWarnings("unchecked") + final TreeSet datas = (TreeSet) datasTemplate.clone(); + while (!datas.isEmpty()) { + final int start = datas.pollFirst(); + String message = type+"/"+start; + int current = start; + boolean first = true; + while (true) { + current = BlockData.cycle(type, current, -1); + if (first && current == -1) break; + first = false; + message += "->"+current; + assertTrue(message, current >= 0); + assertTrue(message, current < 16); + if (current == start) break; + assertTrue(message, datas.remove(current)); + } } } } } +