From b88fa0bfec6aa8c8dae0f813605ffdde8f861c56 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:57:46 +0200 Subject: [PATCH 1/7] feat(navigator): derive the menu layout from the entries a player may see A navigator with fixed slots cannot hide anything. If the build servers owned slots of their own, a player without titan.navigator.buildserver would find those slots empty every single time, and a slot that is only ever empty for some players is exactly the information US-5.02 and NFR-005 want kept back. So no slot belongs to an entry. NavigatorEntry carries the permission it needs - null means public - and NavigatorLayout filters first and places afterwards: the visible entries become one uninterrupted, centred block, computed from the visible entries alone. A filtered-out entry never existed as far as the layout is concerned, which gives the property the tests assert: the menu of a player without the permission is identical to the menu of a lobby that has no build servers at all. Nothing to count, no hole to notice. Entries that exceed the row are dropped from the end rather than spilling into a second row, so the menu keeps the size every player sees. --- .../common/navigator/NavigatorEntry.java | 104 +++++++++++++ .../common/navigator/NavigatorLayout.java | 106 +++++++++++++ .../titan/common/navigator/package-info.java | 22 +++ .../common/navigator/NavigatorLayoutTest.java | 142 ++++++++++++++++++ .../titan/common/navigator/TestAudience.java | 56 +++++++ 5 files changed, 430 insertions(+) create mode 100644 common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorEntry.java create mode 100644 common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorLayout.java create mode 100644 common/src/main/java/net/onelitefeather/titan/common/navigator/package-info.java create mode 100644 common/src/test/java/net/onelitefeather/titan/common/navigator/NavigatorLayoutTest.java create mode 100644 common/src/test/java/net/onelitefeather/titan/common/navigator/TestAudience.java diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorEntry.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorEntry.java new file mode 100644 index 0000000..8ee9250 --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorEntry.java @@ -0,0 +1,104 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.minestom.server.item.ItemStack; +import net.onelitefeather.deliver.DeliverComponent; +import net.onelitefeather.deliver.DeliverType; +import net.onelitefeather.titan.common.feature.FeatureAudience; +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.Nullable; + +import java.util.UUID; + +/** + * One destination the navigator can offer: the icon shown for it, where a click sends the player, + * and the permission a player needs to be offered it at all. + * + *

An entry with no permission is public. An entry with one is invisible to everyone who does + * not hold it — and invisible means gone, not greyed out and not a reserved slot, which is why + * {@link NavigatorLayout} places entries only after filtering them. + * + * @param icon the item shown in the menu + * @param type whether {@link #destination()} names a CloudNet task or a single service + * @param destination the task or service name a click connects to + * @param permission the permission required to see this entry, or {@code null} when it is public + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ +public record NavigatorEntry(ItemStack icon, DeliverType type, String destination, + @Nullable String permission) { + + /** + * Creates a public entry that connects to the best service of a CloudNet task. + * + * @param icon the item shown in the menu + * @param taskName the CloudNet task a click connects to + * @return an entry every player sees + */ + @Contract(value = "_, _ -> new", pure = true) + public static NavigatorEntry task(ItemStack icon, String taskName) { + return new NavigatorEntry(icon, DeliverType.TASK, taskName, null); + } + + /** + * Creates an entry that connects to one specific CloudNet service and is only offered to + * holders of the given permission. + * + * @param icon the item shown in the menu + * @param serviceName the CloudNet service a click connects to + * @param permission the permission required to see the entry + * @return an entry restricted to holders of that permission + */ + @Contract(value = "_, _, _ -> new", pure = true) + public static NavigatorEntry restrictedServer(ItemStack icon, String serviceName, String permission) { + return new NavigatorEntry(icon, DeliverType.SERVER, serviceName, permission); + } + + /** + * Checks whether this entry may be shown to the given player. + * + * @param playerId the player's unique id + * @param audience the source of permission answers + * @return whether the entry is public or the player holds its permission + */ + @Contract(pure = true) + public boolean isVisibleTo(UUID playerId, FeatureAudience audience) { + return this.permission == null || audience.hasPermission(playerId, this.permission); + } + + /** + * Builds the delivery request a click on this entry produces. + * + *

The request is only a request: whether the player is actually moved is decided again by + * {@link GuardedDeliver}, because the menu the click came from is not trustworthy evidence + * that the player was ever allowed to see this entry (US-5.03). + * + * @param playerId the player to move + * @return the component describing the requested switch + */ + @Contract(value = "_ -> new", pure = true) + public DeliverComponent toComponent(UUID playerId) { + return switch (this.type) { + case TASK -> + DeliverComponent.taskBuilder().playerId(playerId).taskName(this.destination).build(); + case SERVER -> + DeliverComponent.serverBuilder().playerId(playerId).serverName(this.destination).build(); + }; + } +} diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorLayout.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorLayout.java new file mode 100644 index 0000000..3539571 --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/NavigatorLayout.java @@ -0,0 +1,106 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.onelitefeather.titan.common.feature.FeatureAudience; +import org.jetbrains.annotations.Contract; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +/** + * Turns the entries a player may see into slot positions. + * + *

This is the answer to US-5.02 and NFR-005. A fixed layout cannot satisfy them: if the build + * servers owned slots of their own, a player without + * {@value BuildServerAccess#PERMISSION} would find those slots empty every time, and an empty + * slot that is only ever empty for some players is itself the information the requirement wants + * hidden. So no slot belongs to an entry. The visible entries are laid out as one uninterrupted, + * centred block, and the block is computed from the visible entries alone — a filtered-out entry + * never existed as far as the layout is concerned. + * + *

The consequence is the property the tests assert: the menu a player without the permission + * sees is byte-for-byte the menu of a lobby that has no build servers at all. There is nothing to + * count, nothing to compare against and no hole to notice. + * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ +public final class NavigatorLayout { + + private NavigatorLayout() { + throw new UnsupportedOperationException("This class cannot be instantiated"); + } + + /** + * Places the entries the player may see into a row of the given size. + * + *

Entries keep the order they were handed in. If more entries are visible than fit, the + * surplus is dropped from the end rather than spilling into a second row, so the menu keeps + * the size every player sees. + * + * @param entries every entry the navigator could offer, in display order + * @param playerId the player the menu is drawn for + * @param audience the source of permission answers + * @param size the number of slots available + * @return the visible entries with their slots, in ascending slot order + */ + @Contract(pure = true) + public static List plan(List entries, UUID playerId, FeatureAudience audience, int size) { + List visible = visibleTo(entries, playerId, audience); + int shown = Math.min(visible.size(), size); + int start = (size - shown) / 2; + List placements = new ArrayList<>(shown); + for (int index = 0; index < shown; index++) { + placements.add(new Placement(start + index, visible.get(index))); + } + return List.copyOf(placements); + } + + /** + * Filters the entries down to the ones the player is allowed to be offered. + * + * @param entries every entry the navigator could offer, in display order + * @param playerId the player the menu is drawn for + * @param audience the source of permission answers + * @return the visible entries, order preserved + */ + @Contract(pure = true) + public static List visibleTo(List entries, UUID playerId, FeatureAudience audience) { + List visible = new ArrayList<>(entries.size()); + for (NavigatorEntry entry : entries) { + if (entry.isVisibleTo(playerId, audience)) { + visible.add(entry); + } + } + return List.copyOf(visible); + } + + /** + * One entry and the slot it was given. + * + * @param slot the slot index inside the menu + * @param entry the entry shown there + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ + public record Placement(int slot, NavigatorEntry entry) { + } +} diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/package-info.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/package-info.java new file mode 100644 index 0000000..8eef0c3 --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/package-info.java @@ -0,0 +1,22 @@ +/** + * The navigator menu and the build servers it may offer (spec stage 5). + * + *

The package holds three things that belong together: + * + *

+ */ +@NotNullByDefault +package net.onelitefeather.titan.common.navigator; + +import org.jetbrains.annotations.NotNullByDefault; diff --git a/common/src/test/java/net/onelitefeather/titan/common/navigator/NavigatorLayoutTest.java b/common/src/test/java/net/onelitefeather/titan/common/navigator/NavigatorLayoutTest.java new file mode 100644 index 0000000..8967d1f --- /dev/null +++ b/common/src/test/java/net/onelitefeather/titan/common/navigator/NavigatorLayoutTest.java @@ -0,0 +1,142 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.kyori.adventure.text.Component; +import net.minestom.server.item.ItemStack; +import net.minestom.server.item.Material; +import net.minestom.testing.extension.MicrotusExtension; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +@ExtendWith(MicrotusExtension.class) +class NavigatorLayoutTest { + + private static final int ROW = 9; + + // Instance fields, not constants: building an ItemStack needs the Minestom registries, which + // the extension only loads once the test instance is being created. + private final List publicEntries = List.of(entry("ElytraRace"), entry("Survival"), entry("Slender"), entry("Creative")); + + private final List withBuildServers = concat(this.publicEntries, List.of(buildServer("Build-1"), buildServer("Build-2"))); + + private final UUID player = UUID.randomUUID(); + private final TestAudience audience = new TestAudience(); + + @DisplayName("A team member sees the build servers next to the public entries") + @Test + void testBuildServersVisibleWithPermission() { + this.audience.grant(this.player, BuildServerAccess.PERMISSION); + + List placements = NavigatorLayout.plan(this.withBuildServers, this.player, this.audience, ROW); + + Assertions.assertEquals(6, placements.size(), "All six entries should be offered"); + Assertions.assertEquals(List.of("Build-1", "Build-2"), destinations(placements).subList(4, 6), "The build servers should follow the public entries"); + } + + @DisplayName("Without the permission the build server entries are not shown at all") + @Test + void testBuildServersHiddenWithoutPermission() { + List placements = NavigatorLayout.plan(this.withBuildServers, this.player, this.audience, ROW); + + Assertions.assertEquals(4, placements.size(), "Only the public entries should be offered"); + Assertions.assertFalse(destinations(placements).contains("Build-1"), "A hidden entry must not appear"); + Assertions.assertFalse(destinations(placements).contains("Build-2"), "A hidden entry must not appear"); + } + + @DisplayName("A hidden entry leaves no reserved slot: the slots stay one uninterrupted block") + @Test + void testHiddenEntriesLeaveNoGap() { + List placements = NavigatorLayout.plan(this.withBuildServers, this.player, this.audience, ROW); + + List slots = placements.stream().map(NavigatorLayout.Placement::slot).toList(); + Assertions.assertEquals(List.of(2, 3, 4, 5), slots, "Four visible entries should sit centred and adjacent"); + for (int index = 1; index < slots.size(); index++) { + Assertions.assertEquals(slots.get(index - 1) + 1, slots.get(index), "Slots must not skip a position"); + } + } + + @DisplayName("The menu without the permission is identical to a lobby that has no build servers") + @Test + void testHiddenMenuIsIndistinguishableFromNoBuildServers() { + List withHidden = NavigatorLayout.plan(this.withBuildServers, this.player, this.audience, ROW); + List withoutAny = NavigatorLayout.plan(this.publicEntries, this.player, this.audience, ROW); + + // NFR-005: the absence must not reveal that something is hidden. Nothing in the menu - + // neither a slot nor an item - differs between the two lobbies. + Assertions.assertEquals(withoutAny, withHidden, "The two menus must be indistinguishable"); + } + + @DisplayName("Adding or removing build servers does not move the entries of an unprivileged player") + @Test + void testUnprivilegedLayoutIsIndependentOfBuildServerCount() { + List none = NavigatorLayout.plan(this.publicEntries, this.player, this.audience, ROW); + List five = NavigatorLayout.plan(concat(this.publicEntries, List.of(buildServer("Build-1"), buildServer("Build-2"), buildServer("Build-3"), buildServer("Build-4"), buildServer("Build-5"))), this.player, this.audience, ROW); + + Assertions.assertEquals(none, five, "The count of hidden entries must not be observable"); + } + + @DisplayName("More visible entries than slots are truncated instead of shifting the menu size") + @Test + void testSurplusEntriesAreDropped() { + this.audience.grant(this.player, BuildServerAccess.PERMISSION); + List many = new ArrayList<>(this.publicEntries); + for (int index = 0; index < 10; index++) { + many.add(buildServer("Build-" + index)); + } + + List placements = NavigatorLayout.plan(many, this.player, this.audience, ROW); + + Assertions.assertEquals(ROW, placements.size(), "The menu should fill but never exceed the row"); + Assertions.assertEquals(0, placements.getFirst().slot(), "A full row starts at the first slot"); + Assertions.assertEquals(ROW - 1, placements.getLast().slot(), "A full row ends at the last slot"); + } + + @DisplayName("An empty entry list produces an empty layout instead of failing") + @Test + void testEmptyEntries() { + Assertions.assertTrue(NavigatorLayout.plan(List.of(), this.player, this.audience, ROW).isEmpty()); + } + + private static List destinations(List placements) { + return placements.stream().map(placement -> placement.entry().destination()).toList(); + } + + private static List concat(List first, List second) { + List entries = new ArrayList<>(first); + entries.addAll(second); + return List.copyOf(entries); + } + + private static NavigatorEntry entry(String taskName) { + return NavigatorEntry.task(icon(taskName), taskName); + } + + private static NavigatorEntry buildServer(String serviceName) { + return NavigatorEntry.restrictedServer(icon(serviceName), serviceName, BuildServerAccess.PERMISSION); + } + + private static ItemStack icon(String name) { + return ItemStack.builder(Material.PAPER).customName(Component.text(name)).build(); + } +} diff --git a/common/src/test/java/net/onelitefeather/titan/common/navigator/TestAudience.java b/common/src/test/java/net/onelitefeather/titan/common/navigator/TestAudience.java new file mode 100644 index 0000000..a236d44 --- /dev/null +++ b/common/src/test/java/net/onelitefeather/titan/common/navigator/TestAudience.java @@ -0,0 +1,56 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.onelitefeather.titan.common.feature.FeatureAudience; + +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; + +/** + * Fixture standing in for LuckPerms. Permissions can be taken away again, which is the whole + * point: a permission held while the menu was drawn may be gone by the time the click arrives. + */ +final class TestAudience implements FeatureAudience { + + private final Set permissions = new HashSet<>(); + + TestAudience grant(UUID playerId, String permission) { + this.permissions.add(key(playerId, permission)); + return this; + } + + TestAudience revoke(UUID playerId, String permission) { + this.permissions.remove(key(playerId, permission)); + return this; + } + + @Override + public boolean hasPermission(UUID playerId, String permission) { + return this.permissions.contains(key(playerId, permission)); + } + + @Override + public boolean inGroup(UUID playerId, String group) { + return false; + } + + private static String key(UUID playerId, String value) { + return playerId + "/" + value; + } +} From 61debd241e8c88b8560f0cb5ec91e816eced08e2 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:57:57 +0200 Subject: [PATCH 2/7] feat(navigator): check the build server permission again on the way out US-5.03: the check that decides what is drawn is not a permission. A click arrives as a packet - it can name a slot the menu never had, it can arrive long after the menu was built, and the permission can have been withdrawn in between. GuardedDeliver therefore decides once more when the switch is requested, against the permission the player holds at that moment, and it sits in the Deliver chain rather than in the navigator because every path to another server goes through Deliver, including the ones that never involved a menu. Everything that is not a build server passes through untouched. BuildServerAccess answers what a build server is, and does so by name rather than by the list of servers that happen to be online: a destination is a build server because of the task it belongs to. Membership tied to the reachable list would let a request naming a stopped build server slip past the guard as an ordinary destination. CloudNet names a service -, so the task is recoverable from the service name with JDK types only. The task defaults to Build and can be overridden per deployment via titan.buildserver.task. BuildServerDirectory plus TitanBuildServerDirectory are the seam for the reachable list (US-5.04). CloudNet lives behind a classloader boundary, so the implementation is installed by the bridge extension and only a List crosses over - the same rule ServerConnector and TitanPermissionBridge already follow. Until one is installed the directory reports nothing, which hides the build servers rather than guessing. --- .../common/navigator/BuildServerAccess.java | 95 ++++++++++++++ .../navigator/BuildServerDirectory.java | 54 ++++++++ .../common/navigator/GuardedDeliver.java | 93 +++++++++++++ .../navigator/TitanBuildServerDirectory.java | 61 +++++++++ .../navigator/BuildServerAccessTest.java | 101 ++++++++++++++ .../common/navigator/GuardedDeliverTest.java | 123 ++++++++++++++++++ 6 files changed, 527 insertions(+) create mode 100644 common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java create mode 100644 common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java create mode 100644 common/src/main/java/net/onelitefeather/titan/common/navigator/GuardedDeliver.java create mode 100644 common/src/main/java/net/onelitefeather/titan/common/navigator/TitanBuildServerDirectory.java create mode 100644 common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java create mode 100644 common/src/test/java/net/onelitefeather/titan/common/navigator/GuardedDeliverTest.java diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java new file mode 100644 index 0000000..3955de9 --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java @@ -0,0 +1,95 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.onelitefeather.deliver.DeliverComponent; +import org.jetbrains.annotations.Contract; + +import java.util.Locale; + +/** + * Says which CloudNet destinations count as build servers and which permission they require. + * + *

The membership test deliberately works on names rather than on the list of servers that + * happen to be online. A destination is a build server because of the task it belongs to, not + * because it is currently reachable — otherwise a request naming a stopped build server would + * slip past the guard as an ordinary destination, and the check in {@link GuardedDeliver} would + * only hold for as long as the menu was accurate. CloudNet names a service {@code -}, + * so the task name is recoverable from the service name alone, with JDK types only. + * + * @param taskName the CloudNet task the build servers belong to + * @param permission the permission required to see and to reach them + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ +public record BuildServerAccess(String taskName, String permission) { + + /** Permission a team member needs for the build servers (US-5.01). */ + public static final String PERMISSION = "titan.navigator.buildserver"; + + /** CloudNet task the build servers run under when nothing else is configured. */ + public static final String DEFAULT_TASK = "Build"; + + /** + * System property overriding {@value #DEFAULT_TASK} for a network that names its task + * differently. + */ + public static final String TASK_PROPERTY = "titan.buildserver.task"; + + /** + * Returns the access rule for this deployment: the task from {@value #TASK_PROPERTY} or + * {@value #DEFAULT_TASK}, guarded by {@value #PERMISSION}. + * + * @return the configured access rule + */ + @Contract(value = "-> new", pure = true) + public static BuildServerAccess defaults() { + String configured = System.getProperty(TASK_PROPERTY, DEFAULT_TASK).trim(); + return new BuildServerAccess(configured.isEmpty() ? DEFAULT_TASK : configured, PERMISSION); + } + + /** + * Checks whether a service name belongs to the build task. + * + * @param serviceName the CloudNet service name, for example {@code Build-1} + * @return whether the service is a build server + */ + @Contract(pure = true) + public boolean covers(String serviceName) { + String name = serviceName.toLowerCase(Locale.ROOT); + String task = this.taskName.toLowerCase(Locale.ROOT); + return name.equals(task) || name.startsWith(task + "-"); + } + + /** + * Checks whether a requested switch targets a build server, whether it names the task or one + * of its services. + * + * @param component the requested switch + * @return whether the request needs {@link #permission()} + */ + @Contract(pure = true) + public boolean covers(DeliverComponent component) { + return switch (component) { + case DeliverComponent.TaskComponent taskComponent -> covers(taskComponent.taskName()); + case DeliverComponent.ServerDeliverComponent serverComponent -> + covers(serverComponent.gameServer()); + default -> false; + }; + } +} diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java new file mode 100644 index 0000000..4a0eccd --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java @@ -0,0 +1,54 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import java.util.List; + +/** + * Lists the build servers that are reachable right now (US-5.04). + * + *

CloudNet is the source of that list, and CloudNet classes are not on the application + * classpath: the bridge runs in its own extension classloader. The production implementation + * therefore lives in the {@code :bridge} extension and is installed through + * {@link TitanBuildServerDirectory}; only a {@code List} crosses the boundary. This is + * the same rule {@code net.onelitefeather.titan.common.deliver.ServerConnector} and + * {@code net.onelitefeather.titan.common.permission.TitanPermissionBridge} follow. + * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ +@FunctionalInterface +public interface BuildServerDirectory { + + /** + * Returns a directory that never reports a build server. Used standalone — local runs, tests + * and AOT training — where there is no CloudNet to ask. + * + * @return a directory reporting nothing + */ + static BuildServerDirectory empty() { + return List::of; + } + + /** + * Returns the names of the build-task services that are running and connected at this moment. + * + * @return the reachable service names; empty when CloudNet is absent or cannot be reached + */ + List reachableServices(); +} diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/GuardedDeliver.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/GuardedDeliver.java new file mode 100644 index 0000000..fa8bf66 --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/GuardedDeliver.java @@ -0,0 +1,93 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.minestom.server.entity.Player; +import net.onelitefeather.deliver.DeliverComponent; +import net.onelitefeather.titan.api.deliver.Deliver; +import net.onelitefeather.titan.common.feature.FeatureAudience; +import org.jetbrains.annotations.Contract; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.UUID; + +/** + * Checks the build-server permission again at the moment a switch is requested (US-5.03). + * + *

The check in the navigator decides what is drawn, and a drawing is not a + * permission. A click arrives as a packet: it can name a slot the menu never had, it can arrive + * long after the menu was built, and the permission can have been taken away in between. So the + * decision is made once more here, on the way out, against the permission the player holds + * now — and here rather than in the navigator because every path to another server goes + * through {@link Deliver}, including the ones that never involved a menu. + * + *

Everything that is not a build server passes through untouched; this decorator guards one + * destination, it is not a general permission layer. + * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ +public final class GuardedDeliver implements Deliver { + + private static final Logger LOGGER = LoggerFactory.getLogger(GuardedDeliver.class); + + private final Deliver delegate; + private final FeatureAudience audience; + private final BuildServerAccess access; + + private GuardedDeliver(Deliver delegate, FeatureAudience audience, BuildServerAccess access) { + this.delegate = delegate; + this.audience = audience; + this.access = access; + } + + /** + * Wraps a delivery implementation with the build-server check. + * + * @param delegate the delivery that performs the switch once it is allowed + * @param audience the source of permission answers, asked at request time + * @param access which destinations are build servers and what they require + * @return the guarded delivery + */ + @Contract(value = "_, _, _ -> new", pure = true) + public static GuardedDeliver wrap(Deliver delegate, FeatureAudience audience, BuildServerAccess access) { + return new GuardedDeliver(delegate, audience, access); + } + + @Override + public void sendPlayer(Player player, DeliverComponent component) { + if (!allows(player.getUuid(), component)) { + LOGGER.warn("Refused build server switch for {}: player does not hold {}", player.getUuid(), this.access.permission()); + return; + } + this.delegate.sendPlayer(player, component); + } + + /** + * Decides whether the requested switch may go through. + * + * @param playerId the player requesting the switch + * @param component the requested destination + * @return whether the destination is unguarded or the player currently holds the permission + */ + @Contract(pure = true) + public boolean allows(UUID playerId, DeliverComponent component) { + return !this.access.covers(component) || this.audience.hasPermission(playerId, this.access.permission()); + } +} diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/TitanBuildServerDirectory.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/TitanBuildServerDirectory.java new file mode 100644 index 0000000..cb3e495 --- /dev/null +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/TitanBuildServerDirectory.java @@ -0,0 +1,61 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import org.jetbrains.annotations.Nullable; + +import java.util.List; + +/** + * Cross-classloader holder for the reachable build servers. + * + *

Lives on the shared application classloader so the CloudNet bridge extension can install a + * {@link BuildServerDirectory} that the application can read without ever naming a CloudNet + * class. Until the extension installs one — standalone runs, tests, the window before the bridge + * has started — the holder reports no build servers, which hides them rather than guessing. + * + * @author TheMeinerLP + * @version 1.0.0 + * @since 1.15.0 + */ +public final class TitanBuildServerDirectory { + + private static volatile @Nullable BuildServerDirectory directory; + + private TitanBuildServerDirectory() { + } + + /** + * Installs the directory. Called by the bridge extension once the CloudNet driver is up. + * + * @param buildServerDirectory the directory backed by the CloudNet service list + */ + public static void setDirectory(BuildServerDirectory buildServerDirectory) { + directory = buildServerDirectory; + } + + /** + * Returns the currently reachable build servers, or an empty list when no directory has been + * installed. + * + * @return the reachable service names + */ + public static List reachableServices() { + BuildServerDirectory current = directory; + return current == null ? List.of() : current.reachableServices(); + } +} diff --git a/common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java b/common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java new file mode 100644 index 0000000..deab0ec --- /dev/null +++ b/common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java @@ -0,0 +1,101 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.onelitefeather.deliver.DeliverComponent; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.UUID; + +class BuildServerAccessTest { + + private final BuildServerAccess access = new BuildServerAccess("Build", BuildServerAccess.PERMISSION); + private final UUID player = UUID.randomUUID(); + + @DisplayName("A service of the build task is recognised by its name") + @Test + void testServiceNamesOfTheBuildTask() { + Assertions.assertTrue(this.access.covers("Build-1")); + Assertions.assertTrue(this.access.covers("Build-42")); + Assertions.assertTrue(this.access.covers("Build")); + Assertions.assertTrue(this.access.covers("build-1"), "CloudNet task names are not case sensitive"); + } + + @DisplayName("A service that only starts like the build task is not one") + @Test + void testForeignServiceNames() { + Assertions.assertFalse(this.access.covers("Lobby-1")); + Assertions.assertFalse(this.access.covers("BuildBattle-1"), "A different task with the same prefix is not the build task"); + Assertions.assertFalse(this.access.covers("")); + } + + @DisplayName("A stopped build server is still a build server") + @Test + void testOfflineBuildServerIsStillGuarded() { + // The guard must not depend on the service being in the reachable list: otherwise a + // request naming a stopped build server would look like an ordinary destination. + Assertions.assertTrue(this.access.covers(server("Build-9")), "Membership follows the task name, not the service list"); + } + + @DisplayName("Both a task request and a service request are guarded") + @Test + void testComponentsAreCovered() { + Assertions.assertTrue(this.access.covers(task("Build"))); + Assertions.assertTrue(this.access.covers(server("Build-1"))); + Assertions.assertFalse(this.access.covers(task("Survival"))); + Assertions.assertFalse(this.access.covers(server("Lobby-1"))); + } + + @DisplayName("The task name can be overridden per deployment") + @Test + void testTaskNameOverride() { + String previous = System.getProperty(BuildServerAccess.TASK_PROPERTY); + try { + System.setProperty(BuildServerAccess.TASK_PROPERTY, "Bauserver"); + BuildServerAccess overridden = BuildServerAccess.defaults(); + + Assertions.assertEquals("Bauserver", overridden.taskName()); + Assertions.assertTrue(overridden.covers("Bauserver-1")); + Assertions.assertFalse(overridden.covers("Build-1")); + } finally { + if (previous == null) { + System.clearProperty(BuildServerAccess.TASK_PROPERTY); + } else { + System.setProperty(BuildServerAccess.TASK_PROPERTY, previous); + } + } + } + + @DisplayName("Without an override the defaults name the Build task and the navigator permission") + @Test + void testDefaults() { + BuildServerAccess defaults = BuildServerAccess.defaults(); + + Assertions.assertEquals(BuildServerAccess.DEFAULT_TASK, defaults.taskName()); + Assertions.assertEquals("titan.navigator.buildserver", defaults.permission()); + } + + private DeliverComponent task(String taskName) { + return DeliverComponent.taskBuilder().playerId(this.player).taskName(taskName).build(); + } + + private DeliverComponent server(String serviceName) { + return DeliverComponent.serverBuilder().playerId(this.player).serverName(serviceName).build(); + } +} diff --git a/common/src/test/java/net/onelitefeather/titan/common/navigator/GuardedDeliverTest.java b/common/src/test/java/net/onelitefeather/titan/common/navigator/GuardedDeliverTest.java new file mode 100644 index 0000000..d29fa75 --- /dev/null +++ b/common/src/test/java/net/onelitefeather/titan/common/navigator/GuardedDeliverTest.java @@ -0,0 +1,123 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.common.navigator; + +import net.kyori.adventure.text.Component; +import net.minestom.server.entity.Player; +import net.minestom.server.instance.Instance; +import net.minestom.server.item.ItemStack; +import net.minestom.server.item.Material; +import net.minestom.testing.Env; +import net.minestom.testing.extension.MicrotusExtension; +import net.onelitefeather.deliver.DeliverComponent; +import net.onelitefeather.titan.api.deliver.Deliver; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +import java.util.ArrayList; +import java.util.List; + +@ExtendWith(MicrotusExtension.class) +class GuardedDeliverTest { + + private static final int ROW = 9; + + private final BuildServerAccess access = new BuildServerAccess("Build", BuildServerAccess.PERMISSION); + private final TestAudience audience = new TestAudience(); + private final RecordingDeliver delegate = new RecordingDeliver(); + + @DisplayName("A team member holding the permission reaches the build server") + @Test + void testSwitchAllowedWithPermission(Env env) { + Player player = player(env); + this.audience.grant(player.getUuid(), BuildServerAccess.PERMISSION); + Deliver deliver = GuardedDeliver.wrap(this.delegate, this.audience, this.access); + + deliver.sendPlayer(player, server(player, "Build-1")); + + Assertions.assertEquals(1, this.delegate.delivered.size(), "The switch should have been performed"); + } + + @DisplayName("A permission withdrawn between opening the menu and clicking refuses the switch") + @Test + void testSwitchRefusedAfterPermissionRevoked(Env env) { + Player player = player(env); + this.audience.grant(player.getUuid(), BuildServerAccess.PERMISSION); + Deliver deliver = GuardedDeliver.wrap(this.delegate, this.audience, this.access); + + // The menu is drawn while the player still holds the permission, so the entry is there. + List entries = List.of(NavigatorEntry.restrictedServer(icon(), "Build-1", BuildServerAccess.PERMISSION)); + List menu = NavigatorLayout.plan(entries, player.getUuid(), this.audience, ROW); + Assertions.assertEquals(1, menu.size(), "The entry should be offered while the permission is held"); + + // The permission is taken away, then the click arrives. The drawn menu proves nothing. + this.audience.revoke(player.getUuid(), BuildServerAccess.PERMISSION); + deliver.sendPlayer(player, menu.getFirst().entry().toComponent(player.getUuid())); + + Assertions.assertTrue(this.delegate.delivered.isEmpty(), "A revoked permission must refuse the switch (US-5.03)"); + } + + @DisplayName("A tampered click naming a build server the menu never offered is refused") + @Test + void testTamperedRequestRefused(Env env) { + Player player = player(env); + Deliver deliver = GuardedDeliver.wrap(this.delegate, this.audience, this.access); + + deliver.sendPlayer(player, server(player, "Build-1")); + deliver.sendPlayer(player, server(player, "Build-99")); + deliver.sendPlayer(player, DeliverComponent.taskBuilder().player(player).taskName("Build").build()); + + Assertions.assertTrue(this.delegate.delivered.isEmpty(), "No build server may be reached without the permission"); + } + + @DisplayName("Public destinations pass through untouched") + @Test + void testUnguardedDestinationPassesThrough(Env env) { + Player player = player(env); + Deliver deliver = GuardedDeliver.wrap(this.delegate, this.audience, this.access); + + deliver.sendPlayer(player, DeliverComponent.taskBuilder().player(player).taskName("Survival").build()); + + Assertions.assertEquals(1, this.delegate.delivered.size(), "The guard covers the build servers only"); + } + + private Player player(Env env) { + Instance instance = env.createFlatInstance(); + return env.createPlayer(instance); + } + + private DeliverComponent server(Player player, String serviceName) { + return DeliverComponent.serverBuilder().player(player).serverName(serviceName).build(); + } + + private static ItemStack icon() { + return ItemStack.builder(Material.SCAFFOLDING).customName(Component.text("Build-1")).build(); + } + + /** Stands in for the real delivery and only records what it was asked to do. */ + private static final class RecordingDeliver implements Deliver { + + private final List delivered = new ArrayList<>(); + + @Override + public void sendPlayer(Player player, DeliverComponent component) { + this.delivered.add(component); + } + } +} From ff4a71ae8d38597c06147c6e242a703dafe14ceb Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:58:09 +0200 Subject: [PATCH 3/7] feat(app): offer the build servers to team members in the navigator The navigator no longer writes items into hardcoded slots. It builds the list of entries for the player it is drawing for - the four game modes always, the reachable build servers only for a holder of titan.navigator.buildserver - and asks NavigatorLayout where they go (US-5.01, US-5.02). Both the permission and the list of reachable build servers are read while the menu is being drawn rather than when the helper is created, so a menu opened again reflects a stopped server or a withdrawn permission (US-5.04). A player without the permission is not a reason to ask CloudNet anything: their menu must not depend on the answer, so the lookup is skipped entirely for them. Titan wraps whatever DeliverProvider returned in GuardedDeliver, which makes the second permission check unavoidable for every switch the lobby performs, and hands the navigator the same audience and the TitanBuildServerDirectory holder. The build server icon names the service as plain text rather than through MiniMessage, so a service whose name contains tag-like characters cannot inject formatting into the menu. The tests open the real menu and read what the player sees. The unprivileged menu is asserted to be byte-identical to the one of a lobby with no build servers, slot by slot - absence, not a reserved gap. --- .../net/onelitefeather/titan/app/Titan.java | 19 +- .../titan/app/helper/NavigationHelper.java | 164 ++++++++---- .../titan/app/helper/package-info.java | 12 + .../app/helper/NavigationHelperTest.java | 234 +++++++++++++++--- .../titan/app/testutils/TestAudience.java | 56 +++++ .../titan/common/utils/Items.java | 13 + 6 files changed, 412 insertions(+), 86 deletions(-) create mode 100644 app/src/main/java/net/onelitefeather/titan/app/helper/package-info.java create mode 100644 app/src/test/java/net/onelitefeather/titan/app/testutils/TestAudience.java diff --git a/app/src/main/java/net/onelitefeather/titan/app/Titan.java b/app/src/main/java/net/onelitefeather/titan/app/Titan.java index 95959cd..ba896a4 100644 --- a/app/src/main/java/net/onelitefeather/titan/app/Titan.java +++ b/app/src/main/java/net/onelitefeather/titan/app/Titan.java @@ -36,9 +36,13 @@ import net.onelitefeather.titan.app.listener.*; import net.onelitefeather.titan.app.player.TitanPlayer; import net.onelitefeather.titan.common.config.AppConfigProvider; +import net.onelitefeather.titan.common.feature.FeatureAudience; import net.onelitefeather.titan.common.feature.FeatureGate; import net.onelitefeather.titan.common.feature.SeasonWindowActivationStrategy; import net.onelitefeather.titan.common.deliver.DeliverProvider; +import net.onelitefeather.titan.common.navigator.BuildServerAccess; +import net.onelitefeather.titan.common.navigator.GuardedDeliver; +import net.onelitefeather.titan.common.navigator.TitanBuildServerDirectory; import net.onelitefeather.titan.common.event.EntityDismountEvent; import net.onelitefeather.titan.common.helper.BlockHandlerHelper; import net.onelitefeather.titan.common.map.MapProvider; @@ -52,7 +56,7 @@ public final class Titan { private final Path path; private final EventNode eventNode = EventNode.all("titan"); - private final Deliver deliver = DeliverProvider.create(); + private final Deliver deliver; private final MapProvider mapProvider; private final AppConfigProvider appConfigProvider; private final NavigationHelper navigationHelper; @@ -77,8 +81,17 @@ public Titan(Clock clock, ZoneId zone) { MinecraftServer.getInstanceManager().registerInstance(instance); this.mapProvider = MapProvider.create(this.path, instance); this.appConfigProvider = AppConfigProvider.create(this.path); - this.featureGate = FeatureGate.create(LuckPermsFeatureAudience.create(), clock, zone); - this.navigationHelper = NavigationHelper.instance(this.deliver, this.featureGate); + FeatureAudience audience = LuckPermsFeatureAudience.create(); + BuildServerAccess buildServerAccess = BuildServerAccess.defaults(); + // Every path to another server runs through this Deliver, so the build server permission + // is checked here once more when a switch is requested - a click is a packet, not a proof + // that the menu ever offered the destination (US-5.03). + this.deliver = GuardedDeliver.wrap(DeliverProvider.create(), audience, buildServerAccess); + // The gate is built first: the navigator asks it whether a destination is released at all + // before the build server permission narrows the list any further. + this.featureGate = FeatureGate.create(audience, clock, zone); + this.navigationHelper = NavigationHelper.instance( + this.deliver, audience, this.featureGate, TitanBuildServerDirectory::reachableServices, buildServerAccess); } public void initialize() { diff --git a/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java b/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java index 29da7d0..b1e72da 100644 --- a/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java +++ b/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java @@ -23,40 +23,87 @@ import net.minestom.server.entity.EquipmentSlot; import net.minestom.server.entity.Player; import net.minestom.server.inventory.InventoryType; -import net.minestom.server.inventory.click.Click; -import net.minestom.server.item.ItemStack; -import net.onelitefeather.deliver.DeliverComponent; import net.onelitefeather.titan.api.deliver.Deliver; +import net.onelitefeather.titan.common.feature.FeatureAudience; import net.onelitefeather.titan.common.feature.FeatureGate; import net.onelitefeather.titan.common.feature.TitanFeatures; +import net.onelitefeather.titan.common.navigator.BuildServerAccess; +import net.onelitefeather.titan.common.navigator.BuildServerDirectory; +import net.onelitefeather.titan.common.navigator.NavigatorEntry; +import net.onelitefeather.titan.common.navigator.NavigatorLayout; import net.onelitefeather.titan.common.utils.Items; import net.theevilreaper.aves.inventory.InventoryLayout; import net.theevilreaper.aves.inventory.PersonalInventoryBuilder; import net.theevilreaper.aves.inventory.click.ClickHolder; import net.theevilreaper.aves.inventory.util.LayoutCalculator; +import org.jetbrains.annotations.Nullable; +import org.togglz.core.user.SimpleFeatureUser; +import org.togglz.core.user.thread.ThreadLocalUserProvider; import java.time.Duration; +import java.util.ArrayList; +import java.util.List; import java.util.UUID; -import java.util.function.Consumer; +/** + * Builds and opens the navigator menu. + * + *

The menu is one row and always one row, for everyone. Which entries a player is offered is + * decided per player: the four game modes are public, the build servers need + * {@value BuildServerAccess#PERMISSION} (US-5.01). The slots are not fixed — they are computed + * from the entries the player may see, so an entry that was filtered out leaves no reserved slot + * behind and a player without the permission sees exactly the menu of a lobby that has no build + * servers at all (US-5.02, NFR-005). See {@link NavigatorLayout}. + * + *

Both the permission and the list of reachable build servers are read while the menu is being + * drawn, not when the helper is created, so a menu opened a second time reflects a stopped server + * or a withdrawn permission (US-5.04). + * + * @author TheMeinerLP + * @version 2.0.0 + * @since 1.15.0 + */ public class NavigationHelper { + private static final InventoryType NAVIGATOR_TYPE = InventoryType.CHEST_1_ROW; + + /** + * The destinations every lobby offers, each paired with the feature that releases it. + * A destination is drawn only when {@link FeatureGate} admits its feature for the player + * (US-3.01 to US-3.04, US-3.06) - the build servers add their own permission check on top. + */ + private static final List PUBLIC_ENTRIES = List.of( + new GatedEntry(TitanFeatures.NAVIGATOR_ELYTRA, NavigatorEntry.task(Items.NAVIGATOR_ELYTRA_ITEM_STACK, "ElytraRace")), new GatedEntry(TitanFeatures.NAVIGATOR_SURVIVAL, NavigatorEntry.task(Items.NAVIGATOR_SURVIVAL_ITEM_STACK, "Survival")), new GatedEntry(TitanFeatures.NAVIGATOR_SLENDER, NavigatorEntry.task(Items.NAVIGATOR_SLENDER_ITEM_STACK, "cygnus")), new GatedEntry(TitanFeatures.NAVIGATOR_CREATIVE, NavigatorEntry.task(Items.NAVIGATOR_CREATIVE_ITEM_STACK, "MemberBuild"))); + + /** + * A public destination together with the feature flag that releases it. + * + * @param feature the feature deciding whether the destination is offered at all + * @param entry the destination as the layout sees it + */ + private record GatedEntry(TitanFeatures feature, NavigatorEntry entry) { + } + private final String inventoryName = "Navigator"; private final Deliver deliver; + private final FeatureAudience audience; private final FeatureGate featureGate; + private final BuildServerDirectory buildServers; + private final BuildServerAccess access; private final LoadingCache inventoryBuilderLoadingCache = Caffeine.newBuilder().maximumSize(10000).expireAfterWrite(Duration.ofMinutes(5)).refreshAfterWrite(Duration.ofMinutes(1)).build(key -> createPersonalInventoryBuilder( MinecraftServer.getConnectionManager().getOnlinePlayerByUuid(key))); - private NavigationHelper(Deliver deliver, FeatureGate featureGate) { + private NavigationHelper(Deliver deliver, FeatureAudience audience, FeatureGate featureGate, BuildServerDirectory buildServers, BuildServerAccess access) { this.deliver = deliver; + this.audience = audience; this.featureGate = featureGate; + this.buildServers = buildServers; + this.access = access; } public void openNavigator(Player player) { PersonalInventoryBuilder personalInventoryBuilder = inventoryBuilderLoadingCache.get(player.getUuid()); - // The builder is cached per player, the layout is not: invalidating it runs the data - // layout function again, so a flag changed since the last open takes effect on this open. personalInventoryBuilder.invalidateDataLayout(); personalInventoryBuilder.open(); } @@ -67,62 +114,91 @@ public void setItems(Player player) { player.getInventory().setEquipment(EquipmentSlot.CHESTPLATE, (byte) EquipmentSlot.CHESTPLATE.armorSlot(), Items.PLAYER_ELYTRA); } - private PersonalInventoryBuilder createPersonalInventoryBuilder(Player player) { + private @Nullable PersonalInventoryBuilder createPersonalInventoryBuilder(@Nullable Player player) { if (player == null) return null; PersonalInventoryBuilder inventoryBuilder = new PersonalInventoryBuilder( - MiniMessage.miniMessage().deserialize(inventoryName), InventoryType.CHEST_1_ROW, player); - inventoryBuilder.setLayout(InventoryLayout.fromType(InventoryType.CHEST_1_ROW)); + MiniMessage.miniMessage().deserialize(inventoryName), NAVIGATOR_TYPE, player); + inventoryBuilder.setLayout(InventoryLayout.fromType(NAVIGATOR_TYPE)); inventoryBuilder.setDataLayoutFunction(layout -> { - InventoryLayout finalLayout = layout != null ? layout : InventoryLayout.fromType(InventoryType.CHEST_1_ROW); - - finalLayout.setItems(LayoutCalculator.fillRow(InventoryType.CHEST_1_ROW), Items.NAVIGATOR_BLANK_ITEM_STACK); - // Every destination is gated (US-3.01 to US-3.04, US-3.06). A denied entry is not - // written, so its slot keeps the filler pane the whole row was just filled with. - if (isVisible(TitanFeatures.NAVIGATOR_ELYTRA, player)) { - finalLayout.setItem(0, Items.NAVIGATOR_ELYTRA_ITEM_STACK, this::clickElytra); - } - if (isVisible(TitanFeatures.NAVIGATOR_SURVIVAL, player)) { - finalLayout.setItem(4, Items.NAVIGATOR_SURVIVAL_ITEM_STACK, this::clickSurvival); - } - if (isVisible(TitanFeatures.NAVIGATOR_SLENDER, player)) { - finalLayout.setItem(5, Items.NAVIGATOR_SLENDER_ITEM_STACK, this::clickSlender); - } - if (isVisible(TitanFeatures.NAVIGATOR_CREATIVE, player)) { - finalLayout.setItem(8, Items.NAVIGATOR_CREATIVE_ITEM_STACK, this::clickCreative); + InventoryLayout finalLayout = layout != null ? layout : InventoryLayout.fromType(NAVIGATOR_TYPE); + + // Blank the whole row first: every slot that no visible entry claims is filler, and + // filler is what a slot holding a hidden entry would have to look like anyway. + finalLayout.setItems(LayoutCalculator.fillRow(NAVIGATOR_TYPE), Items.NAVIGATOR_BLANK_ITEM_STACK); + ThreadLocalUserProvider.bind(toUser(player)); + for (NavigatorLayout.Placement placement : layoutFor(player.getUuid())) { + NavigatorEntry entry = placement.entry(); + finalLayout.setItem(placement.slot(), entry.icon(), (clicker, slot, click, itemStack, result) -> { + this.deliver.sendPlayer(clicker, entry.toComponent(clicker.getUuid())); + result.accept(ClickHolder.cancelClick()); + }); } + ThreadLocalUserProvider.release(); return finalLayout; }); inventoryBuilder.register(); return inventoryBuilder; } - private boolean isVisible(TitanFeatures feature, Player player) { - return this.featureGate.isVisibleTo(feature, player.getUuid()); - } - - private void clickElytra(Player player, int slot, Click click, ItemStack itemStack, Consumer result) { - deliver.sendPlayer(player, DeliverComponent.taskBuilder().taskName("ElytraRace").player(player).build()); - result.accept(ClickHolder.cancelClick()); + /** + * Plans the menu for one player: which entries they are offered and where those sit. + * + * @param playerId the player the menu is drawn for + * @return the visible entries with their slots, in ascending slot order + */ + List layoutFor(UUID playerId) { + return NavigatorLayout.plan(entriesFor(playerId), playerId, this.audience, NAVIGATOR_TYPE.getSize()); } - private void clickSurvival(Player player, int slot, Click click, ItemStack itemStack, Consumer result) { - deliver.sendPlayer(player, DeliverComponent.taskBuilder().player(player).taskName("Survival").build()); - result.accept(ClickHolder.cancelClick()); + /** + * Collects every entry the navigator could offer this player. The build servers are only + * looked up for a player who holds the permission — a player who does not is not a reason to + * ask CloudNet anything, and their menu must not depend on the answer. + * + * @param playerId the player the menu is drawn for + * @return the public entries, followed by the reachable build servers in a stable order + */ + private List entriesFor(UUID playerId) { + List entries = new ArrayList<>(PUBLIC_ENTRIES.size()); + for (GatedEntry gated : PUBLIC_ENTRIES) { + if (this.featureGate.isVisibleTo(gated.feature(), playerId)) { + entries.add(gated.entry()); + } + } + if (!this.audience.hasPermission(playerId, this.access.permission())) { + return List.copyOf(entries); + } + this.buildServers.reachableServices().stream().filter(this.access::covers).sorted().map(service -> NavigatorEntry.restrictedServer(Items.navigatorBuildServer(service), service, this.access.permission())).forEach(entries::add); + return List.copyOf(entries); } - private void clickSlender(Player player, int slot, Click click, ItemStack itemStack, Consumer result) { - deliver.sendPlayer(player, DeliverComponent.taskBuilder().player(player).taskName("cygnus").build()); - result.accept(ClickHolder.cancelClick()); + private SimpleFeatureUser toUser(Player player) { + return new SimpleFeatureUser(player.getUsername()); } - private void clickCreative(Player player, int slot, Click click, ItemStack itemStack, Consumer result) { - deliver.sendPlayer(player, DeliverComponent.taskBuilder().player(player).taskName("MemberBuild").build()); - result.accept(ClickHolder.cancelClick()); + /** + * Creates a navigator that offers the public entries only. Used where no permission backend is + * available, which is the safe reading of "unknown player". + * + * @param deliver the delivery used to move a player on click + * @return a navigator without build servers + */ + public static NavigationHelper instance(Deliver deliver, FeatureGate featureGate) { + return instance(deliver, FeatureAudience.denyAll(), featureGate, BuildServerDirectory.empty(), BuildServerAccess.defaults()); } - public static NavigationHelper instance(Deliver deliver, FeatureGate featureGate) { - return new NavigationHelper(deliver, featureGate); + /** + * Creates a navigator that can also offer the build servers. + * + * @param deliver the delivery used to move a player on click + * @param audience the source of permission answers, asked every time the menu is drawn + * @param buildServers the currently reachable build servers + * @param access which destinations are build servers and what they require + * @return the navigator + */ + public static NavigationHelper instance(Deliver deliver, FeatureAudience audience, FeatureGate featureGate, BuildServerDirectory buildServers, BuildServerAccess access) { + return new NavigationHelper(deliver, audience, featureGate, buildServers, access); } } diff --git a/app/src/main/java/net/onelitefeather/titan/app/helper/package-info.java b/app/src/main/java/net/onelitefeather/titan/app/helper/package-info.java new file mode 100644 index 0000000..0f9bdab --- /dev/null +++ b/app/src/main/java/net/onelitefeather/titan/app/helper/package-info.java @@ -0,0 +1,12 @@ +/** + * Application-side helpers that turn Titan's domain types into what a player actually sees — the + * navigator menu above all. + * + *

The rules the navigator follows live in + * {@link net.onelitefeather.titan.common.navigator}; this package only wires them to Aves and to + * the running server. + */ +@NotNullByDefault +package net.onelitefeather.titan.app.helper; + +import org.jetbrains.annotations.NotNullByDefault; diff --git a/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java b/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java index be8c015..eca139b 100644 --- a/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java +++ b/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java @@ -20,48 +20,40 @@ import net.minestom.server.entity.Player; import net.minestom.server.instance.Instance; import net.minestom.server.inventory.PlayerInventory; +import net.minestom.server.item.ItemStack; import net.minestom.server.item.Material; import net.minestom.testing.Env; import net.minestom.testing.extension.MicrotusExtension; import net.onelitefeather.titan.app.testutils.DummyDeliver; +import net.onelitefeather.titan.app.testutils.TestAudience; import net.onelitefeather.titan.app.testutils.TestFeatureGate; +import net.onelitefeather.titan.common.feature.FeatureAudience; +import net.onelitefeather.titan.common.feature.FeatureGate; import net.onelitefeather.titan.common.feature.ReleaseStage; import net.onelitefeather.titan.common.feature.TitanFeatures; +import net.onelitefeather.titan.common.navigator.BuildServerAccess; +import net.onelitefeather.titan.common.navigator.BuildServerDirectory; import net.onelitefeather.titan.common.utils.Items; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.*; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; @ExtendWith(MicrotusExtension.class) class NavigationHelperTest { - private static final int SLOT_ELYTRA = 0; - private static final int SLOT_SURVIVAL = 4; - private static final int SLOT_SLENDER = 5; - private static final int SLOT_CREATIVE = 8; - - /** A fixture in which every navigator destination is generally released. */ - private static TestFeatureGate allReleased() { - return TestFeatureGate.create().release(TitanFeatures.NAVIGATOR_ELYTRA, ReleaseStage.GA).release(TitanFeatures.NAVIGATOR_SURVIVAL, ReleaseStage.GA).release(TitanFeatures.NAVIGATOR_SLENDER, ReleaseStage.GA).release(TitanFeatures.NAVIGATOR_CREATIVE, ReleaseStage.GA); - } - - /** - * Opens the navigator and reports the material a slot ended up showing. - * - *

Aves applies the data layout on the next tick ({@code InventoryBuilder.retrieveDataLayout} - * schedules it), so the inventory is still empty right after {@code open()} - the tick is what - * makes this assert against what a player actually sees. - */ - private static Material openedSlot(Env env, NavigationHelper helper, Player player, int slot) { - helper.openNavigator(player); - Assertions.assertNotNull(player.getOpenInventory(), "the navigator should be open"); - env.tick(); - return player.getOpenInventory().getItemStack(slot).material(); - } + /** The material of the filler pane every slot that no visible entry claims falls back to. */ + private static final Material FILLER = Material.GRAY_STAINED_GLASS_PANE; @DisplayName("Test if the NavigationHelper is set with the correct items") @Test @@ -94,16 +86,78 @@ void testNavigationHelperOpenNavigationGui(Env env) { Assertions.assertNotNull(realPlayer.getOpenInventory()); } + @DisplayName("A team member with the permission sees the reachable build servers") + @Test + void testBuildServersShownToTeamMember(Env env) { + Instance flatInstance = env.createFlatInstance(); + Player player = env.createPlayer(flatInstance); + TestAudience audience = new TestAudience().grant(player.getUuid(), BuildServerAccess.PERMISSION); + + ItemStack[] contents = openWith(env, player, audience, "Build-1", "Build-2"); + + Assertions.assertTrue(contains(contents, Items.navigatorBuildServer("Build-1")), "The first build server should be offered"); + Assertions.assertTrue(contains(contents, Items.navigatorBuildServer("Build-2")), "The second build server should be offered"); + } + + @DisplayName("A player without the permission sees no build server and no gap where one would be") + @Test + void testBuildServersHiddenWithoutPermission(Env env) { + Instance flatInstance = env.createFlatInstance(); + Player player = env.createPlayer(flatInstance); + FeatureAudience audience = FeatureAudience.denyAll(); + + ItemStack[] withBuildServers = openWith(env, player, audience, "Build-1", "Build-2"); + ItemStack[] withoutBuildServers = openWith(env, player, audience); + + Assertions.assertFalse(contains(withBuildServers, Items.navigatorBuildServer("Build-1")), "A hidden build server must not be rendered"); + // NFR-005: not merely absent - indistinguishable. The menu is identical to the one of a + // lobby that has no build servers at all, so no empty slot hints at what was removed. + Assertions.assertArrayEquals(withoutBuildServers, withBuildServers, "The hidden entries must leave the menu unchanged"); + } + + @DisplayName("The unprivileged menu is one uninterrupted block of public entries between filler") + @Test + void testUnprivilegedMenuHasNoEmptySlot(Env env) { + Instance flatInstance = env.createFlatInstance(); + Player player = env.createPlayer(flatInstance); + + ItemStack[] contents = openWith(env, player, FeatureAudience.denyAll(), "Build-1", "Build-2"); + + Assertions.assertTrue(contents[2].isSimilar(Items.NAVIGATOR_ELYTRA_ITEM_STACK), "The public entries should start at slot 2"); + Assertions.assertTrue(contents[3].isSimilar(Items.NAVIGATOR_SURVIVAL_ITEM_STACK)); + Assertions.assertTrue(contents[4].isSimilar(Items.NAVIGATOR_SLENDER_ITEM_STACK)); + Assertions.assertTrue(contents[5].isSimilar(Items.NAVIGATOR_CREATIVE_ITEM_STACK)); + for (int slot : new int[]{0, 1, 6, 7, 8}) { + Assertions.assertTrue(contents[slot].isSimilar(Items.NAVIGATOR_BLANK_ITEM_STACK), "Slot " + slot + " should hold the same filler every player sees"); + } + } + + @DisplayName("A build server that stopped is dropped from the menu on the next open") + @Test + void testOnlyReachableBuildServersAreShown(Env env) { + Instance flatInstance = env.createFlatInstance(); + Player player = env.createPlayer(flatInstance); + TestAudience audience = new TestAudience().grant(player.getUuid(), BuildServerAccess.PERMISSION); + + ItemStack[] running = openWith(env, player, audience, "Build-1"); + ItemStack[] stopped = openWith(env, player, audience); + + Assertions.assertTrue(contains(running, Items.navigatorBuildServer("Build-1")), "A running build server should be offered"); + Assertions.assertFalse(contains(stopped, Items.navigatorBuildServer("Build-1")), "A build server that is no longer reachable must disappear (US-5.04)"); + } + @DisplayName("A generally released destination is shown to an ordinary player") @Test void generallyReleasedEntriesAreShown(Env env) { NavigationHelper helper = NavigationHelper.instance(DummyDeliver.instance(), allReleased().gate()); Player player = env.createPlayer(env.createFlatInstance()); - assertEquals(Material.ELYTRA, openedSlot(env, helper, player, SLOT_ELYTRA)); - assertEquals(Material.GRASS_BLOCK, openedSlot(env, helper, player, SLOT_SURVIVAL)); - assertEquals(Material.ENDERMAN_SPAWN_EGG, openedSlot(env, helper, player, SLOT_SLENDER)); - assertEquals(Material.WOODEN_AXE, openedSlot(env, helper, player, SLOT_CREATIVE)); + Set shown = openedMaterials(env, helper, player); + + Assertions.assertTrue(shown.contains(Material.ELYTRA), "the elytra race should be offered"); + Assertions.assertTrue(shown.contains(Material.GRASS_BLOCK), "survival should be offered"); + Assertions.assertTrue(shown.contains(Material.ENDERMAN_SPAWN_EGG), "slender should be offered"); + Assertions.assertTrue(shown.contains(Material.WOODEN_AXE), "creative should be offered"); } @DisplayName("The kill switch removes the destination from the navigator, not just from /season status") @@ -115,10 +169,12 @@ void killSwitchHidesTheEntry(Env env) { NavigationHelper helper = NavigationHelper.instance(DummyDeliver.instance(), features.gate()); Player player = env.createPlayer(env.createFlatInstance()); - assertEquals(Material.GRAY_STAINED_GLASS_PANE, openedSlot(env, helper, player, SLOT_ELYTRA), "the elytra slot must fall back to the filler pane"); - assertEquals(Material.GRASS_BLOCK, openedSlot(env, helper, player, SLOT_SURVIVAL), "the other destinations keep their slots"); - assertEquals(Material.ENDERMAN_SPAWN_EGG, openedSlot(env, helper, player, SLOT_SLENDER)); - assertEquals(Material.WOODEN_AXE, openedSlot(env, helper, player, SLOT_CREATIVE)); + Set shown = openedMaterials(env, helper, player); + + Assertions.assertFalse(shown.contains(Material.ELYTRA), "the switched-off elytra race must be gone from the menu"); + Assertions.assertTrue(shown.contains(Material.GRASS_BLOCK), "the other destinations stay"); + Assertions.assertTrue(shown.contains(Material.ENDERMAN_SPAWN_EGG), "the other destinations stay"); + Assertions.assertTrue(shown.contains(Material.WOODEN_AXE), "the other destinations stay"); } @DisplayName("An internal destination is hidden from a player without the permission") @@ -128,7 +184,7 @@ void internalStageHidesTheEntryFromOrdinaryPlayers(Env env) { NavigationHelper helper = NavigationHelper.instance(DummyDeliver.instance(), features.gate()); Player player = env.createPlayer(env.createFlatInstance()); - assertEquals(Material.GRAY_STAINED_GLASS_PANE, openedSlot(env, helper, player, SLOT_SURVIVAL)); + Assertions.assertFalse(openedMaterials(env, helper, player).contains(Material.GRASS_BLOCK), "an internal destination must not reach an ordinary player"); } @DisplayName("An internal destination is shown to a team member") @@ -139,7 +195,7 @@ void internalStageShowsTheEntryToTheTeam(Env env) { Player player = env.createPlayer(env.createFlatInstance()); features.grant(player.getUuid(), ReleaseStage.INTERNAL_PERMISSION); - assertEquals(Material.GRASS_BLOCK, openedSlot(env, helper, player, SLOT_SURVIVAL)); + Assertions.assertTrue(openedMaterials(env, helper, player).contains(Material.GRASS_BLOCK), "a team member should be offered the internal destination"); } @DisplayName("A lite destination is shown to the lite group and hidden from everyone else") @@ -152,8 +208,8 @@ void liteStageFollowsTheGroup(Env env) { Player lite = env.createPlayer(instance); features.join(lite.getUuid(), ReleaseStage.LITE_GROUP); - assertEquals(Material.GRAY_STAINED_GLASS_PANE, openedSlot(env, helper, ordinary, SLOT_SLENDER)); - assertEquals(Material.ENDERMAN_SPAWN_EGG, openedSlot(env, helper, lite, SLOT_SLENDER)); + Assertions.assertFalse(openedMaterials(env, helper, ordinary).contains(Material.ENDERMAN_SPAWN_EGG), "a lite destination must stay hidden from everyone outside the group"); + Assertions.assertTrue(openedMaterials(env, helper, lite).contains(Material.ENDERMAN_SPAWN_EGG), "a member of the lite group should be offered it"); } @DisplayName("A flag flipped between two opens takes effect on the second open") @@ -163,11 +219,111 @@ void reopeningPicksUpAFlagChange(Env env) { NavigationHelper helper = NavigationHelper.instance(DummyDeliver.instance(), features.gate()); Player player = env.createPlayer(env.createFlatInstance()); - assertEquals(Material.ELYTRA, openedSlot(env, helper, player, SLOT_ELYTRA)); + Assertions.assertTrue(openedMaterials(env, helper, player).contains(Material.ELYTRA), "the released destination is offered on the first open"); // The per-player inventory builder is cached; the layout must not be. features.killSwitch(TitanFeatures.NAVIGATOR_ELYTRA); - assertEquals(Material.GRAY_STAINED_GLASS_PANE, openedSlot(env, helper, player, SLOT_ELYTRA)); + Assertions.assertFalse(openedMaterials(env, helper, player).contains(Material.ELYTRA), "the second open must reflect the flag change"); + } + + /** A fixture in which every navigator destination is generally released. */ + private static TestFeatureGate allReleased() { + return TestFeatureGate.create().release(TitanFeatures.NAVIGATOR_ELYTRA, ReleaseStage.GA).release(TitanFeatures.NAVIGATOR_SURVIVAL, ReleaseStage.GA).release(TitanFeatures.NAVIGATOR_SLENDER, ReleaseStage.GA).release(TitanFeatures.NAVIGATOR_CREATIVE, ReleaseStage.GA); + } + + /** + * Opens the navigator and returns what the player actually sees. Aves fills the data layout on + * the next tick ({@code InventoryBuilder.retrieveDataLayout} schedules it), so the inventory is + * still empty right after {@code open()} - the tick is part of opening the menu. + */ + private static ItemStack[] opened(Env env, NavigationHelper helper, Player player) { + helper.openNavigator(player); + Assertions.assertNotNull(player.getOpenInventory(), "the navigator should be open"); + env.tick(); + return player.getOpenInventory().getItemStacks().clone(); + } + + /** + * Opens the navigator and reports which destinations it ended up offering, by icon material. + * + *

Slots are derived from the entries that survive filtering, so nothing here may depend on + * a slot index: the question a gate test asks is whether a destination is in the menu at all. + * The filler is not a destination and is dropped, so {@code contains(FILLER)} can never be the + * accident that makes an assertion pass. + */ + private static Set openedMaterials(Env env, NavigationHelper helper, Player player) { + Set materials = new HashSet<>(); + for (ItemStack stack : opened(env, helper, player)) { + if (stack != null && !stack.isAir() && stack.material() != FILLER) { + materials.add(stack.material()); + } + } + return materials; + } + + /** + * Opens the navigator of a lobby offering the given build servers, with every public + * destination generally released, and returns what the player actually sees. + */ + private static ItemStack[] openWith(Env env, Player player, FeatureAudience audience, String... reachableServices) { + BuildServerDirectory directory = () -> List.of(reachableServices); + FeatureGate gate = allReleased().gate(); + NavigationHelper helper = NavigationHelper.instance(DummyDeliver.instance(), audience, gate, directory, BuildServerAccess.defaults()); + return opened(env, helper, player); + } + + private static boolean contains(ItemStack[] contents, ItemStack expected) { + return Arrays.stream(contents).anyMatch(stack -> stack != null && stack.isSimilar(expected)); } + + // @Disabled + // @DisplayName("Test if clicked on the teleporter item the navigation gui is + // opened") + // @Test + // void testNavigationHelperOpenNavigationGuiByClick(Env env) { + // Deliver deliver = spy(DummyDeliver.instance()); + // NavigationHelper helper = NavigationHelper.instance(deliver); + // + // Instance flatInstance = env.createFlatInstance(); + // Player realPlayer = env.createPlayer(flatInstance); + // + // helper.setItems(realPlayer); + // helper.openNavigator(realPlayer); + // System.out.println(realPlayer.getOpenInventory().getWindowId()); + // + // leftClickOpenInventory(realPlayer, 0, Items.NAVIGATOR_ELYTRA_ITEM_STACK); + // verify(deliver, atLeastOnce()).sendPlayer(any(), any()); + // leftClickOpenInventory(realPlayer, 3, Items.NAVIGATOR_SLENDER_ITEM_STACK); + // leftClickOpenInventory(realPlayer, 4, Items.NAVIGATOR_SURVIVAL_ITEM_STACK); + // leftClickOpenInventory(realPlayer, 5, Items.NAVIGATOR_SLENDER_ITEM_STACK); + // leftClickOpenInventory(realPlayer, 8, Items.NAVIGATOR_CREATIVE_ITEM_STACK); + // env.tick(); + // + // + // } + // + // private void leftClickOpenInventory(Player player, int slot, ItemStack + // clickedItem) { + // _leftClick(player.getOpenInventory(), true, player, slot, clickedItem); + // } + // private void _leftClick(AbstractInventory openInventory, boolean + // clickOpenInventory, Player player, int slot, ItemStack clickedItem) { + // final byte windowId = openInventory != null ? openInventory.getWindowId() : + // 0; + // if (clickOpenInventory) { + // assert openInventory != null; + // // Do not touch slot + // } else { + // int offset = openInventory != null ? openInventory.getInnerSize() : 0; + // slot = PlayerInventoryUtils.convertMinestomSlotToPlayerInventorySlot(slot); + // if (openInventory != null) { + // slot = slot - 9 + offset; + // } + // } + // player.addPacketToQueue(new ClientClickWindowPacket(windowId, 0, (short) + // slot, (byte) 0, + // ClientClickWindowPacket.ClickType.PICKUP, Map.of(), clickedItem)); + // player.interpretPacketQueue(); + // } } diff --git a/app/src/test/java/net/onelitefeather/titan/app/testutils/TestAudience.java b/app/src/test/java/net/onelitefeather/titan/app/testutils/TestAudience.java new file mode 100644 index 0000000..869fed7 --- /dev/null +++ b/app/src/test/java/net/onelitefeather/titan/app/testutils/TestAudience.java @@ -0,0 +1,56 @@ +/** + * Copyright (C) 2025 OneLiteFeather Network + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ +package net.onelitefeather.titan.app.testutils; + +import net.onelitefeather.titan.common.feature.FeatureAudience; + +import java.util.HashSet; +import java.util.Set; +import java.util.UUID; + +/** + * Fixture standing in for LuckPerms: a mutable set of granted permissions, so a test can take a + * permission away again between drawing a menu and acting on a click. + */ +public final class TestAudience implements FeatureAudience { + + private final Set permissions = new HashSet<>(); + + public TestAudience grant(UUID playerId, String permission) { + this.permissions.add(key(playerId, permission)); + return this; + } + + public TestAudience revoke(UUID playerId, String permission) { + this.permissions.remove(key(playerId, permission)); + return this; + } + + @Override + public boolean hasPermission(UUID playerId, String permission) { + return this.permissions.contains(key(playerId, permission)); + } + + @Override + public boolean inGroup(UUID playerId, String group) { + return false; + } + + private static String key(UUID playerId, String value) { + return playerId + "/" + value; + } +} diff --git a/common/src/main/java/net/onelitefeather/titan/common/utils/Items.java b/common/src/main/java/net/onelitefeather/titan/common/utils/Items.java index 927977a..32f1617 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/utils/Items.java +++ b/common/src/main/java/net/onelitefeather/titan/common/utils/Items.java @@ -18,6 +18,7 @@ import net.kyori.adventure.text.Component; import net.kyori.adventure.text.format.NamedTextColor; +import net.kyori.adventure.text.format.TextDecoration; import net.kyori.adventure.text.minimessage.MiniMessage; import net.minestom.server.component.DataComponents; import net.minestom.server.item.ItemStack; @@ -45,4 +46,16 @@ private Items() { public static final ItemStack NAVIGATOR_SURVIVAL_ITEM_STACK = ItemStack.builder(Material.GRASS_BLOCK).customName(MiniMessage.miniMessage().deserialize("Survival")).build(); public static final ItemStack NAVIGATOR_CREATIVE_ITEM_STACK = ItemStack.builder(Material.WOODEN_AXE).customName(MiniMessage.miniMessage().deserialize("Creative")).build(); + + /** + * Builds the navigator icon for one reachable build server. The service name is put in as + * plain text rather than through MiniMessage, so a service named with tag-like characters + * cannot inject formatting into the menu. + * + * @param serviceName the CloudNet service name, for example {@code Build-1} + * @return the icon shown for that build server + */ + public static ItemStack navigatorBuildServer(String serviceName) { + return ItemStack.builder(Material.SCAFFOLDING).customName(Component.text(serviceName, NamedTextColor.GOLD).decoration(TextDecoration.ITALIC, false)).build(); + } } From 9528e10ef096ab0389eb9fa7acc94c6dbaead6ef Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:58:17 +0200 Subject: [PATCH 4/7] feat(bridge): report the reachable build servers to the application US-5.04 asks for the servers that are reachable at the moment the menu opens, and CloudNet is the only source of that. :app deliberately does not depend on CloudNet, so the lookup lives here, where the driver is visible, and is installed into the TitanBuildServerDirectory holder; the application receives nothing but a List of service names. A service counts as reachable when it is RUNNING and connected to its node - a started process the node has not seen connect is not somewhere to send a player. Anything that cannot be answered, a missing driver or a failed lookup, is reported as "no build servers" rather than as a stale list, because the navigator promises reachability. --- .../TitanBridgePermissionExtension.java | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java b/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java index 1876b3c..e8b8e44 100644 --- a/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java +++ b/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java @@ -16,16 +16,26 @@ */ package net.onelitefeather.titan.bridge; +import eu.cloudnetservice.driver.inject.InjectionLayer; +import eu.cloudnetservice.driver.provider.CloudServiceProvider; import eu.cloudnetservice.driver.registry.ServiceRegistry; +import eu.cloudnetservice.driver.service.ServiceInfoSnapshot; +import eu.cloudnetservice.driver.service.ServiceLifeCycle; import eu.cloudnetservice.modules.bridge.impl.platform.minestom.MinestomPermissionChecker; import eu.cloudnetservice.modules.bridge.player.PlayerManager; import eu.cloudnetservice.modules.bridge.player.executor.PlayerExecutor; import eu.cloudnetservice.modules.bridge.player.executor.ServerSelectorType; +import java.util.ArrayList; +import java.util.List; import java.util.UUID; import net.minestom.server.extensions.Extension; import net.onelitefeather.titan.common.deliver.ServerConnector; import net.onelitefeather.titan.common.deliver.TitanServerConnector; +import net.onelitefeather.titan.common.navigator.BuildServerAccess; +import net.onelitefeather.titan.common.navigator.TitanBuildServerDirectory; import net.onelitefeather.titan.common.permission.TitanPermissionBridge; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Minestom extension that wires the CloudNet bridge to Titan across the classloader boundary. @@ -45,12 +55,20 @@ *

  • Server switching: installs a {@link ServerConnector} (used by * {@code MessageChannelDeliver}) that connects players through the bridge * {@link PlayerManager} / {@link PlayerExecutor}. + *
  • Build servers: installs the directory of reachable build servers the navigator + * reads (US-5.04). The CloudNet service list is only available here; the application receives + * nothing but a {@code List} of service names. * */ public final class TitanBridgePermissionExtension extends Extension { + private static final Logger LOGGER = LoggerFactory.getLogger(TitanBridgePermissionExtension.class); + @Override public void initialize() { + String buildServerTask = BuildServerAccess.defaults().taskName(); + TitanBuildServerDirectory.setDirectory(() -> reachableBuildServers(buildServerTask)); + MinestomPermissionChecker checker = (player, permission) -> TitanPermissionBridge.hasPermission(player.getUuid(), permission); ServiceRegistry.registry().registerProvider(MinestomPermissionChecker.class, "titan-luckperms", checker).markAsDefaultService(); @@ -73,6 +91,30 @@ public void connectToServer(UUID playerId, String serviceName) { }); } + /** + * Lists the services of the build task that are running and connected right now. Anything + * that cannot be answered - no driver, a failed lookup - is reported as "no build servers" + * rather than as a stale list, because the navigator promises reachability (US-5.04). + * + * @param taskName the CloudNet task the build servers run under + * @return the reachable service names + */ + private static List reachableBuildServers(String taskName) { + try { + CloudServiceProvider provider = InjectionLayer.boot().instance(CloudServiceProvider.class); + List names = new ArrayList<>(); + for (ServiceInfoSnapshot snapshot : provider.servicesByTask(taskName)) { + if (snapshot.lifeCycle() == ServiceLifeCycle.RUNNING && snapshot.connected()) { + names.add(snapshot.name()); + } + } + return List.copyOf(names); + } catch (RuntimeException exception) { + LOGGER.warn("Could not read the CloudNet service list for task {}; reporting no build servers", taskName, exception); + return List.of(); + } + } + private static PlayerExecutor playerExecutor(UUID playerId) { var registration = ServiceRegistry.registry().registration(PlayerManager.class, "PlayerManager"); return registration == null ? null : registration.serviceInstance().playerExecutor(playerId); From 052256fa8091c8125afb55abe13f12a875d50960 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 10:58:17 +0200 Subject: [PATCH 5/7] docs: mark stage 5 as implemented US-5.01 to US-5.04 are in place, and the acceptance criterion they satisfy - a player without titan.navigator.buildserver neither sees the build servers nor reaches them through a tampered click - is ticked. --- docs/spec-lobby-saison-events.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/spec-lobby-saison-events.md b/docs/spec-lobby-saison-events.md index e784f32..db94933 100644 --- a/docs/spec-lobby-saison-events.md +++ b/docs/spec-lobby-saison-events.md @@ -201,10 +201,10 @@ Abschnitt 6a. | ID | Story | Akzeptanzkriterium (EARS) | Schnittstelle | Priorität | Status | |---|---|---|---|---|---| -| US-5.01 | Als Teammitglied möchte ich die Build-Server im Navigator sehen, damit ich ohne Befehl dorthin komme. | Where ein Spieler die Berechtigung `titan.navigator.buildserver` hat, shall der Navigator die verfügbaren Build-Server als Ziele anzeigen. | `NavigationHelper` | Should | offen | -| US-5.02 | Als Betreiber möchte ich, dass Spieler ohne Berechtigung diese Ziele gar nicht sehen, damit ihre Existenz nicht verrät, dass es sie gibt. | If ein Spieler die Berechtigung nicht hat, then shall der Navigator die Build-Server-Einträge weder anzeigen noch ihren Platz freihalten. | `NavigationHelper` | Must | offen | -| US-5.03 | Als Betreiber möchte ich, dass die Berechtigung auch beim Wechsel geprüft wird, damit ein manipulierter Klick nichts bewirkt. | When ein Wechsel zu einem Build-Server angefordert wird, shall die Lobby die Berechtigung erneut prüfen, bevor sie den Spieler weiterleitet. | `Deliver` | Must | offen | -| US-5.04 | Als Teammitglied möchte ich sehen, welche Build-Server gerade laufen, damit ich nicht auf einen gestoppten klicke. | The Navigator shall nur Build-Server anzeigen, die zum Zeitpunkt des Öffnens als erreichbar gemeldet sind. | CloudNet-Dienstliste | Should | offen | +| US-5.01 | Als Teammitglied möchte ich die Build-Server im Navigator sehen, damit ich ohne Befehl dorthin komme. | Where ein Spieler die Berechtigung `titan.navigator.buildserver` hat, shall der Navigator die verfügbaren Build-Server als Ziele anzeigen. | `NavigationHelper` | Should | umgesetzt | +| US-5.02 | Als Betreiber möchte ich, dass Spieler ohne Berechtigung diese Ziele gar nicht sehen, damit ihre Existenz nicht verrät, dass es sie gibt. | If ein Spieler die Berechtigung nicht hat, then shall der Navigator die Build-Server-Einträge weder anzeigen noch ihren Platz freihalten. | `NavigationHelper` | Must | umgesetzt | +| US-5.03 | Als Betreiber möchte ich, dass die Berechtigung auch beim Wechsel geprüft wird, damit ein manipulierter Klick nichts bewirkt. | When ein Wechsel zu einem Build-Server angefordert wird, shall die Lobby die Berechtigung erneut prüfen, bevor sie den Spieler weiterleitet. | `Deliver` | Must | umgesetzt | +| US-5.04 | Als Teammitglied möchte ich sehen, welche Build-Server gerade laufen, damit ich nicht auf einen gestoppten klicke. | The Navigator shall nur Build-Server anzeigen, die zum Zeitpunkt des Öffnens als erreichbar gemeldet sind. | CloudNet-Dienstliste | Should | umgesetzt | ### Stufe 6 — Resource Packs (später) @@ -354,8 +354,8 @@ bekommen den Zeitpunkt übergeben, statt selbst auf die Uhr zu sehen. Die - [ ] Die Tageszeit der Lobby entspricht der Uhrzeit in Berlin, auch über eine Sommerzeitumstellung hinweg. - [ ] Die Zeitsteuerung ist mit einer festen `Clock` testbar; ein Test prüft Winter im Sommer. - [x] Ein Feature lässt sich nacheinander auf intern, lite und ga stellen, ohne dass Code geändert wird. -- [x] Der Notausschalter wirkt innerhalb von zwei Sekunden und schlägt Stufe und Zeitfenster. — *Einschränkung: die Prüfung erfolgt beim Zeichnen des Menüs. Wer den Navigator bereits offen hat, sieht das alte Bild bis zum nächsten Öffnen. Ein abgelehnter Eintrag bekommt keinen Klick-Handler, und `InventoryPreClickEvent` wird global abgebrochen — das Fenster ist also eng, aber vorhanden. Eine Prüfung zur Klickzeit gehört zu `NavigatorEntry` aus Stufe 5.* -- [ ] Ein Spieler ohne `titan.navigator.buildserver` sieht die Build-Server nicht und kann sie auch durch einen manipulierten Klick nicht erreichen. +- [x] Der Notausschalter wirkt innerhalb von zwei Sekunden und schlägt Stufe und Zeitfenster. — *Einschränkung: die Prüfung erfolgt beim Zeichnen des Menüs. Wer den Navigator bereits offen hat, sieht das alte Bild bis zum nächsten Öffnen. Ein abgelehnter Eintrag bekommt keinen Klick-Handler, und `InventoryPreClickEvent` wird global abgebrochen — das Fenster ist also eng, aber vorhanden. Die Prüfung zur Klickzeit ist mit Stufe 5 in `GuardedDeliver` nachgezogen, greift dort aber nur für die Berechtigung eines Eintrags, nicht für den Notausschalter.* +- [x] Ein Spieler ohne `titan.navigator.buildserver` sieht die Build-Server nicht und kann sie auch durch einen manipulierten Klick nicht erreichen. - [ ] Die Lobby startet ohne Saison-Paket vollständig funktionsfähig. - [ ] Ein Saison-Paket lässt sich entfernen, ohne dass Reste in der Welt zurückbleiben. - [ ] Der Rollout-Stand jedes Features ist in `docs/rollout-log.md` nachvollziehbar. From 472b169310fdeee7ff6e1516652869f810656ff6 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 12:14:51 +0200 Subject: [PATCH 6/7] fix(navigator): let CloudNet decide which services are build servers The navigator re-derived build-server membership from the service name after the bridge had already asked CloudNet for the services of the build task. That re-derivation assumed the name separator is always "-", but CloudNet configures it per task (ServiceTask/ServiceId.nameSplitter, default "-"): a Build task set to "_" produces Build_1, every entry was filtered away again, and a team member holding the permission saw an empty list with no explanation. The list now comes from servicesByTask unchanged, because that query is the authority on which services belong to the task. The name-based rule stays where it is needed and cannot be replaced: the guard has to recognise a build server that has stopped and is in no service list at all, otherwise a request naming it would look like an ordinary destination. That rule now follows CloudNet's own naming instead of guessing at it. A service is named , so the splitter is configuration here as well (-Dtitan.buildserver.namesplitter) and the numeric tail is required - which also retires the latent false positive where a separate task named Build-Test would have had its services claimed by the Build rule. The bridge reads the task's real splitter once and warns when it disagrees with the configured one, so the drift cannot go unnoticed the way it did here. Also resolves driver services through InjectionLayer.ext() rather than boot(): ext is the layer CloudNet documents for external components such as extensions, and it is a child of boot, so every boot binding remains visible through it. --- .../titan/app/helper/NavigationHelper.java | 10 +- .../app/helper/NavigationHelperTest.java | 17 +++ .../TitanBridgePermissionExtension.java | 68 ++++++++++-- .../common/navigator/BuildServerAccess.java | 101 ++++++++++++++---- .../navigator/BuildServerDirectory.java | 6 ++ .../navigator/BuildServerAccessTest.java | 45 ++++++++ docs/olf-minestom-project-standard.md | 6 +- 7 files changed, 219 insertions(+), 34 deletions(-) diff --git a/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java b/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java index b1e72da..e7b3ea5 100644 --- a/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java +++ b/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java @@ -156,6 +156,14 @@ List layoutFor(UUID playerId) { * looked up for a player who holds the permission — a player who does not is not a reason to * ask CloudNet anything, and their menu must not depend on the answer. * + *

    What the directory reports is taken as it stands. CloudNet is asked for the services of + * the build task by name ({@code servicesByTask}) and is the authority on which those are; + * re-deriving the membership from the service name here could only ever subtract from that + * answer, and would subtract everything for a task whose {@code nameSplitter} is not the one + * Titan is configured with. Deriving membership from a name is still what + * {@link BuildServerAccess} does for the guard, where it has to be a name, because the guard + * must also recognise a build server that is not in this list any more. + * * @param playerId the player the menu is drawn for * @return the public entries, followed by the reachable build servers in a stable order */ @@ -169,7 +177,7 @@ private List entriesFor(UUID playerId) { if (!this.audience.hasPermission(playerId, this.access.permission())) { return List.copyOf(entries); } - this.buildServers.reachableServices().stream().filter(this.access::covers).sorted().map(service -> NavigatorEntry.restrictedServer(Items.navigatorBuildServer(service), service, this.access.permission())).forEach(entries::add); + this.buildServers.reachableServices().stream().sorted().map(service -> NavigatorEntry.restrictedServer(Items.navigatorBuildServer(service), service, this.access.permission())).forEach(entries::add); return List.copyOf(entries); } diff --git a/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java b/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java index eca139b..a98fef3 100644 --- a/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java +++ b/app/src/test/java/net/onelitefeather/titan/app/helper/NavigationHelperTest.java @@ -132,6 +132,23 @@ void testUnprivilegedMenuHasNoEmptySlot(Env env) { } } + @DisplayName("A build server whose name does not follow the assumed pattern is still offered") + @Test + void testBuildServersAreNotSecondGuessedByName(Env env) { + Instance flatInstance = env.createFlatInstance(); + Player player = env.createPlayer(flatInstance); + TestAudience audience = new TestAudience().grant(player.getUuid(), BuildServerAccess.PERMISSION); + + // CloudNet was asked for the services of the build task by name and answered with these, + // so these are build servers - whatever splitter the task happens to use (a task set to + // "_" produces Build_1). Re-deriving membership from the name here would drop every one + // of them and leave a team member staring at an empty list with no explanation. + ItemStack[] contents = openWith(env, player, audience, "Build_1", "Build_2"); + + Assertions.assertTrue(contains(contents, Items.navigatorBuildServer("Build_1")), "The directory is the authority on which services belong to the task"); + Assertions.assertTrue(contains(contents, Items.navigatorBuildServer("Build_2"))); + } + @DisplayName("A build server that stopped is dropped from the menu on the next open") @Test void testOnlyReachableBuildServersAreShown(Env env) { diff --git a/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java b/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java index e8b8e44..4ae2207 100644 --- a/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java +++ b/bridge/src/main/java/net/onelitefeather/titan/bridge/TitanBridgePermissionExtension.java @@ -18,9 +18,11 @@ import eu.cloudnetservice.driver.inject.InjectionLayer; import eu.cloudnetservice.driver.provider.CloudServiceProvider; +import eu.cloudnetservice.driver.provider.ServiceTaskProvider; import eu.cloudnetservice.driver.registry.ServiceRegistry; import eu.cloudnetservice.driver.service.ServiceInfoSnapshot; import eu.cloudnetservice.driver.service.ServiceLifeCycle; +import eu.cloudnetservice.driver.service.ServiceTask; import eu.cloudnetservice.modules.bridge.impl.platform.minestom.MinestomPermissionChecker; import eu.cloudnetservice.modules.bridge.player.PlayerManager; import eu.cloudnetservice.modules.bridge.player.executor.PlayerExecutor; @@ -28,6 +30,7 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; import net.minestom.server.extensions.Extension; import net.onelitefeather.titan.common.deliver.ServerConnector; import net.onelitefeather.titan.common.deliver.TitanServerConnector; @@ -59,15 +62,24 @@ * reads (US-5.04). The CloudNet service list is only available here; the application receives * nothing but a {@code List} of service names. * + * + *

    Driver services are resolved through {@link InjectionLayer#ext()}, the layer CloudNet + * documents for "all kinds of external component injection (like plugins)" — an extension is such + * a component. The ext layer is created as a child of the boot layer + * ({@code InjectionLayerProvider.boot()} ends with {@code ext = child(boot, "ext")}), so every + * boot binding is visible through it and nothing is lost by not asking boot directly. */ public final class TitanBridgePermissionExtension extends Extension { private static final Logger LOGGER = LoggerFactory.getLogger(TitanBridgePermissionExtension.class); + /** Guards the one-off name-splitter check so a mismatch is reported once, not every menu. */ + private static final AtomicBoolean NAME_SPLITTER_CHECKED = new AtomicBoolean(); + @Override public void initialize() { - String buildServerTask = BuildServerAccess.defaults().taskName(); - TitanBuildServerDirectory.setDirectory(() -> reachableBuildServers(buildServerTask)); + BuildServerAccess buildServerAccess = BuildServerAccess.defaults(); + TitanBuildServerDirectory.setDirectory(() -> reachableBuildServers(buildServerAccess)); MinestomPermissionChecker checker = (player, permission) -> TitanPermissionBridge.hasPermission(player.getUuid(), permission); ServiceRegistry.registry().registerProvider(MinestomPermissionChecker.class, "titan-luckperms", checker).markAsDefaultService(); @@ -92,29 +104,65 @@ public void connectToServer(UUID playerId, String serviceName) { } /** - * Lists the services of the build task that are running and connected right now. Anything - * that cannot be answered - no driver, a failed lookup - is reported as "no build servers" - * rather than as a stale list, because the navigator promises reachability (US-5.04). + * Lists the services of the build task that are running and connected right now. CloudNet is + * asked by task name, so it - not a name pattern on this side - decides which services belong + * to the task; the application offers what comes back unchanged. Anything that cannot be + * answered - no driver, a failed lookup - is reported as "no build servers" rather than as a + * stale list, because the navigator promises reachability (US-5.04). * - * @param taskName the CloudNet task the build servers run under + * @param access the task the build servers run under and how it names its services * @return the reachable service names */ - private static List reachableBuildServers(String taskName) { + private static List reachableBuildServers(BuildServerAccess access) { + verifyNameSplitter(access); try { - CloudServiceProvider provider = InjectionLayer.boot().instance(CloudServiceProvider.class); + CloudServiceProvider provider = InjectionLayer.ext().instance(CloudServiceProvider.class); List names = new ArrayList<>(); - for (ServiceInfoSnapshot snapshot : provider.servicesByTask(taskName)) { + for (ServiceInfoSnapshot snapshot : provider.servicesByTask(access.taskName())) { if (snapshot.lifeCycle() == ServiceLifeCycle.RUNNING && snapshot.connected()) { names.add(snapshot.name()); } } return List.copyOf(names); } catch (RuntimeException exception) { - LOGGER.warn("Could not read the CloudNet service list for task {}; reporting no build servers", taskName, exception); + LOGGER.warn("Could not read the CloudNet service list for task {}; reporting no build servers", access.taskName(), exception); return List.of(); } } + /** + * Compares the name splitter Titan assumes against the one the task is really configured with, + * once the driver can answer. The menu does not depend on the splitter any more, but the guard + * in {@code GuardedDeliver} does: it has to recognise a build server by name, including one + * that has stopped and is in no service list. A task switched to {@code _} while Titan still + * assumes {@code -} would leave those destinations unguarded, and nothing else would say so. + * + *

    The result is reported once and then never again - the check is diagnostics, and a line + * per opened menu would be noise. A lookup that fails is not a result: the flag stays down so + * the next menu tries again, and the failure itself stays at debug level because the service + * list below reports the same outage where an operator will look for it. + * + * @param access the task and splitter Titan is configured with + */ + private static void verifyNameSplitter(BuildServerAccess access) { + if (NAME_SPLITTER_CHECKED.get()) { + return; + } + try { + ServiceTask task = InjectionLayer.ext().instance(ServiceTaskProvider.class).serviceTask(access.taskName()); + if (!NAME_SPLITTER_CHECKED.compareAndSet(false, true)) { + return; + } + if (task == null) { + LOGGER.warn("CloudNet knows no task named {}; the navigator will never offer a build server. Set -D{} if the task is named differently.", access.taskName(), BuildServerAccess.TASK_PROPERTY); + } else if (!task.nameSplitter().equals(access.nameSplitter())) { + LOGGER.warn("Task {} names its services with the splitter '{}' but Titan assumes '{}', so a build server would not be recognised as one when a switch is requested. Set -D{}={}.", access.taskName(), task.nameSplitter(), access.nameSplitter(), BuildServerAccess.SPLITTER_PROPERTY, task.nameSplitter()); + } + } catch (RuntimeException exception) { + LOGGER.debug("Could not read the CloudNet task {} to verify its name splitter", access.taskName(), exception); + } + } + private static PlayerExecutor playerExecutor(UUID playerId) { var registration = ServiceRegistry.registry().registration(PlayerManager.class, "PlayerManager"); return registration == null ? null : registration.serviceInstance().playerExecutor(playerId); diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java index 3955de9..aac889b 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerAccess.java @@ -19,25 +19,34 @@ import net.onelitefeather.deliver.DeliverComponent; import org.jetbrains.annotations.Contract; -import java.util.Locale; - /** * Says which CloudNet destinations count as build servers and which permission they require. * - *

    The membership test deliberately works on names rather than on the list of servers that - * happen to be online. A destination is a build server because of the task it belongs to, not - * because it is currently reachable — otherwise a request naming a stopped build server would - * slip past the guard as an ordinary destination, and the check in {@link GuardedDeliver} would - * only hold for as long as the menu was accurate. CloudNet names a service {@code -}, - * so the task name is recoverable from the service name alone, with JDK types only. + *

    This is the rule the guard uses, and it deliberately works on names rather than on + * the list of servers that happen to be online. A destination is a build server because of the + * task it belongs to, not because it is currently reachable — otherwise a request naming a stopped + * build server would slip past the guard as an ordinary destination, and the check in + * {@link GuardedDeliver} would only hold for as long as the menu was accurate. + * + *

    Which names those are is fixed by CloudNet: {@code ServiceId.name()} returns + * {@code taskName + nameSplitter + taskServiceId}, where the id is a number and the splitter is + * configured per task — {@code -} by default, but a task set to {@code _} produces + * {@code Build_1}. Both halves are therefore configuration here as well, and the numeric tail is + * required: it is what keeps a different task named {@code Build-Test} from having its services + * ({@code Build-Test-1}) mistaken for services of {@code Build}. + * + *

    Membership of the menu is a different question and is not answered here. CloudNet + * answers that one authoritatively through {@code servicesByTask}; see + * {@link BuildServerDirectory}. * - * @param taskName the CloudNet task the build servers belong to - * @param permission the permission required to see and to reach them + * @param taskName the CloudNet task the build servers belong to + * @param nameSplitter what the task puts between its name and the numeric service id + * @param permission the permission required to see and to reach them * @author TheMeinerLP - * @version 1.0.0 + * @version 2.0.0 * @since 1.15.0 */ -public record BuildServerAccess(String taskName, String permission) { +public record BuildServerAccess(String taskName, String nameSplitter, String permission) { /** Permission a team member needs for the build servers (US-5.01). */ public static final String PERMISSION = "titan.navigator.buildserver"; @@ -45,6 +54,9 @@ public record BuildServerAccess(String taskName, String permission) { /** CloudNet task the build servers run under when nothing else is configured. */ public static final String DEFAULT_TASK = "Build"; + /** The name splitter CloudNet gives a task that does not configure one. */ + public static final String DEFAULT_NAME_SPLITTER = "-"; + /** * System property overriding {@value #DEFAULT_TASK} for a network that names its task * differently. @@ -52,28 +64,50 @@ public record BuildServerAccess(String taskName, String permission) { public static final String TASK_PROPERTY = "titan.buildserver.task"; /** - * Returns the access rule for this deployment: the task from {@value #TASK_PROPERTY} or - * {@value #DEFAULT_TASK}, guarded by {@value #PERMISSION}. + * System property overriding {@value #DEFAULT_NAME_SPLITTER} for a task whose CloudNet + * {@code nameSplitter} is not the default. The bridge warns when this disagrees with what + * CloudNet actually reports for the task. + */ + public static final String SPLITTER_PROPERTY = "titan.buildserver.namesplitter"; + + /** + * Creates a rule for a task that uses CloudNet's default name splitter. + * + * @param taskName the CloudNet task the build servers belong to + * @param permission the permission required to see and to reach them + */ + public BuildServerAccess(String taskName, String permission) { + this(taskName, DEFAULT_NAME_SPLITTER, permission); + } + + /** + * Returns the access rule for this deployment: the task from {@value #TASK_PROPERTY} and the + * splitter from {@value #SPLITTER_PROPERTY}, guarded by {@value #PERMISSION}. * * @return the configured access rule */ @Contract(value = "-> new", pure = true) public static BuildServerAccess defaults() { - String configured = System.getProperty(TASK_PROPERTY, DEFAULT_TASK).trim(); - return new BuildServerAccess(configured.isEmpty() ? DEFAULT_TASK : configured, PERMISSION); + return new BuildServerAccess(property(TASK_PROPERTY, DEFAULT_TASK), property(SPLITTER_PROPERTY, DEFAULT_NAME_SPLITTER), PERMISSION); } /** - * Checks whether a service name belongs to the build task. + * Checks whether a name belongs to the build task — either the task name itself or one of its + * services, {@code }. * - * @param serviceName the CloudNet service name, for example {@code Build-1} - * @return whether the service is a build server + * @param serviceName the CloudNet task or service name, for example {@code Build-1} + * @return whether the name is the build task or one of its services */ @Contract(pure = true) public boolean covers(String serviceName) { - String name = serviceName.toLowerCase(Locale.ROOT); - String task = this.taskName.toLowerCase(Locale.ROOT); - return name.equals(task) || name.startsWith(task + "-"); + if (serviceName.equalsIgnoreCase(this.taskName)) { + return true; + } + String prefix = this.taskName + this.nameSplitter; + if (!serviceName.regionMatches(true, 0, prefix, 0, prefix.length())) { + return false; + } + return isServiceId(serviceName.substring(prefix.length())); } /** @@ -92,4 +126,27 @@ public boolean covers(DeliverComponent component) { default -> false; }; } + + /** + * Whether the tail behind the splitter is a CloudNet service id, which is a plain number. + * Anything else — {@code Test-1} behind {@code Build-} — belongs to a different task. + */ + @Contract(pure = true) + private static boolean isServiceId(String tail) { + if (tail.isEmpty()) { + return false; + } + for (int index = 0; index < tail.length(); index++) { + char character = tail.charAt(index); + if (character < '0' || character > '9') { + return false; + } + } + return true; + } + + private static String property(String key, String fallback) { + String configured = System.getProperty(key, fallback).trim(); + return configured.isEmpty() ? fallback : configured; + } } diff --git a/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java index 4a0eccd..795c608 100644 --- a/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java +++ b/common/src/main/java/net/onelitefeather/titan/common/navigator/BuildServerDirectory.java @@ -28,6 +28,12 @@ * the same rule {@code net.onelitefeather.titan.common.deliver.ServerConnector} and * {@code net.onelitefeather.titan.common.permission.TitanPermissionBridge} follow. * + *

    An implementation reports build servers and nothing else — it is asked to select them, not + * merely to list services for someone else to filter. The navigator therefore offers what it + * receives unchanged, which is why the production implementation queries CloudNet by task name + * instead of by a name pattern. {@link BuildServerAccess} answers the other question, the one the + * guard asks about a destination that may no longer be in this list at all. + * * @author TheMeinerLP * @version 1.0.0 * @since 1.15.0 diff --git a/common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java b/common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java index deab0ec..a63da14 100644 --- a/common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java +++ b/common/src/test/java/net/onelitefeather/titan/common/navigator/BuildServerAccessTest.java @@ -42,9 +42,53 @@ void testServiceNamesOfTheBuildTask() { void testForeignServiceNames() { Assertions.assertFalse(this.access.covers("Lobby-1")); Assertions.assertFalse(this.access.covers("BuildBattle-1"), "A different task with the same prefix is not the build task"); + Assertions.assertFalse(this.access.covers("Build-"), "A name with no service id behind the splitter is no service"); Assertions.assertFalse(this.access.covers("")); } + @DisplayName("A different task whose name starts with the build task is not the build task") + @Test + void testTaskWhoseNameStartsWithTheBuildTask() { + // CloudNet names a service , so the tail behind the splitter is + // the deciding evidence: 'Test-1' is not a number, therefore 'Build-Test-1' belongs to a + // task called Build-Test and must not be guarded as if it were a build server. + Assertions.assertFalse(this.access.covers("Build-Test")); + Assertions.assertFalse(this.access.covers("Build-Test-1")); + Assertions.assertFalse(this.access.covers(server("Build-Test-1"))); + } + + @DisplayName("A task that splits its service names differently is recognised once it is configured") + @Test + void testConfiguredNameSplitter() { + // The splitter is a per-task CloudNet setting (ServiceTask.nameSplitter, default "-"), so + // a Build task set to "_" produces Build_1 and the rule has to be told about it. + BuildServerAccess underscore = new BuildServerAccess("Build", "_", BuildServerAccess.PERMISSION); + + Assertions.assertTrue(underscore.covers("Build_1")); + Assertions.assertTrue(underscore.covers(server("Build_9")), "A stopped one is guarded just the same"); + Assertions.assertFalse(underscore.covers("Build-1"), "A splitter the task does not use names no service of it"); + Assertions.assertFalse(this.access.covers("Build_1"), "The default rule expects CloudNet's default splitter"); + } + + @DisplayName("The name splitter can be overridden per deployment") + @Test + void testNameSplitterOverride() { + String previous = System.getProperty(BuildServerAccess.SPLITTER_PROPERTY); + try { + System.setProperty(BuildServerAccess.SPLITTER_PROPERTY, "_"); + BuildServerAccess overridden = BuildServerAccess.defaults(); + + Assertions.assertEquals("_", overridden.nameSplitter()); + Assertions.assertTrue(overridden.covers("Build_1")); + } finally { + if (previous == null) { + System.clearProperty(BuildServerAccess.SPLITTER_PROPERTY); + } else { + System.setProperty(BuildServerAccess.SPLITTER_PROPERTY, previous); + } + } + } + @DisplayName("A stopped build server is still a build server") @Test void testOfflineBuildServerIsStillGuarded() { @@ -88,6 +132,7 @@ void testDefaults() { BuildServerAccess defaults = BuildServerAccess.defaults(); Assertions.assertEquals(BuildServerAccess.DEFAULT_TASK, defaults.taskName()); + Assertions.assertEquals(BuildServerAccess.DEFAULT_NAME_SPLITTER, defaults.nameSplitter()); Assertions.assertEquals("titan.navigator.buildserver", defaults.permission()); } diff --git a/docs/olf-minestom-project-standard.md b/docs/olf-minestom-project-standard.md index fbc1ca2..c3ad29c 100644 --- a/docs/olf-minestom-project-standard.md +++ b/docs/olf-minestom-project-standard.md @@ -932,7 +932,11 @@ setzt voraus, dass eine spätere bereits begonnen wurde. ### Phase 3 — Reuse statt Eigenbau - `common/utils` und `common/helper` fachlich auflösen (OLF-L3-02, Tabelle in - Abschnitt 3) + Abschnitt 3). **Anmerkung:** `Items` hat in Stufe 5 die Factory + `navigatorBuildServer(...)` dazubekommen und liegt damit weiter in `utils`. + Bewusst nicht vorgezogen: die Methode gehört zu den `NAVIGATOR_*`-Konstanten + daneben, ein Einzelumzug würde die Navigator-Icons auf zwei Pakete verteilen. + Sie zieht mit der ganzen Klasse nach `common/item`. - `app/listener` nach Fachlichkeit untergliedern (OLF-L3-03) - `ThreadHelper` und `SingletonFeatureManagerProvider` in Titan löschen und aus Butterfly beziehen (OLF-L2-04). **Vorbedingung:** Butterfly muss sie als From acab8d7ed49bb51494ed3cafa9054530a7964413 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Fri, 28 Aug 2026 12:15:11 +0200 Subject: [PATCH 7/7] refactor(navigator): drop the Togglz thread-local user binding NavigationHelper bound a SimpleFeatureUser to the tick thread while the layout was computed and released it afterwards, without try/finally - any throw from FeatureGate or LuckPerms in between leaked the binding onto a thread that lives for the whole server. Wrapping it properly would have preserved nothing: FeatureGate.decide reads the FeatureState directly and takes the player id as a parameter, so it never asks Togglz's UserProvider anything. The binding is left over from the pre-gate implementation and is deleted rather than repaired. --- .../onelitefeather/titan/app/helper/NavigationHelper.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java b/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java index e7b3ea5..cc3a2e8 100644 --- a/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java +++ b/app/src/main/java/net/onelitefeather/titan/app/helper/NavigationHelper.java @@ -37,8 +37,6 @@ import net.theevilreaper.aves.inventory.click.ClickHolder; import net.theevilreaper.aves.inventory.util.LayoutCalculator; import org.jetbrains.annotations.Nullable; -import org.togglz.core.user.SimpleFeatureUser; -import org.togglz.core.user.thread.ThreadLocalUserProvider; import java.time.Duration; import java.util.ArrayList; @@ -126,7 +124,6 @@ public void setItems(Player player) { // Blank the whole row first: every slot that no visible entry claims is filler, and // filler is what a slot holding a hidden entry would have to look like anyway. finalLayout.setItems(LayoutCalculator.fillRow(NAVIGATOR_TYPE), Items.NAVIGATOR_BLANK_ITEM_STACK); - ThreadLocalUserProvider.bind(toUser(player)); for (NavigatorLayout.Placement placement : layoutFor(player.getUuid())) { NavigatorEntry entry = placement.entry(); finalLayout.setItem(placement.slot(), entry.icon(), (clicker, slot, click, itemStack, result) -> { @@ -134,7 +131,6 @@ public void setItems(Player player) { result.accept(ClickHolder.cancelClick()); }); } - ThreadLocalUserProvider.release(); return finalLayout; }); inventoryBuilder.register(); @@ -181,10 +177,6 @@ private List entriesFor(UUID playerId) { return List.copyOf(entries); } - private SimpleFeatureUser toUser(Player player) { - return new SimpleFeatureUser(player.getUsername()); - } - /** * Creates a navigator that offers the public entries only. Used where no permission backend is * available, which is the safe reading of "unknown player".