From 14ec7fb8168b44ed12697a42d5bcebd9115a37eb Mon Sep 17 00:00:00 2001 From: Hannes Greule Date: Sat, 5 Dec 2020 17:29:01 +0100 Subject: [PATCH] Fix PlotRangeIterator --- .../java/com/plotsquared/core/plot/Plot.java | 24 ++-- .../com/plotsquared/core/plot/PlotId.java | 18 +-- .../core/plot/PlotRangeIteratorTest.java | 108 ++++++++++++++++++ 3 files changed, 131 insertions(+), 19 deletions(-) create mode 100644 Core/src/test/java/com/plotsquared/core/plot/PlotRangeIteratorTest.java diff --git a/Core/src/main/java/com/plotsquared/core/plot/Plot.java b/Core/src/main/java/com/plotsquared/core/plot/Plot.java index d9c64417f..2833c54e9 100644 --- a/Core/src/main/java/com/plotsquared/core/plot/Plot.java +++ b/Core/src/main/java/com/plotsquared/core/plot/Plot.java @@ -2299,12 +2299,13 @@ public class Plot { continue; } boolean merge = true; - PlotId bot = PlotId.of(current.getId().getX(), current.getId().getY()); - PlotId top = PlotId.of(current.getId().getX(), current.getId().getY()); + PlotId bot = current.getId(); + PlotId top = current.getId(); while (merge) { merge = false; - List ids = Lists.newArrayList((Iterable) PlotId.PlotRangeIterator - .range(PlotId.of(bot.getX(), bot.getY() - 1), PlotId.of(top.getX(), bot.getY() - 1))); + Iterable ids = PlotId.PlotRangeIterator.range( + PlotId.of(bot.getX(), bot.getY() - 1), + PlotId.of(top.getX(), bot.getY() - 1)); boolean tmp = true; for (PlotId id : ids) { Plot plot = this.area.getPlotAbs(id); @@ -2316,8 +2317,9 @@ public class Plot { merge = true; bot = PlotId.of(bot.getX(), bot.getY() - 1); } - ids = Lists.newArrayList((Iterable) PlotId.PlotRangeIterator - .range(PlotId.of(top.getX() + 1, bot.getY()), PlotId.of(top.getX() + 1, top.getY()))); + ids = PlotId.PlotRangeIterator.range( + PlotId.of(top.getX() + 1, bot.getY()), + PlotId.of(top.getX() + 1, top.getY())); tmp = true; for (PlotId id : ids) { Plot plot = this.area.getPlotAbs(id); @@ -2329,8 +2331,9 @@ public class Plot { merge = true; top = PlotId.of(top.getX() + 1, top.getY()); } - ids = Lists.newArrayList((Iterable) PlotId.PlotRangeIterator - .range(PlotId.of(bot.getX(), top.getY() + 1), PlotId.of(top.getX(), top.getY() + 1))); + ids = PlotId.PlotRangeIterator.range( + PlotId.of(bot.getX(), top.getY() + 1), + PlotId.of(top.getX(), top.getY() + 1)); tmp = true; for (PlotId id : ids) { Plot plot = this.area.getPlotAbs(id); @@ -2342,8 +2345,9 @@ public class Plot { merge = true; top = PlotId.of(top.getX(), top.getY() + 1); } - ids = Lists.newArrayList((Iterable) PlotId.PlotRangeIterator - .range(PlotId.of(bot.getX() - 1, bot.getY()), PlotId.of(bot.getX() - 1, top.getY()))); + ids = PlotId.PlotRangeIterator.range( + PlotId.of(bot.getX() - 1, bot.getY()), + PlotId.of(bot.getX() - 1, top.getY())); tmp = true; for (PlotId id : ids) { Plot plot = this.area.getPlotAbs(id); diff --git a/Core/src/main/java/com/plotsquared/core/plot/PlotId.java b/Core/src/main/java/com/plotsquared/core/plot/PlotId.java index 98a18dfa7..2c1f78df7 100644 --- a/Core/src/main/java/com/plotsquared/core/plot/PlotId.java +++ b/Core/src/main/java/com/plotsquared/core/plot/PlotId.java @@ -30,6 +30,7 @@ import com.plotsquared.core.location.Direction; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.util.Iterator; +import java.util.NoSuchElementException; /** * Plot (X,Y) tuples for plot locations @@ -265,26 +266,25 @@ public final class PlotId { } @Override public boolean hasNext() { - if (this.x < this.end.getX()) { - return true; - } else if (this.x == this.end.getX()) { - return this.y < this.end.getY(); - } else { - return false; - } + // end is fully included + return this.x <= this.end.getX() && this.y <= this.end.getY(); } @Override public PlotId next() { if (!hasNext()) { - throw new IndexOutOfBoundsException("The iterator has no more entries"); + throw new NoSuchElementException("The iterator has no more entries"); } + // increment *after* getting the result to include the minimum + // the id to return + PlotId result = PlotId.of(this.x, this.y); + // first increase y, then x if (this.y == this.end.getY()) { this.x++; this.y = 0; } else { this.y++; } - return PlotId.of(this.start.getX() + this.x, this.start.getY() + this.y); + return result; } @Nonnull @Override public Iterator iterator() { diff --git a/Core/src/test/java/com/plotsquared/core/plot/PlotRangeIteratorTest.java b/Core/src/test/java/com/plotsquared/core/plot/PlotRangeIteratorTest.java new file mode 100644 index 000000000..011a8ec17 --- /dev/null +++ b/Core/src/test/java/com/plotsquared/core/plot/PlotRangeIteratorTest.java @@ -0,0 +1,108 @@ +/* + * _____ _ _ _____ _ + * | __ \| | | | / ____| | | + * | |__) | | ___ | |_| (___ __ _ _ _ __ _ _ __ ___ __| | + * | ___/| |/ _ \| __|\___ \ / _` | | | |/ _` | '__/ _ \/ _` | + * | | | | (_) | |_ ____) | (_| | |_| | (_| | | | __/ (_| | + * |_| |_|\___/ \__|_____/ \__, |\__,_|\__,_|_| \___|\__,_| + * | | + * |_| + * PlotSquared plot management system for Minecraft + * Copyright (C) 2020 IntellectualSites + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.plotsquared.core.plot; + +import org.junit.Test; + +import java.util.Arrays; +import java.util.List; +import java.util.NoSuchElementException; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class PlotRangeIteratorTest { + + @Test + public void singlePlotIterator() { + // an iterator that should only contain the given plot + PlotId id = PlotId.of(3, 7); + PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id, id); + assertTrue(range.hasNext()); + assertEquals(id, range.next()); + assertFalse(range.hasNext()); + assertThrows(NoSuchElementException.class, range::next); + } + + // the tests below assume a specific order (first increasing y, then increasing x) + // this is not a written requirement but makes testing easier + + @Test + public void squareAreaPlotIterator() { + PlotId id00 = PlotId.of(0, 0); + PlotId id01 = PlotId.of(0, 1); + PlotId id10 = PlotId.of(1, 0); + PlotId id11 = PlotId.of(1, 1); + List all = Arrays.asList(id00, id01, id10, id11); + PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id00, id11); + for (PlotId id : all) { + assertTrue(range.hasNext()); + assertEquals(id, range.next()); + } + assertFalse(range.hasNext()); + assertThrows(NoSuchElementException.class, range::next); + } + + @Test + public void rectangleYAreaPlotIterator() { + // max y > max x + PlotId id00 = PlotId.of(0, 0); + PlotId id01 = PlotId.of(0, 1); + PlotId id02 = PlotId.of(0, 2); + PlotId id10 = PlotId.of(1, 0); + PlotId id11 = PlotId.of(1, 1); + PlotId id12 = PlotId.of(1, 2); + List all = Arrays.asList(id00, id01, id02, id10, id11, id12); + PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id00, id12); + for (PlotId id : all) { + assertTrue(range.hasNext()); + assertEquals(id, range.next()); + } + assertFalse(range.hasNext()); + assertThrows(NoSuchElementException.class, range::next); + } + + @Test + public void rectangleXAreaPlotIterator() { + // max x > max y + PlotId id00 = PlotId.of(0, 0); + PlotId id01 = PlotId.of(0, 1); + PlotId id10 = PlotId.of(1, 0); + PlotId id11 = PlotId.of(1, 1); + PlotId id20 = PlotId.of(2, 0); + PlotId id21 = PlotId.of(2, 1); + List all = Arrays.asList(id00, id01, id10, id11, id20, id21); + PlotId.PlotRangeIterator range = PlotId.PlotRangeIterator.range(id00, id21); + for (PlotId id : all) { + assertTrue(range.hasNext()); + assertEquals(id, range.next()); + } + assertFalse(range.hasNext()); + assertThrows(NoSuchElementException.class, range::next); + } +}