diff --git a/common/src/main/java/net/onelitefeather/cygnus/common/config/GameConfig.java b/common/src/main/java/net/onelitefeather/cygnus/common/config/GameConfig.java index 0f9ee1c3..40fa4c1e 100644 --- a/common/src/main/java/net/onelitefeather/cygnus/common/config/GameConfig.java +++ b/common/src/main/java/net/onelitefeather/cygnus/common/config/GameConfig.java @@ -36,6 +36,48 @@ public sealed interface GameConfig permits GameConfigImpl, InternalGameConfig { int MIN_ACTIVE_PAGE_COUNT = 4 * 2; + /** + * How many seconds after a round starts before the first pages spawn. + *

+ * Spawning immediately at {@code GameStartEvent} let survivors grab a page before they had even + * moved from the spawn point. The delay gives them time to spread across the map first. + *

+ * + * @since 2.15.0 + */ + int PAGE_SPAWN_DELAY = 10; + + /** + * How many seconds {@link #PAGE_SPAWN_DELAY} may randomly shift up or down, re-rolled every + * round. + *

+ * Without this the delay lands on the exact same tick every round, which players learn and + * plan around; the jitter keeps the moment the first pages appear unpredictable. + *

+ * + * @since 2.15.0 + */ + int PAGE_SPAWN_DELAY_JITTER = 2; + + /** + * How many seconds a found page stays hidden when there is no free spot left to move it to. + *

+ * The page then has to come back on the spot it was found on. Without the delay it would be + * collectible again right away, letting a survivor pick up page after page on the same spot. + *

+ * + * @since 2.15.0 + */ + int PAGE_RESPAWN_DELAY = 15; + + /** + * How many seconds {@link #PAGE_RESPAWN_DELAY} may randomly shift up or down, re-rolled for every + * hidden page, so waiting next to the spot does not pay off. + * + * @since 2.15.0 + */ + int PAGE_RESPAWN_DELAY_JITTER = 5; + int PAGE_TTL_TIME = 60; int FORCE_START_TIME = 11; diff --git a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageCalculation.java b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageCalculation.java index 8da41b60..aa2578ad 100644 --- a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageCalculation.java +++ b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageCalculation.java @@ -3,6 +3,8 @@ import net.minestom.server.MinecraftServer; import net.onelitefeather.cygnus.common.config.GameConfig; +import java.util.concurrent.ThreadLocalRandom; + /** * Utility class for calculating the number of pages allocated for the dynamic page system. * The page count is based on the number of current online players. @@ -17,23 +19,69 @@ public final class PageCalculation { private static final int PLAYER_SIZE_FOR_DYNAMIC_PAGE_ALLOCATION = 4; private static final int PAGE_COUNT_MULTIPLIER = 2; + /** + * The largest amount {@link #calculatePageAmount()} may add on top of the base amount, so the + * total can't be worked out in advance from the player count alone. + *

+ * Not exposed through {@link GameConfig}: unlike the other page settings, this one is + * deliberately not something a server operator should be able to turn off or tune. + *

+ */ + private static final int PAGE_COUNT_JITTER_MAX = 2; + + private static final int PLAYER_SIZE_FOR_ACTIVE_PAGE_SCALING = 8; + + /** + * How many extra survivors above {@value #PLAYER_SIZE_FOR_ACTIVE_PAGE_SCALING} it takes for + * {@link #calculateActivePageAmount()} to add one more page. + *

+ * Playtesting a steeper step (one active page per extra survivor) reached 16 concurrently + * active pages, which felt like clutter; this keeps the top of the range below 10. + *

+ */ + private static final int ACTIVE_PAGE_COUNT_STEP_SIZE = 3; + /** * Calculates the number of pages to allocate for the dynamic page system. *

* If the number of online players (excluding one) is less than {@value #PLAYER_SIZE_FOR_DYNAMIC_PAGE_ALLOCATION}, - * {@link GameConfig#MIN_PAGE_COUNT} is returned. Otherwise, the page count is determined by - * multiplying the adjusted player count by {@value #PAGE_COUNT_MULTIPLIER}. + * the base amount is {@link GameConfig#MIN_PAGE_COUNT}. Otherwise, it is the adjusted player + * count multiplied by {@value #PAGE_COUNT_MULTIPLIER}. A random amount between 0 and + * {@value #PAGE_COUNT_JITTER_MAX} is then added on top, re-rolled every call, so the total + * can't be derived from the player count alone. * * @return the number of pages to allocate, at least {@link GameConfig#MIN_PAGE_COUNT} */ public static int calculatePageAmount() { int currentPlayers = MinecraftServer.getConnectionManager().getOnlinePlayers().size(); - if (currentPlayers - 1 < PLAYER_SIZE_FOR_DYNAMIC_PAGE_ALLOCATION) { - return GameConfig.MIN_PAGE_COUNT; - } else { - return (currentPlayers - 1) * PAGE_COUNT_MULTIPLIER; + int baseAmount = currentPlayers - 1 < PLAYER_SIZE_FOR_DYNAMIC_PAGE_ALLOCATION + ? GameConfig.MIN_PAGE_COUNT + : (currentPlayers - 1) * PAGE_COUNT_MULTIPLIER; + + return baseAmount + ThreadLocalRandom.current().nextInt(PAGE_COUNT_JITTER_MAX + 1); + } + + /** + * Calculates how many pages should be concurrently active (spawned in the world at once). + *

+ * This stays below {@link #calculatePageAmount()} on purpose: it governs how many pages exist + * in the world at the same time, not the total pool a round draws from. If the number of online + * players (excluding one) is less than {@value #PLAYER_SIZE_FOR_ACTIVE_PAGE_SCALING}, + * {@link GameConfig#MIN_ACTIVE_PAGE_COUNT} is returned. Otherwise, one page is added for every + * {@value #ACTIVE_PAGE_COUNT_STEP_SIZE} survivors past that threshold, so the count stays flat + * for most of the range and only rises near the top of a full lobby. + * + * @return the number of pages to keep active at once, at least {@link GameConfig#MIN_ACTIVE_PAGE_COUNT} + */ + public static int calculateActivePageAmount() { + int currentPlayers = MinecraftServer.getConnectionManager().getOnlinePlayers().size(); + int survivors = currentPlayers - 1; + + if (survivors < PLAYER_SIZE_FOR_ACTIVE_PAGE_SCALING) { + return GameConfig.MIN_ACTIVE_PAGE_COUNT; } + return GameConfig.MIN_ACTIVE_PAGE_COUNT + (survivors - PLAYER_SIZE_FOR_ACTIVE_PAGE_SCALING) / ACTIVE_PAGE_COUNT_STEP_SIZE; } private PageCalculation() { diff --git a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageEntity.java b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageEntity.java index 436cacb7..0d9fd51e 100644 --- a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageEntity.java +++ b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageEntity.java @@ -15,7 +15,6 @@ import net.onelitefeather.cygnus.common.config.GameConfig; import net.onelitefeather.cygnus.common.page.event.PageExpiredEvent; import net.onelitefeather.cygnus.common.util.Helper; -import org.jetbrains.annotations.Nullable; import java.util.UUID; import java.util.concurrent.CompletableFuture; @@ -48,18 +47,17 @@ public final class PageEntity extends Entity implements PageCreator, PageProximi private boolean send; private boolean interactable = true; private int initialBlockLight; - private @Nullable PageResource resource; + private PageResource resource; /** - * Constructs a new {@link PageEntity}. + * Constructs a new {@link PageEntity}. The page is not part of any instance until {@link #place(Instance)} is called. * - * @param instance the instance where the entity should spawn - * @param spawnPos the position where the entity should spawn + * @param resource the spot the page stands on * @param pageCount the current page count */ - PageEntity(Instance instance, Pos spawnPos, int pageCount) { + PageEntity(PageResource resource, int pageCount) { super(EntityType.ITEM_DISPLAY); - this.setInstance(instance, spawnPos); + this.resource = resource; this.hitBox = new Entity(EntityType.INTERACTION); this.pageItem = createPageItem(pageCount); this.ttlTime = Helper.calculateOffsetTime(GameConfig.PAGE_TTL_TIME); @@ -86,7 +84,6 @@ public final class PageEntity extends Entity implements PageCreator, PageProximi interactionMeta.setResponse(true); interactionMeta.setNotifyAboutChanges(true); interactionMeta.setHasGlowingEffect(true); - this.hitBox.setInstance(instance, spawnPos.sub(HALF_BLOCK)); this.hitBox.setAutoViewable(true); this.hitBox.setTag(Tags.PAGE_TAG, this.hitBox.getUuid()); } @@ -108,6 +105,16 @@ public void disableInteraction() { this.interactable = false; } + /** + * Takes the page out of play for the given time. It comes back on its current spot afterwards. + * + * @param seconds how long the page stays hidden + */ + void hideFor(int seconds) { + this.disableInteraction(); + this.ttlTime = seconds; + } + /** * Updates the item stack for the page. * @@ -164,6 +171,11 @@ public void tick(long time) { } if (currentTickTime >= ttlTime) { + // A page is only out of play while it waits to come back, see hideFor + if (!this.interactable) { + this.enableInteraction(); + return; + } this.disableInteraction(); send = true; EventDispatcher.call(new PageExpiredEvent(this)); @@ -240,20 +252,35 @@ public UUID getHitBoxUUID() { } /** - * Sets the {@link PageResource} the page currently stands on. + * Places the page and its hit box into the given instance, on the spot of its resource. + * + * @param instance the instance to place the page in + * @return a future that completes once both entities are in the instance + */ + public CompletableFuture place(Instance instance) { + Pos position = Helper.updatePosition(this.resource.position().asPos(), this.resource.face()); + return CompletableFuture.allOf( + this.hitBox.setInstance(instance, position.sub(HALF_BLOCK)), + this.setInstance(instance, position) + ); + } + + /** + * Moves the already placed page to the spot of the given resource. * - * @param resource the resource, or {@code null} once the page has no spot of its own + * @param resource the spot to move to */ - void setResource(@Nullable PageResource resource) { + void moveTo(PageResource resource) { this.resource = resource; + this.teleport(Helper.updatePosition(resource.position().asPos(), resource.face())); } /** * Returns the {@link PageResource} the page currently stands on. * - * @return the resource, or {@code null} if none is set + * @return the resource */ - @Nullable PageResource getResource() { + PageResource getResource() { return this.resource; } diff --git a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageFactory.java b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageFactory.java index fd86085a..107a82a1 100644 --- a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageFactory.java +++ b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageFactory.java @@ -1,7 +1,5 @@ package net.onelitefeather.cygnus.common.page; -import net.minestom.server.coordinate.Pos; -import net.minestom.server.instance.Instance; import net.minestom.server.utils.Direction; import net.minestom.server.utils.validate.Check; import org.jetbrains.annotations.Contract; @@ -20,21 +18,15 @@ private PageFactory() {} /** * Creates a new reference from a {@link PageEntity} class. - * @param instance the instance where the entity should be spawned - * @param spawnPos the position to spawn - * @param direction the direction for the spawning + * @param resource the spot the page should stand on * @param pageCount the current count of the page * @return the created entity reference */ - @Contract(value = "_, _, _, _ -> new" , pure = true) - public static PageEntity createPage( - Instance instance, - Pos spawnPos, - Direction direction, - int pageCount - ) { + @Contract(value = "_, _ -> new" , pure = true) + public static PageEntity createPage(PageResource resource, int pageCount) { + Direction direction = resource.face(); Check.argCondition(direction == Direction.UP || direction == Direction.DOWN, "The direction " + direction + " is not supported"); Check.argCondition(pageCount < 1, "The page count can't be zero or negative"); - return new PageEntity(instance, spawnPos, pageCount); + return new PageEntity(resource, pageCount); } } diff --git a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageProvider.java b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageProvider.java index e2108620..249fc99d 100644 --- a/common/src/main/java/net/onelitefeather/cygnus/common/page/PageProvider.java +++ b/common/src/main/java/net/onelitefeather/cygnus/common/page/PageProvider.java @@ -2,25 +2,21 @@ import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; -import net.minestom.server.coordinate.Pos; import net.minestom.server.entity.Player; import net.minestom.server.event.EventDispatcher; import net.minestom.server.instance.Instance; -import net.minestom.server.utils.Direction; import net.minestom.server.utils.validate.Check; import net.onelitefeather.cygnus.common.Messages; +import net.onelitefeather.cygnus.common.config.GameConfig; import net.onelitefeather.cygnus.common.page.event.PageDiscoveryCompletedEvent; import net.onelitefeather.cygnus.common.page.event.PageFoundEvent; -import net.onelitefeather.cygnus.common.util.Helper; import net.theevilreaper.aves.util.Broadcaster; import net.theevilreaper.xerus.api.phase.GamePhase; -import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Queue; @@ -28,15 +24,18 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicInteger; -import static net.onelitefeather.cygnus.common.config.GameConfig.MIN_ACTIVE_PAGE_COUNT; - /** * Handles the logic to manage and spawn pages during the {@link GamePhase}. + *

+ * The provider owns a pool of free spots. A page that expires hands its spot back to the pool, + * a page that is found uses its spot up for the rest of the round. + *

* * @author theEvilReaper - * @version 1.2.0 + * @version 1.3.0 * @since 1.0.0 **/ @SuppressWarnings("java:S3252") @@ -44,21 +43,12 @@ public final class PageProvider { private static final Logger LOGGER = LoggerFactory.getLogger(PageProvider.class); - private final Queue globalCache; - private final Map activePages; - private final AtomicInteger currentPageCount; - private final AtomicInteger currentFoundedPageCount; - + private final Queue freeSpots = new ConcurrentLinkedQueue<>(); + private final Map activePages = new ConcurrentHashMap<>(); + private final AtomicInteger pageNumber = new AtomicInteger(1); + private final AtomicInteger foundPages = new AtomicInteger(); private int maxPageAmount; - public PageProvider() { - this.globalCache = new ConcurrentLinkedQueue<>(); - this.activePages = new ConcurrentHashMap<>(); - this.maxPageAmount = 0; - this.currentFoundedPageCount = new AtomicInteger(0); - this.currentPageCount = new AtomicInteger(1); - } - /** * Loads the required page data from the given set of {@link PageResource}s. * The resources are shuffled once so the queue can be drained without picking a random index on every access. @@ -66,36 +56,40 @@ public PageProvider() { * @param resources given set of page resources */ public void loadPageData(Set resources) { - Check.argCondition(!globalCache.isEmpty(), "Can't load pages twice"); + Check.argCondition(!this.freeSpots.isEmpty(), "Can't load pages twice"); if (resources.isEmpty()) { throw new IllegalStateException("Can't load a map without any pages"); } List shuffled = new ArrayList<>(resources); Collections.shuffle(shuffled); - this.globalCache.addAll(shuffled); + this.freeSpots.addAll(shuffled); } - public void collectStartPages(Instance instance) { - Check.argCondition(this.globalCache.size() < MIN_ACTIVE_PAGE_COUNT, "Not enough pages to start the game"); - int counter = 0; - - Set candidateHashes = new HashSet<>(); + /** + * Picks the spots for the pages that are active when a round starts and creates the pages on them. + * The pages only appear in the world once {@link #spawn(Instance)} is called. + * + * @param activePageCount how many pages to keep concurrently active, e.g. from {@link PageCalculation#calculateActivePageAmount()} + */ + public void collectStartPages(int activePageCount) { + Check.argCondition(this.freeSpots.size() < activePageCount, "Not enough pages to start the game"); + for (int i = 0; i < activePageCount; i++) { + PageEntity page = PageFactory.createPage(this.freeSpots.poll(), this.pageNumber.getAndIncrement()); + this.activePages.put(page.getHitBoxUUID(), page); + } + LOGGER.info("Collected {} start pages", activePageCount); + } - while (counter < MIN_ACTIVE_PAGE_COUNT && !this.globalCache.isEmpty()) { - PageResource page = this.globalCache.poll(); - if (candidateHashes.add(page.hashCode())) { - Direction direction = page.face(); - Pos position = Helper.updatePosition(page.position().asPos(), direction); - PageEntity entity = PageFactory.createPage(instance, position, direction, this.currentPageCount.getAndIncrement()); - entity.setResource(page); - this.activePages.put(entity.getHitBoxUUID(), entity); - counter++; - continue; - } - this.globalCache.add(page); + /** + * Places every collected page into the given instance. + * + * @param instance the instance the round is played in + */ + public void spawn(Instance instance) { + for (PageEntity page : this.activePages.values()) { + page.place(instance); } - LOGGER.info("This current page count is {}", currentPageCount.get()); } /** @@ -110,98 +104,79 @@ public void setMaxPageAmount(int maxPageAmount) { this.maxPageAmount = maxPageAmount; } - /** - * Spawns all pages that are currently in the active page map. - */ - public void spawn() { - for (Map.Entry pointPageEntityEntry : this.activePages.entrySet()) { - pointPageEntityEntry.getValue().spawn(); - } - } - public void cleanUp() { - if (this.activePages.isEmpty()) return; for (UUID uuid : List.copyOf(this.activePages.keySet())) { - PageEntity value = this.activePages.remove(uuid); - if (value == null) continue; - value.disableInteraction(); - value.remove(); + PageEntity page = this.activePages.remove(uuid); + if (page == null) continue; + page.disableInteraction(); + page.remove(); } } public void triggerTTLHandling(UUID uuid) { - if (this.globalCache.isEmpty()) { - PageEntity page = this.activePages.computeIfPresent(uuid, (key, value) -> { - value.enableInteraction(); - return value; - }); - if (page == null) { - LOGGER.debug("Page {} was already claimed when its TTL expired, ignoring", uuid); - } - return; - } - - PageEntity pageEntity = this.removeEntity(uuid); - if (pageEntity == null) { + PageEntity page = this.activePages.remove(uuid); + if (page == null) { LOGGER.debug("Page {} was already claimed when its TTL expired, ignoring", uuid); return; } - - PageResource newPos = this.globalCache.poll(); - if (newPos != null) { - PageResource expired = pageEntity.getResource(); - pageEntity.teleport(Helper.updatePosition(newPos.position().asPos(), newPos.face())); - pageEntity.setResource(newPos); - // Polled first, so the page can't draw its own spot again; queued last, so the spot only - // comes back once every other one had its turn. Found spots stay used up. - if (expired != null) { - this.globalCache.add(expired); - } - } - this.activePages.put(pageEntity.getHitBoxUUID(), pageEntity); - pageEntity.enableInteraction(); + this.relocate(page, true); } public boolean triggerPageFound(Player player, UUID uuid) { - PageEntity pageEntity = removeEntity(uuid); - if (pageEntity == null) { - LOGGER.debug("Page {} was already claimed when {} interacted, ignoring", uuid, player.getUsername()); + PageEntity page = this.activePages.get(uuid); + // A hidden page still has a hit box, so a click on it must not count + if (page == null || !page.isInteractable() || !this.activePages.remove(uuid, page)) { + LOGGER.debug("Page {} was already claimed or is hidden when {} interacted, ignoring", uuid, player.getUsername()); return false; } - player.getInventory().addItemStack(pageEntity.getPageItem()); + player.getInventory().addItemStack(page.getPageItem()); Broadcaster.broadcast(Messages.getPageFoundComponent(player)); - int foundCount = this.currentFoundedPageCount.incrementAndGet(); + int foundCount = this.foundPages.incrementAndGet(); EventDispatcher.call(new PageFoundEvent(player, foundCount, this.maxPageAmount)); - if (foundCount >= maxPageAmount) { + if (foundCount >= this.maxPageAmount) { EventDispatcher.call(new PageDiscoveryCompletedEvent()); } - // Re-inserting the entity makes it discoverable again, so this must happen last: + page.updateItemStack(this.pageNumber.incrementAndGet()); + // Re-inserting the page makes it discoverable again, so this must happen last: // doing it earlier reopens a window where a concurrent call for the same uuid // legitimately re-claims it and double-credits the find. - updatePageData(pageEntity); + this.relocate(page, false); return true; } /** - * Returns the positions of every page a player could currently walk up to and collect. - *

- * Pages that ran out of TTL are left out: they are invisible and do not respond to interaction, - * so pointing a player at one would be a lie. - *

+ * Moves a page that was taken out of play to a free spot and puts it back into play. + * Without a free spot the page stays where it is; a found page is hidden for a while first. * - * @return the positions of the collectible pages, in no particular order - * @since 2.12.0 + * @param page the page to move + * @param returnOldSpot whether the spot the page leaves goes back into the pool */ - public List interactablePagePositions() { - List positions = new ArrayList<>(this.activePages.size()); - for (PageEntity entity : this.activePages.values()) { - if (entity.isInteractable()) { - positions.add(entity.getPosition()); + private void relocate(PageEntity page, boolean returnOldSpot) { + PageResource oldSpot = page.getResource(); + // Polled first, so the page can't draw its own spot again; queued last, so the spot only + // comes back once every other one had its turn. + PageResource newSpot = this.freeSpots.poll(); + if (newSpot == null && !returnOldSpot) { + // Found with nowhere else to go: the spot has to be reused, but not right away + page.hideFor(respawnDelay()); + } else { + if (newSpot != null) { + page.moveTo(newSpot); + if (returnOldSpot) { + this.freeSpots.add(oldSpot); + } } + // Shows the current item and restarts the TTL: on its spot the page counts as a fresh one + page.enableInteraction(); } - return positions; + this.activePages.put(page.getHitBoxUUID(), page); + } + + private static int respawnDelay() { + int jitter = GameConfig.PAGE_RESPAWN_DELAY_JITTER; + return GameConfig.PAGE_RESPAWN_DELAY + ThreadLocalRandom.current().nextInt(-jitter, jitter + 1); } /** @@ -212,37 +187,14 @@ public List interactablePagePositions() { */ public List interactablePages() { List pages = new ArrayList<>(this.activePages.size()); - for (PageEntity entity : this.activePages.values()) { - if (entity.isInteractable()) { - pages.add(entity); + for (PageEntity page : this.activePages.values()) { + if (page.isInteractable()) { + pages.add(page); } } return pages; } - private void updatePageData(PageEntity entity) { - PageResource resource = this.globalCache.poll(); - // Cleared when nothing is left, so the found spot can never make it back into the pool - entity.setResource(resource); - if (resource != null) { - entity.teleport(Helper.updatePosition(resource.position().asPos(), resource.face())); - } - entity.updateItemStack(this.currentPageCount.incrementAndGet()); - // Shows the new item and restarts the TTL: on its new spot the page counts as a fresh one - entity.enableInteraction(); - this.activePages.put(entity.getHitBoxUUID(), entity); - } - - /** - * Returns a {@link PageEntity} that matches wit the given id - * - * @param uuid pf the entity - * @return the fetched reference or null - */ - private @Nullable PageEntity removeEntity(UUID uuid) { - return this.activePages.remove(uuid); - } - /** * Returns the {@link Component} which contains a textual representation of the current page status. * @@ -250,7 +202,7 @@ private void updatePageData(PageEntity entity) { */ public Component getPageStatus() { // Built on every call: a cached copy written by concurrent finds could end up with a stale count - return Component.text(this.currentFoundedPageCount.get(), NamedTextColor.GREEN) + return Component.text(this.foundPages.get(), NamedTextColor.GREEN) .append(Component.space()) .append(Component.text("/", NamedTextColor.GRAY)) .append(Component.space()) @@ -265,4 +217,4 @@ public Component getPageStatus() { public int getMaxPageAmount() { return maxPageAmount; } -} \ No newline at end of file +} diff --git a/common/src/main/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculation.java b/common/src/main/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculation.java index e4b48541..d3bc8bfa 100644 --- a/common/src/main/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculation.java +++ b/common/src/main/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculation.java @@ -1,7 +1,6 @@ package net.onelitefeather.cygnus.common.util; import net.minestom.server.MinecraftServer; -import net.onelitefeather.cygnus.common.config.GameConfig; import java.util.concurrent.ThreadLocalRandom; import java.util.random.RandomGenerator; @@ -23,12 +22,12 @@ public final class HealthScalingCalculation { /** * Calculates the additional health that should be given to the player based on the number of pages they have collected. * - * @param pageCount the number of pages for the game + * From four survivors on there is no bonus. + * * @return the additional health that should be given to the player, always a multiple of 2 (whole hearts) */ - public static float getAdditionalHealth(int pageCount) { - if (pageCount > GameConfig.MIN_PAGE_COUNT) return 0.0f; - int playerCount = Math.min(MinecraftServer.getConnectionManager().getOnlinePlayerCount() - 1, 4); + public static float getAdditionalHealth() { + int playerCount = Math.clamp(MinecraftServer.getConnectionManager().getOnlinePlayerCount() - 1, 0, 4); float rawBonus = MAX_HEALTH * (1.0f - (float) playerCount / 4); return roundToWholeHeart(rawBonus, ThreadLocalRandom.current()); } diff --git a/common/src/main/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculation.java b/common/src/main/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculation.java index 1b77b4b9..967807ab 100644 --- a/common/src/main/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculation.java +++ b/common/src/main/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculation.java @@ -1,7 +1,6 @@ package net.onelitefeather.cygnus.common.util; import net.minestom.server.MinecraftServer; -import net.onelitefeather.cygnus.common.config.GameConfig; /** * This class is responsible for calculating the additional movement speed that should be given to the player based on the number of players online. @@ -18,12 +17,12 @@ public final class SpeedScalingCalculation { /** * Calculates the additional movement speed that should be given to the player based on the number of players online. * - * @param pageCount the number of pages for the game + * From four survivors on there is no bonus. + * * @return the additional movement speed that should be given to the player */ - public static double getAdditionalSpeed(int pageCount) { - if (pageCount > GameConfig.MIN_PAGE_COUNT) return 0.0; - int playerCount = Math.min(MinecraftServer.getConnectionManager().getOnlinePlayerCount() - 1, 4); + public static double getAdditionalSpeed() { + int playerCount = Math.clamp(MinecraftServer.getConnectionManager().getOnlinePlayerCount() - 1, 0, 4); return MAX_SPEED_BONUS * (1.0 - (double) playerCount / 4); } diff --git a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageCalculationTest.java b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageCalculationTest.java index a9547738..b28493cf 100644 --- a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageCalculationTest.java +++ b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageCalculationTest.java @@ -8,11 +8,21 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import java.util.HashSet; +import java.util.Set; + import static org.junit.jupiter.api.Assertions.*; @ExtendWith(MicrotusExtension.class) class PageCalculationTest { + /** + * The page count carries an unpredictable +0..+2 on top of the base amount (see + * {@link #testPageCalculationVariesAcrossRounds}), so exact-value assertions elsewhere in this + * class check a range instead of a single number. + */ + private static final int MAX_JITTER = 2; + @Test void testPageCalculationWithoutScaling(@NotNull Env env) { Instance instance = env.createFlatInstance(); @@ -22,7 +32,8 @@ void testPageCalculationWithoutScaling(@NotNull Env env) { } int pageCount = PageCalculation.calculatePageAmount(); - assertEquals(GameConfig.MIN_PAGE_COUNT, pageCount); + assertTrue(pageCount >= GameConfig.MIN_PAGE_COUNT && pageCount <= GameConfig.MIN_PAGE_COUNT + MAX_JITTER, + "the page count must be the minimum plus at most the jitter, was " + pageCount); env.destroyInstance(instance, true); } @@ -36,7 +47,75 @@ void testPageCalculationWithScaling(@NotNull Env env) { } int pageCount = PageCalculation.calculatePageAmount(); - assertEquals(18, pageCount); + assertTrue(pageCount >= 18 && pageCount <= 18 + MAX_JITTER, + "the page count must be the scaled amount plus at most the jitter, was " + pageCount); + + env.destroyInstance(instance, true); + } + + @Test + void testPageCalculationVariesAcrossRounds(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + + for (int i = 0; i < 10; i++) { + env.createPlayer(instance); + } + + Set observed = new HashSet<>(); + for (int i = 0; i < 50; i++) { + observed.add(PageCalculation.calculatePageAmount()); + } + + assertTrue(observed.size() > 1, + "the page count must vary across rounds instead of always landing on the same number, observed " + observed); + for (int value : observed) { + assertTrue(value >= 18 && value <= 18 + MAX_JITTER, + "each roll must stay within the base amount plus at most the jitter, was " + value); + } + + env.destroyInstance(instance, true); + } + + @Test + void testActivePageCalculationWithoutScaling(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + + for (int i = 0; i < 3; i++) { + env.createPlayer(instance); + } + + int activePageCount = PageCalculation.calculateActivePageAmount(); + assertEquals(GameConfig.MIN_ACTIVE_PAGE_COUNT, activePageCount); + + env.destroyInstance(instance, true); + } + + @Test + void testActivePageCalculationStaysFlatUntilNearTheTopOfTheRange(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + + for (int i = 0; i < 11; i++) { + env.createPlayer(instance); + } + + int activePageCount = PageCalculation.calculateActivePageAmount(); + assertEquals(GameConfig.MIN_ACTIVE_PAGE_COUNT, activePageCount, + "the active page count must still be at the minimum just below the top of the range"); + + env.destroyInstance(instance, true); + } + + @Test + void testActivePageCalculationWithScaling(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + + for (int i = 0; i < 13; i++) { + env.createPlayer(instance); + } + + int activePageCount = PageCalculation.calculateActivePageAmount(); + assertEquals(9, activePageCount, + "the active page count at the top of the range must stay below 10"); env.destroyInstance(instance, true); } diff --git a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageEntityTest.java b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageEntityTest.java index 8f52ef94..9bd308f4 100644 --- a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageEntityTest.java +++ b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageEntityTest.java @@ -6,12 +6,16 @@ import net.minestom.server.coordinate.Pos; import net.minestom.server.entity.metadata.display.ItemDisplayMeta; import net.minestom.server.instance.Instance; +import net.minestom.server.utils.Direction; import net.minestom.testing.Env; import net.minestom.testing.extension.MicrotusExtension; +import net.onelitefeather.cygnus.common.util.Helper; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import java.lang.reflect.Field; + import static org.junit.jupiter.api.Assertions.*; @ExtendWith(MicrotusExtension.class) @@ -21,11 +25,11 @@ class PageEntityTest { void testPageEntityCreation(@NotNull Env env) { Instance instance = env.createFlatInstance(); - PageEntity pageEntity = new PageEntity(instance, Pos.ZERO, 1); + PageEntity pageEntity = placedPage(instance); assertNotNull(pageEntity); assertNotNull(pageEntity.getPageItem()); - assertEquals(Pos.ZERO, pageEntity.getPosition()); + assertEquals(Helper.updatePosition(Pos.ZERO, Direction.NORTH), pageEntity.getPosition()); assertNotEquals(pageEntity.getUuid(), pageEntity.getHitBoxUUID()); Component displayName = pageEntity.getPageItem().get(DataComponents.CUSTOM_NAME); @@ -43,7 +47,7 @@ void testPageEntityCreation(@NotNull Env env) { void testInteractionStateFollowsEnableAndDisable(@NotNull Env env) { Instance instance = env.createFlatInstance(); - PageEntity pageEntity = new PageEntity(instance, Pos.ZERO, 1); + PageEntity pageEntity = placedPage(instance); assertTrue(pageEntity.isInteractable(), "a freshly spawned page must be collectible"); @@ -61,7 +65,7 @@ void testInteractionStateFollowsEnableAndDisable(@NotNull Env env) { void testPageIsLitIndependentlyFromTheEnvironment(@NotNull Env env) { Instance instance = env.createFlatInstance(); - PageEntity pageEntity = new PageEntity(instance, Pos.ZERO, 1); + PageEntity pageEntity = placedPage(instance); ItemDisplayMeta itemDisplayMeta = (ItemDisplayMeta) pageEntity.getEntityMeta(); int blockLight = itemDisplayMeta.getBlockLight(); @@ -81,7 +85,7 @@ void testPageIsLitIndependentlyFromTheEnvironment(@NotNull Env env) { void testPageBrightnessResetOnEnableInteraction(@NotNull Env env) { Instance instance = env.createFlatInstance(); - PageEntity pageEntity = new PageEntity(instance, Pos.ZERO, 1); + PageEntity pageEntity = placedPage(instance); ItemDisplayMeta itemDisplayMeta = (ItemDisplayMeta) pageEntity.getEntityMeta(); pageEntity.disableInteraction(); @@ -97,4 +101,46 @@ void testPageBrightnessResetOnEnableInteraction(@NotNull Env env) { pageEntity.remove(); env.destroyInstance(instance); } + + @Test + void testPageIsOnlyInTheWorldOncePlaced(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + + PageEntity pageEntity = new PageEntity(new PageResource(Pos.ZERO, Direction.NORTH), 1); + assertNull(pageEntity.getInstance(), "creating a page must not put it into the world yet"); + + pageEntity.place(instance).join(); + assertEquals(instance, pageEntity.getInstance()); + + pageEntity.remove(); + env.destroyInstance(instance); + } + + @Test + void testAHiddenPageComesBackOnceItsDelayIsOver(@NotNull Env env) throws Exception { + Instance instance = env.createFlatInstance(); + PageEntity pageEntity = placedPage(instance); + Field tickTime = PageEntity.class.getDeclaredField("currentTickTime"); + tickTime.setAccessible(true); + + pageEntity.hideFor(3); + assertFalse(pageEntity.isInteractable()); + + tickTime.setInt(pageEntity, 2); + pageEntity.tick(0); + assertFalse(pageEntity.isInteractable(), "the page must stay hidden until its delay is over"); + + tickTime.setInt(pageEntity, 3); + pageEntity.tick(0); + assertTrue(pageEntity.isInteractable(), "the page must come back once its delay is over"); + + pageEntity.remove(); + env.destroyInstance(instance); + } + + private static PageEntity placedPage(Instance instance) { + PageEntity pageEntity = new PageEntity(new PageResource(Pos.ZERO, Direction.NORTH), 1); + pageEntity.place(instance).join(); + return pageEntity; + } } diff --git a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageFactoryTest.java b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageFactoryTest.java index ca408ac3..3ead868d 100644 --- a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageFactoryTest.java +++ b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageFactoryTest.java @@ -5,6 +5,7 @@ import net.minestom.server.utils.Direction; import net.minestom.testing.Env; import net.minestom.testing.extension.MicrotusExtension; +import net.onelitefeather.cygnus.common.util.Helper; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -15,40 +16,38 @@ class PageFactoryTest { @Test - void testPageCreationWithInvalidDirection(@NotNull Env env) { - Instance instance = env.createFlatInstance(); + void testPageCreationWithInvalidDirection() { assertThrowsExactly( IllegalArgumentException.class, - () -> PageFactory.createPage(instance, Pos.ZERO, Direction.UP, 0), + () -> PageFactory.createPage(new PageResource(Pos.ZERO, Direction.UP), 0), "The direction " + Direction.UP + " is not supported" ); assertThrowsExactly( IllegalArgumentException.class, - () -> PageFactory.createPage(instance, Pos.ZERO, Direction.DOWN, 0), + () -> PageFactory.createPage(new PageResource(Pos.ZERO, Direction.DOWN), 0), "The direction " + Direction.DOWN + " is not supported" ); - env.destroyInstance(instance); } @Test - void testPageCreationWithInvalidPageCount(@NotNull Env env) { - Instance instance = env.createFlatInstance(); + void testPageCreationWithInvalidPageCount() { assertThrowsExactly( IllegalArgumentException.class, - () -> PageFactory.createPage(instance, Pos.ZERO, Direction.SOUTH, -1), + () -> PageFactory.createPage(new PageResource(Pos.ZERO, Direction.SOUTH), -1), "The page count can't be zero or negative" ); - env.destroyInstance(instance); } @Test void testPageCreationViaFactory(@NotNull Env env) { Instance instance = env.createFlatInstance(); - PageEntity pageEntity = PageFactory.createPage(instance, Pos.ZERO, Direction.SOUTH, 1); + PageResource resource = new PageResource(Pos.ZERO, Direction.SOUTH); + PageEntity pageEntity = PageFactory.createPage(resource, 1); + pageEntity.place(instance).join(); - assertNotNull(pageEntity); - assertEquals(Pos.ZERO, pageEntity.getPosition()); + assertEquals(resource, pageEntity.getResource()); + assertEquals(Helper.updatePosition(Pos.ZERO, Direction.SOUTH), pageEntity.getPosition()); assertEquals(instance.getUuid(), pageEntity.getInstance().getUuid()); pageEntity.remove(); diff --git a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageProviderTest.java b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageProviderTest.java index 41b0b751..39bc60d8 100644 --- a/common/src/test/java/net/onelitefeather/cygnus/common/page/PageProviderTest.java +++ b/common/src/test/java/net/onelitefeather/cygnus/common/page/PageProviderTest.java @@ -13,7 +13,6 @@ import net.onelitefeather.cygnus.common.page.event.PageDiscoveryCompletedEvent; import net.onelitefeather.cygnus.common.page.event.PageFoundEvent; import org.jetbrains.annotations.NotNull; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -21,8 +20,6 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.Queue; import java.util.Set; import java.util.UUID; import java.util.concurrent.CountDownLatch; @@ -41,210 +38,113 @@ class PageProviderTest { @Test - void testPageTwiceLoading() { - Set pageResources = Set.of( - new PageResource(Pos.ZERO, Direction.NORTH) - ); + void testLoadingPagesTwiceIsRejected() { PageProvider pageProvider = new PageProvider(); - assertNotNull(pageProvider); - - pageProvider.loadPageData(pageResources); + pageProvider.loadPageData(spots(1)); + Set values = spots(1); IllegalArgumentException exception = assertThrows( IllegalArgumentException.class, - () -> pageProvider.loadPageData(pageResources) + () -> pageProvider.loadPageData(values) ); - - assertInstanceOf(IllegalArgumentException.class, exception); assertEquals("Can't load pages twice", exception.getMessage()); } @Test - void testEmptyPageResourceUsage() { + void testLoadingNoPagesIsRejected() { PageProvider pageProvider = new PageProvider(); - assertNotNull(pageProvider); - Set pageResources = Set.of(); IllegalStateException exception = assertThrows( IllegalStateException.class, - () -> pageProvider.loadPageData(pageResources) + () -> pageProvider.loadPageData(Set.of()) ); - - assertInstanceOf(IllegalStateException.class, exception); assertEquals("Can't load a map without any pages", exception.getMessage()); } - /** - * Reproduces the race where two players interact with the same page hitbox at almost the same time. - * Before the fix, the losing call dereferenced a {@code null} {@link PageEntity} and threw an NPE; - * it must now bail out silently and the winner's count must still be recorded correctly. - */ - @Disabled(value = "Check flakiness") @Test - void testConcurrentDuplicateFind(@NotNull Env env) throws Exception { - Instance instance = env.createFlatInstance(); + void testCollectStartPagesUsesTheGivenActivePageCount() { + int activePageCount = 12; PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData(Set.of(new PageResource(Pos.ZERO, Direction.NORTH))); - pageProvider.setMaxPageAmount(1); - - PageEntity pageEntity = new PageEntity(instance, Pos.ZERO, 1); - UUID uuid = pageEntity.getHitBoxUUID(); - seedActivePages(pageProvider, pageEntity); - - Player player = env.createPlayer(instance); + pageProvider.loadPageData(spots(activePageCount)); - AtomicInteger completedEvents = new AtomicInteger(); - env.process().eventHandler().addListener(PageDiscoveryCompletedEvent.class, event -> completedEvents.incrementAndGet()); + pageProvider.collectStartPages(activePageCount); - runConcurrently(Collections.nCopies(8, (Runnable) () -> pageProvider.triggerPageFound(player, uuid))); - - assertEquals(1, completedEvents.get(), "the completion event must fire exactly once, not zero or more than once"); - assertEquals("1 / 1", plainStatus(pageProvider)); - - env.destroyInstance(instance, true); + assertEquals(activePageCount, pageProvider.interactablePages().size(), + "collectStartPages must collect exactly the requested active page count"); } @Test - void testTriggerPageFound(@NotNull Env env) throws Exception { - Instance instance = env.createFlatInstance(); + void testCollectStartPagesRejectsAnActivePageCountAboveTheAvailableData() { PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData(Set.of(new PageResource(Pos.ZERO, Direction.NORTH))); - pageProvider.setMaxPageAmount(1); - - PageEntity pageEntity = new PageEntity(instance, Pos.ZERO, 1); - UUID uuid = pageEntity.getHitBoxUUID(); - seedActivePages(pageProvider, pageEntity); - - Player player = env.createPlayer(instance); - - boolean firstClaim = pageProvider.triggerPageFound(player, uuid); - boolean nonExistentClaim = pageProvider.triggerPageFound(player, UUID.randomUUID()); - - assertTrue(firstClaim, "first claim must return true"); - assertFalse(nonExistentClaim, "claim on missing uuid must return false"); + pageProvider.loadPageData(spots(8)); - env.destroyInstance(instance, true); + IllegalArgumentException exception = assertThrows( + IllegalArgumentException.class, + () -> pageProvider.collectStartPages(12) + ); + assertEquals("Not enough pages to start the game", exception.getMessage()); } - /** - * Reproduces the lost-update race on {@code currentFoundedPageCount}: with a plain {@code int} and - * {@code ++}, concurrent finds of distinct pages could overwrite each other's increment and the - * displayed count would end up below the real number found, sometimes preventing the completion - * event from ever firing. - */ @Test - void testConcurrentDistinctFinds(@NotNull Env env) throws Exception { + void testCollectedPagesOnlyAppearOnSpawn(@NotNull Env env) { Instance instance = env.createFlatInstance(); - int pageCount = 6; - + instance.loadChunk(0, 0).join(); PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData( - IntStream.range(0, pageCount) - .mapToObj(i -> new PageResource(new Pos(i, 0, 0), Direction.NORTH)) - .collect(Collectors.toSet()) - ); - pageProvider.setMaxPageAmount(pageCount); - - List entities = IntStream.range(0, pageCount) - .mapToObj(i -> new PageEntity(instance, Pos.ZERO, i + 1)) - .toList(); - seedActivePages(pageProvider, entities.toArray(new PageEntity[0])); - - Player player = env.createPlayer(instance); + pageProvider.loadPageData(spots(MIN_ACTIVE_PAGE_COUNT)); - AtomicInteger completedEvents = new AtomicInteger(); - env.process().eventHandler().addListener(PageDiscoveryCompletedEvent.class, event -> completedEvents.incrementAndGet()); + pageProvider.collectStartPages(MIN_ACTIVE_PAGE_COUNT); + assertTrue(pageProvider.interactablePages().stream().allMatch(page -> page.getInstance() == null), + "collecting must not put any page into the world yet"); - runConcurrently(entities.stream() - .map(entity -> (Runnable) () -> pageProvider.triggerPageFound(player, entity.getHitBoxUUID())) - .toList()); - - assertEquals(pageCount + " / " + pageCount, plainStatus(pageProvider), - "every concurrent find must be counted, a lost update would leave the status below " + pageCount); - assertEquals(1, completedEvents.get(), "the completion event must fire exactly once once all pages are found"); + pageProvider.spawn(instance); + assertTrue(pageProvider.interactablePages().stream().allMatch(page -> page.getInstance() == instance), + "spawn must place every collected page"); env.destroyInstance(instance, true); } - /** - * Reproduces the race between game-end cleanup and an in-flight pickup: before the fix, - * {@code cleanUp()} iterated the map without any guard while another thread could mutate it - * concurrently via {@code triggerPageFound}. - */ @Test - void testConcurrentCleanUp(@NotNull Env env) throws Exception { + void testInteractablePagesOnlyListsCollectiblePages(@NotNull Env env) { Instance instance = env.createFlatInstance(); - int pageCount = 6; + PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT); - PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData( - IntStream.range(0, pageCount) - .mapToObj(i -> new PageResource(new Pos(i, 0, 0), Direction.NORTH)) - .collect(Collectors.toSet()) - ); - pageProvider.setMaxPageAmount(pageCount); - - List entities = IntStream.range(0, pageCount) - .mapToObj(i -> new PageEntity(instance, Pos.ZERO, i + 1)) - .toList(); - seedActivePages(pageProvider, entities.toArray(new PageEntity[0])); - - Player player = env.createPlayer(instance); - - List tasks = new ArrayList<>(entities.stream() - .map(entity -> (Runnable) () -> pageProvider.triggerPageFound(player, entity.getHitBoxUUID())) - .toList()); - tasks.add(pageProvider::cleanUp); + PageEntity expired = pageProvider.interactablePages().getFirst(); + expired.disableInteraction(); - assertDoesNotThrow(() -> runConcurrently(tasks)); + assertEquals(MIN_ACTIVE_PAGE_COUNT - 1, pageProvider.interactablePages().size()); + assertFalse(pageProvider.interactablePages().contains(expired), + "an expired page is invisible to the player and must not be announced by a sound"); env.destroyInstance(instance, true); } @Test - void testInteractablePagePositionsOnlyListsCollectiblePages(@NotNull Env env) throws Exception { + void testAClaimOnAnUnknownPageCountsNothing(@NotNull Env env) { Instance instance = env.createFlatInstance(); - PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData(Set.of(new PageResource(Pos.ZERO, Direction.NORTH))); - - PageEntity collectible = new PageEntity(instance, new Pos(10, 64, 20), 1); - PageEntity expired = new PageEntity(instance, new Pos(-5, 64, 7), 2); - expired.disableInteraction(); - seedActivePages(pageProvider, collectible, expired); - - List positions = pageProvider.interactablePagePositions(); + PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT); + Player player = env.createPlayer(instance); + AtomicInteger events = new AtomicInteger(); + env.process().eventHandler().addListener(PageFoundEvent.class, event -> events.incrementAndGet()); - assertEquals(List.of(new Pos(10, 64, 20)), positions, - "an expired page is invisible to the player and must not be announced by a sound"); + assertFalse(pageProvider.triggerPageFound(player, UUID.randomUUID()), "a claim on a missing uuid must return false"); + assertEquals(0, events.get(), "a claim that finds nothing must not raise the tension"); env.destroyInstance(instance, true); } @Test - void testEveryFindFiresAnEventCarryingTheRunningCount(@NotNull Env env) throws Exception { + void testEveryFindFiresAnEventCarryingTheRunningCount(@NotNull Env env) { Instance instance = env.createFlatInstance(); int pageCount = 3; - - PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData( - IntStream.range(0, pageCount) - .mapToObj(i -> new PageResource(new Pos(i, 0, 0), Direction.NORTH)) - .collect(Collectors.toSet()) - ); + PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT); pageProvider.setMaxPageAmount(pageCount); - - List entities = IntStream.range(0, pageCount) - .mapToObj(i -> new PageEntity(instance, Pos.ZERO, i + 1)) - .toList(); - seedActivePages(pageProvider, entities.toArray(new PageEntity[0])); - Player player = env.createPlayer(instance); List events = Collections.synchronizedList(new ArrayList<>()); env.process().eventHandler().addListener(PageFoundEvent.class, events::add); - for (PageEntity entity : entities) { - pageProvider.triggerPageFound(player, entity.getHitBoxUUID()); + for (PageEntity page : pageProvider.interactablePages().subList(0, pageCount)) { + pageProvider.triggerPageFound(player, page.getHitBoxUUID()); } assertEquals(List.of(1, 2, 3), events.stream().map(PageFoundEvent::foundCount).toList(), @@ -255,89 +155,106 @@ void testEveryFindFiresAnEventCarryingTheRunningCount(@NotNull Env env) throws E env.destroyInstance(instance, true); } + /** + * Reproduces the race where two players interact with the same page hitbox at almost the same time. + * Before the fix, the losing call dereferenced a {@code null} {@link PageEntity} and threw an NPE; + * it must now bail out silently and the winner's count must still be recorded correctly. + */ @Test - void testAClaimOnAnUnknownPageFiresNoEvent(@NotNull Env env) throws Exception { + void testConcurrentDuplicateFind(@NotNull Env env) throws Exception { Instance instance = env.createFlatInstance(); - PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData(Set.of(new PageResource(Pos.ZERO, Direction.NORTH))); - pageProvider.setMaxPageAmount(2); - - PageEntity pageEntity = new PageEntity(instance, Pos.ZERO, 1); - seedActivePages(pageProvider, pageEntity); - + PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT); + pageProvider.setMaxPageAmount(1); Player player = env.createPlayer(instance); + UUID uuid = pageProvider.interactablePages().getFirst().getHitBoxUUID(); - AtomicInteger events = new AtomicInteger(); - env.process().eventHandler().addListener(PageFoundEvent.class, event -> events.incrementAndGet()); + AtomicInteger completedEvents = new AtomicInteger(); + env.process().eventHandler().addListener(PageDiscoveryCompletedEvent.class, event -> completedEvents.incrementAndGet()); - pageProvider.triggerPageFound(player, UUID.randomUUID()); + runConcurrently(Collections.nCopies(8, (Runnable) () -> pageProvider.triggerPageFound(player, uuid))); - assertEquals(0, events.get(), "a claim that finds nothing must not raise the tension"); + assertEquals(1, completedEvents.get(), "the completion event must fire exactly once, not zero or more than once"); + assertEquals("1 / 1", plainStatus(pageProvider)); env.destroyInstance(instance, true); } + /** + * Reproduces the lost-update race on the found counter: with a plain {@code int} and {@code ++}, + * concurrent finds of distinct pages could overwrite each other's increment and the displayed + * count would end up below the real number found, sometimes preventing the completion event from + * ever firing. + */ @Test - void testAnExpiredPageHandsItsSpotBackToThePool(@NotNull Env env) { + void testConcurrentDistinctFinds(@NotNull Env env) throws Exception { Instance instance = env.createFlatInstance(); - // One spare spot: without the expired spots coming back, the second expiry finds the pool empty. - PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT + 1); + PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT); + pageProvider.setMaxPageAmount(MIN_ACTIVE_PAGE_COUNT); + Player player = env.createPlayer(instance); - PageEntity page = pageProvider.interactablePages().getFirst(); - Pos startSpot = page.getPosition(); + AtomicInteger completedEvents = new AtomicInteger(); + env.process().eventHandler().addListener(PageDiscoveryCompletedEvent.class, event -> completedEvents.incrementAndGet()); - pageProvider.triggerTTLHandling(page.getHitBoxUUID()); - Pos spareSpot = page.getPosition(); - assertNotEquals(startSpot, spareSpot, "an expired page must move to a new spot"); + runConcurrently(findTasks(pageProvider, player)); - pageProvider.triggerTTLHandling(page.getHitBoxUUID()); - assertEquals(startSpot, page.getPosition(), "the spot it expired on must be back in the pool"); + assertEquals(MIN_ACTIVE_PAGE_COUNT + " / " + MIN_ACTIVE_PAGE_COUNT, plainStatus(pageProvider), + "every concurrent find must be counted, a lost update would leave the status below " + MIN_ACTIVE_PAGE_COUNT); + assertEquals(1, completedEvents.get(), "the completion event must fire exactly once once all pages are found"); env.destroyInstance(instance, true); } + /** + * Reproduces the race between game-end cleanup and an in-flight pickup: before the fix, + * {@code cleanUp()} iterated the map without any guard while another thread could mutate it + * concurrently via {@code triggerPageFound}. + */ @Test - void testAFoundSpotIsNotHandedBackToThePool(@NotNull Env env) throws Exception { + void testConcurrentCleanUp(@NotNull Env env) { Instance instance = env.createFlatInstance(); - PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT + 1); + PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT); Player player = env.createPlayer(instance); - PageEntity page = pageProvider.interactablePages().getFirst(); - assertTrue(pageProvider.triggerPageFound(player, page.getHitBoxUUID())); - assertTrue(globalCache(pageProvider).isEmpty(), "the find used up the spare spot"); + List tasks = new ArrayList<>(findTasks(pageProvider, player)); + tasks.add(pageProvider::cleanUp); - // The page now stands on the former spare spot. Expiring it must not bring the found spot back. - pageProvider.triggerTTLHandling(page.getHitBoxUUID()); - assertTrue(globalCache(pageProvider).isEmpty(), "a found spot must stay used up"); + assertDoesNotThrow(() -> runConcurrently(tasks)); env.destroyInstance(instance, true); } @Test - void testAnExpiredPageIsNotCountedAsFound(@NotNull Env env) { + void testAnExpiredPageMovesOnAndHandsItsSpotBack(@NotNull Env env) { Instance instance = env.createFlatInstance(); + // One spare spot: without the expired spots coming back, the second expiry finds the pool empty PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT + 1); AtomicInteger finds = new AtomicInteger(); env.process().eventHandler().addListener(PageFoundEvent.class, event -> finds.incrementAndGet()); String statusBefore = plainStatus(pageProvider); PageEntity page = pageProvider.interactablePages().getFirst(); - pageProvider.triggerTTLHandling(page.getHitBoxUUID()); + Pos startSpot = page.getPosition(); + pageProvider.triggerTTLHandling(page.getHitBoxUUID()); + assertNotEquals(startSpot, page.getPosition(), "an expired page must move to a new spot"); + assertTrue(page.isInteractable(), "the expired page is collectible again on its new spot"); assertEquals(0, finds.get(), "an expiry must not raise a find"); assertEquals(statusBefore, plainStatus(pageProvider), "an expiry must not change the found count"); - assertTrue(page.isInteractable(), "the expired page is collectible again on its new spot"); + + pageProvider.triggerTTLHandling(page.getHitBoxUUID()); + assertEquals(startSpot, page.getPosition(), "the spot it expired on must be back in the pool"); env.destroyInstance(instance, true); } @Test - void testAFoundPageStartsFreshOnItsNewSpot(@NotNull Env env) throws Exception { + void testAFoundPageStartsFreshOnTheSpareSpotAndUsesItsSpotUp(@NotNull Env env) throws Exception { Instance instance = env.createFlatInstance(); PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT + 1); Player player = env.createPlayer(instance); PageEntity page = pageProvider.interactablePages().getFirst(); + Pos foundAt = page.getPosition(); // Almost run out on its old spot Field tickTime = PageEntity.class.getDeclaredField("currentTickTime"); tickTime.setAccessible(true); @@ -345,48 +262,67 @@ void testAFoundPageStartsFreshOnItsNewSpot(@NotNull Env env) throws Exception { assertTrue(pageProvider.triggerPageFound(player, page.getHitBoxUUID())); + Pos spareSpot = page.getPosition(); + assertNotEquals(foundAt, spareSpot, "a found page must move to the spare spot"); assertEquals(1.0, page.remainingTtlRatio(), "the page must get its full time on the new spot"); ItemStack shown = ((ItemDisplayMeta) page.getEntityMeta()).getItemStack(); assertEquals(page.getPageItem(), shown, "the page on the wall must be the one the next finder gets"); + // The pool is empty now. Had the found spot come back, the expiry would move the page onto it. + pageProvider.triggerTTLHandling(page.getHitBoxUUID()); + assertEquals(spareSpot, page.getPosition(), "a found spot must stay used up"); + + env.destroyInstance(instance, true); + } + + @Test + void testAFoundPageWithoutAFreeSpotIsHiddenBeforeItComesBack(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + // No spare spot: the found page has nowhere else to go + PageProvider pageProvider = spawnedProvider(instance, MIN_ACTIVE_PAGE_COUNT); + Player player = env.createPlayer(instance); + + PageEntity page = pageProvider.interactablePages().getFirst(); + Pos foundAt = page.getPosition(); + assertTrue(pageProvider.triggerPageFound(player, page.getHitBoxUUID())); + + assertEquals(foundAt, page.getPosition(), "without a free spot the page has to stay where it was found"); + assertFalse(page.isInteractable(), "the page must not be collectible again right away"); + assertFalse(pageProvider.interactablePages().contains(page)); + assertFalse(pageProvider.triggerPageFound(player, page.getHitBoxUUID()), "a click on the hidden page must not count"); + env.destroyInstance(instance, true); } + /** + * Creates a provider with {@value GameConfig#MIN_ACTIVE_PAGE_COUNT} pages placed into the instance. + * Every spot beyond that stays free in the pool. + */ private static PageProvider spawnedProvider(Instance instance, int spotCount) { // All spots share one chunk, loaded up front: placing or teleporting a page into a chunk that // isn't loaded yet only completes asynchronously, and the assertions would race it. instance.loadChunk(0, 0).join(); PageProvider pageProvider = new PageProvider(); - pageProvider.loadPageData( - IntStream.range(0, spotCount) - .mapToObj(i -> new PageResource(new Pos(i, 40, 0), Direction.NORTH)) - .collect(Collectors.toSet()) - ); - pageProvider.setMaxPageAmount(100); - pageProvider.collectStartPages(instance); - pageProvider.spawn(); + pageProvider.loadPageData(spots(spotCount)); + pageProvider.collectStartPages(MIN_ACTIVE_PAGE_COUNT); + pageProvider.spawn(instance); return pageProvider; } - @SuppressWarnings("unchecked") - private static Queue globalCache(PageProvider pageProvider) throws ReflectiveOperationException { - Field field = PageProvider.class.getDeclaredField("globalCache"); - field.setAccessible(true); - return (Queue) field.get(pageProvider); + private static Set spots(int count) { + return IntStream.range(0, count) + .mapToObj(i -> new PageResource(new Pos(i, 40, 0), Direction.NORTH)) + .collect(Collectors.toSet()); } - private static String plainStatus(PageProvider pageProvider) { - return PlainTextComponentSerializer.plainText().serialize(pageProvider.getPageStatus()); + private static List findTasks(PageProvider pageProvider, Player player) { + return pageProvider.interactablePages().stream() + .map(page -> (Runnable) () -> pageProvider.triggerPageFound(player, page.getHitBoxUUID())) + .toList(); } - @SuppressWarnings("unchecked") - private static void seedActivePages(PageProvider pageProvider, PageEntity... entities) throws ReflectiveOperationException { - Field field = PageProvider.class.getDeclaredField("activePages"); - field.setAccessible(true); - Map activePages = (Map) field.get(pageProvider); - for (PageEntity entity : entities) { - activePages.put(entity.getHitBoxUUID(), entity); - } + private static String plainStatus(PageProvider pageProvider) { + return PlainTextComponentSerializer.plainText().serialize(pageProvider.getPageStatus()); } /** diff --git a/common/src/test/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculationTest.java b/common/src/test/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculationTest.java index 2cd4ae7d..81a97d21 100644 --- a/common/src/test/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculationTest.java +++ b/common/src/test/java/net/onelitefeather/cygnus/common/util/HealthScalingCalculationTest.java @@ -26,7 +26,7 @@ void testAdditionalHealthCount(int count, @NotNull Env env) { env.createPlayer(instance); } - double additionalHealth = HealthScalingCalculation.getAdditionalHealth(count); + double additionalHealth = HealthScalingCalculation.getAdditionalHealth(); assertNotEquals(0.0D, additionalHealth); assertTrue(additionalHealth <= 20.0D); assertEquals(0.0D, additionalHealth % 2.0D, 0.0001D, "additional health should always be a whole heart (multiple of 2 HP)"); @@ -35,8 +35,15 @@ void testAdditionalHealthCount(int count, @NotNull Env env) { } @Test - void testZeroHealthScaling() { - assertEquals(0.0D, HealthScalingCalculation.getAdditionalHealth(12)); + void testZeroHealthScalingFromFourSurvivors(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + for (int i = 0; i < 5; i++) { + env.createPlayer(instance); + } + + assertEquals(0.0D, HealthScalingCalculation.getAdditionalHealth()); + + env.destroyInstance(instance, true); } @Test diff --git a/common/src/test/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculationTest.java b/common/src/test/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculationTest.java index 0300d0c4..d83d8937 100644 --- a/common/src/test/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculationTest.java +++ b/common/src/test/java/net/onelitefeather/cygnus/common/util/SpeedScalingCalculationTest.java @@ -24,7 +24,7 @@ void testAdditionalSpeedCount(int count, @NotNull Env env) { env.createPlayer(instance); } - double additionalSpeed = SpeedScalingCalculation.getAdditionalSpeed(count); + double additionalSpeed = SpeedScalingCalculation.getAdditionalSpeed(); assertNotEquals(0.0D, additionalSpeed); assertTrue(additionalSpeed <= 0.01D); @@ -32,7 +32,14 @@ void testAdditionalSpeedCount(int count, @NotNull Env env) { } @Test - void testZeroSpeedScaling() { - assertEquals(0.0D, SpeedScalingCalculation.getAdditionalSpeed(12)); + void testZeroSpeedScalingFromFourSurvivors(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + for (int i = 0; i < 5; i++) { + env.createPlayer(instance); + } + + assertEquals(0.0D, SpeedScalingCalculation.getAdditionalSpeed()); + + env.destroyInstance(instance, true); } } diff --git a/game/src/main/java/net/onelitefeather/cygnus/Cygnus.java b/game/src/main/java/net/onelitefeather/cygnus/Cygnus.java index 8bed1822..f426d869 100644 --- a/game/src/main/java/net/onelitefeather/cygnus/Cygnus.java +++ b/game/src/main/java/net/onelitefeather/cygnus/Cygnus.java @@ -257,7 +257,7 @@ private void registerGameListener() { handler.addListener(PlayerStopSprintingEvent.class, new PlayerStopSprintingListener(this.staminaService::getFoodBar)); handler.addListener(SlenderReviveEvent.class, new SlenderReviveListener(this.mapProvider::getGameMap, this.staminaService, this.teamService)); handler.addListener(SlenderReviveEvent.class, _ -> this.scoreboardDisplay.sync(this.teamService)); - handler.addListener(GamePreLaunchEvent.class, new GamePreLaunchListener(this.pageProvider::setMaxPageAmount)); + handler.addListener(GamePreLaunchEvent.class, new GamePreLaunchListener(this.pageProvider)); handler.addListener(StaminaStateChangeEvent.class, new StaminaStateChangeListener()); handler.addListener(PageDiscoveryCompletedEvent.class, new PageDiscoveryCompleteListener(this.linearPhaseSeries)); handler.addListener(ViewUpdateEvent.class, new ViewUpdateListener(this.view, this.pageProvider)); diff --git a/game/src/main/java/net/onelitefeather/cygnus/listener/game/GamePreLaunchListener.java b/game/src/main/java/net/onelitefeather/cygnus/listener/game/GamePreLaunchListener.java index 86a4a023..02f41c80 100644 --- a/game/src/main/java/net/onelitefeather/cygnus/listener/game/GamePreLaunchListener.java +++ b/game/src/main/java/net/onelitefeather/cygnus/listener/game/GamePreLaunchListener.java @@ -4,24 +4,23 @@ import net.minestom.server.entity.Player; import net.minestom.server.network.ConnectionManager; import net.onelitefeather.cygnus.attribute.AttributeHelper; -import net.onelitefeather.cygnus.common.config.GameConfig; import net.onelitefeather.cygnus.common.event.GamePreLaunchEvent; import net.onelitefeather.cygnus.common.page.PageCalculation; +import net.onelitefeather.cygnus.common.page.PageProvider; import net.onelitefeather.cygnus.common.util.HealthScalingCalculation; import net.onelitefeather.cygnus.common.util.SpeedScalingCalculation; import net.onelitefeather.cygnus.team.TeamHelper; import java.util.function.Consumer; -import java.util.function.IntConsumer; @SuppressWarnings("java:S3252") public class GamePreLaunchListener implements Consumer { private final ConnectionManager connectionManager; - private final IntConsumer pageCounter; + private final PageProvider pageProvider; - public GamePreLaunchListener(IntConsumer pageCounter) { - this.pageCounter = pageCounter; + public GamePreLaunchListener(PageProvider pageProvider) { + this.pageProvider = pageProvider; this.connectionManager = MinecraftServer.getConnectionManager(); } @@ -29,15 +28,13 @@ public GamePreLaunchListener(IntConsumer pageCounter) { public void accept(GamePreLaunchEvent event) { int pageCount = PageCalculation.calculatePageAmount(); if (pageCount == 0) throw new UnsupportedOperationException("No pages found"); - pageCounter.accept(pageCount); + this.pageProvider.setMaxPageAmount(pageCount); + // Only picked here, placed into the world once the round starts + this.pageProvider.collectStartPages(PageCalculation.calculateActivePageAmount()); - float adjustedHealth = 0; - double adjustedSpeed = 0; - - if (pageCount <= GameConfig.MIN_PAGE_COUNT) { - adjustedHealth = HealthScalingCalculation.getAdditionalHealth(pageCount); - adjustedSpeed = SpeedScalingCalculation.getAdditionalSpeed(pageCount); - } + // Based on the players, not on the page count: the page count carries a random jitter + float adjustedHealth = HealthScalingCalculation.getAdditionalHealth(); + double adjustedSpeed = SpeedScalingCalculation.getAdditionalSpeed(); for (Player player : connectionManager.getOnlinePlayers()) { AttributeHelper.adjustStepHeightAndJump(player); diff --git a/game/src/main/java/net/onelitefeather/cygnus/listener/game/GameStartListener.java b/game/src/main/java/net/onelitefeather/cygnus/listener/game/GameStartListener.java index dea57821..28093fa2 100644 --- a/game/src/main/java/net/onelitefeather/cygnus/listener/game/GameStartListener.java +++ b/game/src/main/java/net/onelitefeather/cygnus/listener/game/GameStartListener.java @@ -1,8 +1,10 @@ package net.onelitefeather.cygnus.listener.game; import net.kyori.adventure.text.Component; +import net.minestom.server.MinecraftServer; import net.minestom.server.entity.Player; import net.minestom.server.event.EventDispatcher; +import net.minestom.server.timer.TaskSchedule; import net.onelitefeather.cygnus.ambient.AmbientProvider; import net.onelitefeather.cygnus.common.Messages; import net.onelitefeather.cygnus.common.Tags; @@ -19,10 +21,13 @@ import net.theevilreaper.xerus.api.team.Team; import net.theevilreaper.xerus.api.team.TeamService; +import java.util.concurrent.ThreadLocalRandom; import java.util.function.Consumer; public final class GameStartListener implements Consumer { + private static final int TICKS_PER_SECOND = 20; + private final TeamService teamService; private final AmbientProvider ambientProvider; private final StaminaService staminaService; @@ -72,10 +77,23 @@ private void handleSurvivorStart() { private void startGlobalMechanics() { this.staminaService.start(); - EventDispatcher.call(new PageSpawnEvent()); this.ambientProvider.startTask(); - // Started after the pages exist: PageSpawnEvent above is what fills the active page map. - this.pageProximityService.startTask(); + // Delayed so survivors get a moment to move away from the spawn point before the first + // pages appear, instead of one being reachable the instant the round starts. The proximity + // task starts alongside it, in the same task, since it depends on pages already existing. + // Jittered so the moment doesn't land on the exact same tick every round. + MinecraftServer.getSchedulerManager().buildTask(() -> { + EventDispatcher.call(new PageSpawnEvent()); + this.pageProximityService.startTask(); + }).delay(TaskSchedule.tick(randomizedSpawnDelayTicks())).schedule(); TeamHelper.updateTabList(this.teamService); } + + private int randomizedSpawnDelayTicks() { + int baseTicks = GameConfig.PAGE_SPAWN_DELAY * TICKS_PER_SECOND; + int jitterTicks = GameConfig.PAGE_SPAWN_DELAY_JITTER * TICKS_PER_SECOND; + ThreadLocalRandom current = ThreadLocalRandom.current(); + int offset = current.nextInt(2 * jitterTicks + 1) - jitterTicks; + return baseTicks + offset; + } } diff --git a/game/src/main/java/net/onelitefeather/cygnus/listener/page/PageSpawnListener.java b/game/src/main/java/net/onelitefeather/cygnus/listener/page/PageSpawnListener.java index 6dda0456..2238ee0c 100644 --- a/game/src/main/java/net/onelitefeather/cygnus/listener/page/PageSpawnListener.java +++ b/game/src/main/java/net/onelitefeather/cygnus/listener/page/PageSpawnListener.java @@ -8,7 +8,8 @@ import java.util.function.Supplier; /** - * Collects the start pages for the active instance and spawns them once the game has started. + * Places the start pages into the active instance once the game has started. The pages are + * collected beforehand, see {@code GamePreLaunchListener}. * * @author theEvilReaper * @version 1.0.0 @@ -28,9 +29,8 @@ public PageSpawnListener(PageProvider pageProvider, Supplier activeIns public void accept(PageSpawnEvent event) { Instance activeInstance = this.activeInstanceSupplier.get(); if (activeInstance == null) { - throw new IllegalStateException("Active instance not available for page collection"); + throw new IllegalStateException("Active instance not available for page spawning"); } - this.pageProvider.collectStartPages(activeInstance); - this.pageProvider.spawn(); + this.pageProvider.spawn(activeInstance); } } diff --git a/game/src/main/java/net/onelitefeather/cygnus/phase/WaitingPhase.java b/game/src/main/java/net/onelitefeather/cygnus/phase/WaitingPhase.java index e86d4ffb..7fc2ca4e 100644 --- a/game/src/main/java/net/onelitefeather/cygnus/phase/WaitingPhase.java +++ b/game/src/main/java/net/onelitefeather/cygnus/phase/WaitingPhase.java @@ -35,7 +35,6 @@ public WaitingPhase(GameView gameView, VoidConsumer instanceSwitch, VoidConsumer @Override public void onStart() { super.onStart(); - EventDispatcher.call(new GamePreLaunchEvent()); this.instanceSwitch.apply(); } @@ -47,6 +46,8 @@ protected void onFinish() { @Override public void onUpdate() { if (getCurrentTicks() == 1) { + // Right before the end, so the page counts are based on the players that actually start + EventDispatcher.call(new GamePreLaunchEvent()); this.teleportLogic.apply(); } } diff --git a/game/src/test/java/net/onelitefeather/cygnus/listener/game/GamePreLaunchListenerTest.java b/game/src/test/java/net/onelitefeather/cygnus/listener/game/GamePreLaunchListenerTest.java new file mode 100644 index 00000000..4c24f2cd --- /dev/null +++ b/game/src/test/java/net/onelitefeather/cygnus/listener/game/GamePreLaunchListenerTest.java @@ -0,0 +1,63 @@ +package net.onelitefeather.cygnus.listener.game; + +import net.minestom.server.coordinate.Pos; +import net.minestom.server.entity.Player; +import net.minestom.server.entity.attribute.Attribute; +import net.minestom.server.instance.Instance; +import net.minestom.server.utils.Direction; +import net.minestom.testing.Env; +import net.minestom.testing.extension.MicrotusExtension; +import net.onelitefeather.cygnus.common.config.GameConfig; +import net.onelitefeather.cygnus.common.event.GamePreLaunchEvent; +import net.onelitefeather.cygnus.common.page.PageProvider; +import net.onelitefeather.cygnus.common.page.PageResource; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +import static org.junit.jupiter.api.Assertions.*; + +@ExtendWith(MicrotusExtension.class) +class GamePreLaunchListenerTest { + + @Test + void testPreLaunchSetsThePageAmountAndCollectsTheStartPages(@NotNull Env env) { + PageProvider pageProvider = loadedProvider(); + + new GamePreLaunchListener(pageProvider).accept(new GamePreLaunchEvent()); + + assertTrue(pageProvider.getMaxPageAmount() >= GameConfig.MIN_PAGE_COUNT); + assertEquals(GameConfig.MIN_ACTIVE_PAGE_COUNT, pageProvider.interactablePages().size(), + "without any players the round starts with the minimum of active pages"); + assertTrue(pageProvider.interactablePages().stream().allMatch(page -> page.getInstance() == null), + "the pages are only collected here, the round start places them"); + } + + @Test + void testTwoPlayersAlwaysGetTheExtraHearts(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + Player survivor = env.createPlayer(instance); + env.createPlayer(instance); + + new GamePreLaunchListener(loadedProvider()).accept(new GamePreLaunchEvent()); + + // The page count carries a random jitter, so the bonus must not depend on it + assertTrue(survivor.getAttributeValue(Attribute.MAX_HEALTH) > 20.0D, + "a small lobby must always get the extra hearts"); + + env.destroyInstance(instance, true); + } + + private static PageProvider loadedProvider() { + PageProvider pageProvider = new PageProvider(); + pageProvider.loadPageData( + IntStream.range(0, GameConfig.MIN_ACTIVE_PAGE_COUNT * 2) + .mapToObj(i -> new PageResource(new Pos(i, 40, 0), Direction.NORTH)) + .collect(Collectors.toSet()) + ); + return pageProvider; + } +} diff --git a/game/src/test/java/net/onelitefeather/cygnus/listener/game/GameStartListenerTest.java b/game/src/test/java/net/onelitefeather/cygnus/listener/game/GameStartListenerTest.java new file mode 100644 index 00000000..d1b18a78 --- /dev/null +++ b/game/src/test/java/net/onelitefeather/cygnus/listener/game/GameStartListenerTest.java @@ -0,0 +1,104 @@ +package net.onelitefeather.cygnus.listener.game; + +import net.minestom.server.instance.Instance; +import net.minestom.testing.Env; +import net.onelitefeather.cygnus.CygnusPlayerTestBase; +import net.onelitefeather.cygnus.ambient.AmbientProvider; +import net.onelitefeather.cygnus.common.config.GameConfig; +import net.onelitefeather.cygnus.common.config.GameConfigReader; +import net.onelitefeather.cygnus.common.page.PageProvider; +import net.onelitefeather.cygnus.common.page.event.PageSpawnEvent; +import net.onelitefeather.cygnus.event.GameStartEvent; +import net.onelitefeather.cygnus.page.PageProximityService; +import net.onelitefeather.cygnus.player.CygnusPlayer; +import net.onelitefeather.cygnus.stamina.StaminaService; +import net.onelitefeather.cygnus.team.TeamCreator; +import net.theevilreaper.xerus.api.team.Team; +import net.theevilreaper.xerus.api.team.TeamService; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Test; + +import java.nio.file.Paths; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Verifies that the first page spawn is delayed rather than happening synchronously with + * {@link GameStartEvent}, so survivors get a moment to move away from the spawn point first, and + * that the delay is jittered within {@code GameConfig.PAGE_SPAWN_DELAY} ± + * {@code GameConfig.PAGE_SPAWN_DELAY_JITTER} rather than landing on the exact same tick every round. + */ +class GameStartListenerTest extends CygnusPlayerTestBase { + + private static final int TICKS_PER_SECOND = 20; + + @Test + void pageSpawnEventNeverFiresBeforeTheMinimumJitteredDelay(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + GameStartListener listener = createListener(env, instance); + + AtomicBoolean pageSpawnFired = new AtomicBoolean(false); + env.process().eventHandler().addListener(PageSpawnEvent.class, event -> pageSpawnFired.set(true)); + + listener.accept(new GameStartEvent()); + // No random source is injectable here, but the jitter can never push the delay below + // PAGE_SPAWN_DELAY - PAGE_SPAWN_DELAY_JITTER regardless of what gets rolled. + int minDelayTicks = (GameConfig.PAGE_SPAWN_DELAY - GameConfig.PAGE_SPAWN_DELAY_JITTER) * TICKS_PER_SECOND; + for (int i = 0; i < minDelayTicks - 1; i++) env.tick(); + assertFalse(pageSpawnFired.get(), "the page spawn must not fire before the minimum jittered delay"); + + // Drain the scheduled task here (whatever it actually rolled), unasserted, so it can't leak + // into a later test sharing this Env. + int maxDelayTicks = (GameConfig.PAGE_SPAWN_DELAY + GameConfig.PAGE_SPAWN_DELAY_JITTER) * TICKS_PER_SECOND; + for (int i = minDelayTicks - 1; i < maxDelayTicks + 5; i++) env.tick(); + + env.destroyInstance(instance, true); + } + + @Test + void pageSpawnEventAlwaysFiresByTheMaximumJitteredDelay(@NotNull Env env) { + Instance instance = env.createFlatInstance(); + GameStartListener listener = createListener(env, instance); + + AtomicBoolean pageSpawnFired = new AtomicBoolean(false); + env.process().eventHandler().addListener(PageSpawnEvent.class, event -> pageSpawnFired.set(true)); + + listener.accept(new GameStartEvent()); + // Jitter can only push the delay later, never past PAGE_SPAWN_DELAY + PAGE_SPAWN_DELAY_JITTER. + int maxDelayTicks = (GameConfig.PAGE_SPAWN_DELAY + GameConfig.PAGE_SPAWN_DELAY_JITTER) * TICKS_PER_SECOND; + for (int i = 0; i < maxDelayTicks + 5; i++) env.tick(); + + assertTrue(pageSpawnFired.get(), "the page spawn must fire once the configured delay has passed"); + + env.destroyInstance(instance, true); + } + + private static GameStartListener createListener(Env env, Instance instance) { + CygnusPlayer slender = (CygnusPlayer) env.createPlayer(instance); + CygnusPlayer survivor = (CygnusPlayer) env.createPlayer(instance); + + GameConfig gameConfig = new GameConfigReader(Paths.get("")).getConfig(); + TeamService teamService = TeamService.of(); + TeamCreator teamCreator = new TeamCreator() {}; + teamCreator.createTeams(gameConfig, teamService); + + Team slenderTeam = teamService.getTeam(GameConfig.SLENDER_KEY).orElseThrow(); + Team survivorTeam = teamService.getTeam(GameConfig.SURVIVOR_KEY).orElseThrow(); + slenderTeam.addPlayer(slender); + survivorTeam.addPlayer(survivor); + + AmbientProvider ambientProvider = new AmbientProvider(survivorTeam); + StaminaService staminaService = new StaminaService(); + PageProvider pageProvider = new PageProvider(); + PageProximityService pageProximityService = new PageProximityService( + gameConfig, + survivorTeam::getPlayers, + List::of + ); + + return new GameStartListener(teamService, ambientProvider, staminaService, pageProvider, pageProximityService); + } +} diff --git a/game/src/test/java/net/onelitefeather/cygnus/listener/page/PageSpawnListenerTest.java b/game/src/test/java/net/onelitefeather/cygnus/listener/page/PageSpawnListenerTest.java index d1127a25..16d544bf 100644 --- a/game/src/test/java/net/onelitefeather/cygnus/listener/page/PageSpawnListenerTest.java +++ b/game/src/test/java/net/onelitefeather/cygnus/listener/page/PageSpawnListenerTest.java @@ -12,9 +12,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import java.lang.reflect.Field; -import java.util.Map; -import java.util.UUID; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -34,37 +31,28 @@ void acceptThrowsWhenActiveInstanceIsUnavailable() { IllegalStateException.class, () -> listener.accept(event) ); - assertEquals("Active instance not available for page collection", exception.getMessage()); + assertEquals("Active instance not available for page spawning", exception.getMessage()); } @Test - void acceptCollectsThePagesForTheActiveInstanceBeforeSpawning(@NotNull Env env) throws Exception { + void acceptPlacesTheCollectedPagesIntoTheActiveInstance(@NotNull Env env) { Instance instance = env.createFlatInstance(); + instance.loadChunk(0, 0).join(); PageProvider pageProvider = new PageProvider(); pageProvider.loadPageData( IntStream.range(0, MIN_ACTIVE_PAGE_COUNT) - .mapToObj(i -> new PageResource(new Pos(i, 0, 0), Direction.NORTH)) + .mapToObj(i -> new PageResource(new Pos(i, 40, 0), Direction.NORTH)) .collect(Collectors.toSet()) ); + pageProvider.collectStartPages(MIN_ACTIVE_PAGE_COUNT); - PageSpawnListener listener = new PageSpawnListener(pageProvider, () -> instance); + new PageSpawnListener(pageProvider, () -> instance).accept(new PageSpawnEvent()); - assertEquals(0, activePageCount(pageProvider), "no pages should exist before the event is handled"); - - assertDoesNotThrow(() -> listener.accept(new PageSpawnEvent())); - - assertEquals(MIN_ACTIVE_PAGE_COUNT, activePageCount(pageProvider), - "collectStartPages must have run so spawn() has something to spawn"); + assertEquals(MIN_ACTIVE_PAGE_COUNT, pageProvider.interactablePages().size()); + assertTrue(pageProvider.interactablePages().stream().allMatch(page -> page.getInstance() == instance), + "every collected page must be placed into the active instance"); env.destroyInstance(instance, true); } - - @SuppressWarnings("unchecked") - private static int activePageCount(PageProvider pageProvider) throws ReflectiveOperationException { - Field field = PageProvider.class.getDeclaredField("activePages"); - field.setAccessible(true); - Map activePages = (Map) field.get(pageProvider); - return activePages.size(); - } } diff --git a/game/src/test/java/net/onelitefeather/cygnus/listener/page/PlayerPageInteractListenerTest.java b/game/src/test/java/net/onelitefeather/cygnus/listener/page/PlayerPageInteractListenerTest.java index ff12a114..4b46627b 100644 --- a/game/src/test/java/net/onelitefeather/cygnus/listener/page/PlayerPageInteractListenerTest.java +++ b/game/src/test/java/net/onelitefeather/cygnus/listener/page/PlayerPageInteractListenerTest.java @@ -16,7 +16,6 @@ import net.onelitefeather.cygnus.common.page.PageFactory; import net.onelitefeather.cygnus.common.page.PageProvider; import net.onelitefeather.cygnus.common.page.PageResource; -import net.onelitefeather.cygnus.common.util.Helper; import net.onelitefeather.cygnus.player.CygnusPlayer; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; @@ -42,7 +41,8 @@ void testPagePickup(@NotNull Env env) throws Exception { pageProvider.loadPageData(Set.of(new PageResource(Pos.ZERO, Direction.NORTH))); pageProvider.setMaxPageAmount(1); - PageEntity pageEntity = PageFactory.createPage(instance, Pos.ZERO, Direction.NORTH, 1); + PageEntity pageEntity = PageFactory.createPage(new PageResource(Pos.ZERO, Direction.NORTH), 1); + pageEntity.place(instance).join(); UUID hitBoxUuid = pageEntity.getHitBoxUUID(); seedActivePage(pageProvider, pageEntity); @@ -88,7 +88,8 @@ void testNonSurvivor(@NotNull Env env) throws Exception { pageProvider.loadPageData(Set.of(new PageResource(Pos.ZERO, Direction.NORTH))); pageProvider.setMaxPageAmount(1); - PageEntity pageEntity = PageFactory.createPage(instance, Pos.ZERO, Direction.NORTH, 1); + PageEntity pageEntity = PageFactory.createPage(new PageResource(Pos.ZERO, Direction.NORTH), 1); + pageEntity.place(instance).join(); UUID hitBoxUuid = pageEntity.getHitBoxUUID(); seedActivePage(pageProvider, pageEntity); @@ -127,8 +128,8 @@ void testFourSidedBlock(@NotNull Env env) throws Exception { int count = 1; for (Direction direction : directions) { - Pos pagePos = Helper.updatePosition(blockPos, direction); - PageEntity entity = PageFactory.createPage(instance, pagePos, direction, count++); + PageEntity entity = PageFactory.createPage(new PageResource(blockPos, direction), count++); + entity.place(instance).join(); entitiesByDir.put(direction, entity); seedActivePage(pageProvider, entity);