Persist NoClip state after login (dependent on /gmic)

README.md

@@ -14,6 +14,10 @@ Supports LuckPerms, Java 17, and Gradle. Built against Purpur 1.19.4, though it
 
 Compatible with [GameModeInventories-OG](https://github.com/true-og/GameModeInventories-OG) (3.5.0+). Players with `noclip.use` permission can freely enter spectator mode even when GMI's `restrict_spectator` option is enabled.
 
+Enter creative with `/gmic` before enabling `/nc`. NoClip remembers the toggle in the player's persistent data across disconnects and restarts, including when the player was standing on the ground. Turning `/nc` off, dying, or leaving creative/spectator while NoClip is active clears it.
+
+[Spawn-OG](https://github.com/true-og/Spawn-OG) controls restoration. With login safety enabled in a managed world, NoClip resumes when `/gmic` and `noclip.use` are still permitted at the login location or the destination of a configured join teleport or rescue. `/spawnback` checks those same permissions and destination rules. A grounded toggle stays grounded when restored. `spawnog.login-migration.bypass` only exempts players from autopsy migration; it does not govern NoClip, `/gmic`, or `/fly` restoration. NoClip never starts phasing merely because saved player data was loaded; without Spawn-OG, enable it again using `/nc`.
+
 NoClip-OG and all other plugins in the "OG" suite are developed for [TrueOG](https://trueog.net), a 100% free and open source Minecraft Server. Need support? Join our [Discord](https://discord.gg/ma9pMYpBU6) and ask your question in #developer-chat.
 
 All contributors to this repository must sign the Contributor License Agreement. See CLA.txt for details.

build.gradle.kts

@@ -48,8 +48,14 @@ repositories {
 dependencies {
     compileOnly("org.purpurmc.purpur:purpur-api:1.19.4-R0.1-SNAPSHOT") // Declare Purpur API version to be packaged.
     compileOnly("io.github.miniplaceholders:miniplaceholders-api:2.2.3") // Import MiniPlaceholders API.
+    testImplementation("org.purpurmc.purpur:purpur-api:1.19.4-R0.1-SNAPSHOT")
+    testImplementation("org.junit.jupiter:junit-jupiter:5.10.2")
+    testImplementation("org.mockito:mockito-core:5.14.2")
+    testRuntimeOnly("org.junit.platform:junit-platform-launcher:1.10.2")
 }
 
+tasks.test { useJUnitPlatform() }
+
 /* ---------------------- Reproducible jars ---------------------------- */
 tasks.withType<AbstractArchiveTask>().configureEach { // Ensure reproducible .jars
     isPreserveFileTimestamps = false

src/main/java/plugin/CommandManager.java

@@ -40,7 +40,9 @@ public class CommandManager implements CommandExecutor {
                 final Player player = (Player) sender;
 
                 // Only run the command in appropriate gamemodes.
-                if (player.getGameMode() == GameMode.CREATIVE || player.getGameMode() == GameMode.SPECTATOR) {
+                if (player.getGameMode() == GameMode.CREATIVE
+                        || player.getGameMode() == GameMode.SPECTATOR && manager.isNoClipping(player.getUniqueId()))
+                {
 
                     // Checks whether the "enabled" code has been run for the player yet.
                     if (manager.isNoClipping(player.getUniqueId())) {

src/main/java/plugin/Listeners.java

@@ -12,6 +12,7 @@ import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
 import org.bukkit.event.entity.PlayerDeathEvent;
 import org.bukkit.event.player.PlayerMoveEvent;
+import org.bukkit.event.player.PlayerGameModeChangeEvent;
 import org.bukkit.event.player.PlayerQuitEvent;
 
 public class Listeners implements Listener {
@@ -87,6 +88,18 @@ public class Listeners implements Listener {
 
     }
 
+    // /gmic off and region-enforced survival end the active toggle. Cancelled
+    // changes and creative/spectator phasing leave it intact. An inactive login
+    // may be normalized before Spawn-OG reads the saved toggle, so leave it alone.
+    @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
+    public void onGameModeChange(PlayerGameModeChangeEvent event) {
+
+        if (event.getNewGameMode() != GameMode.CREATIVE && event.getNewGameMode() != GameMode.SPECTATOR
+                && manager.isNoClipping(event.getPlayer().getUniqueId()))
+            manager.exitNoClip(event.getPlayer());
+
+    }
+
     // MONITOR so Spawn-OG's LOWEST-priority quit snapshot reads NoClip state
     // before it is cleared here. Kicks funnel into this same quit event; a
     // dedicated kick handler would run before that snapshot and corrupt it.
@@ -97,7 +110,7 @@ public class Listeners implements Listener {
         final Player player = playerThatQuit.getPlayer();
 
         // If player has a NoClip instance in the set...
-        if (manager.exitNoClip(player)) {
+        if (manager.suspendNoClip(player)) {
 
             // Leave creative behind, never spectator, so a relog on a server
             // without Spawn-OG does not strand the player phased into a block.

src/main/java/plugin/NoClip.java

@@ -6,6 +6,7 @@ package plugin;
 import java.util.UUID;
 import org.bukkit.Bukkit;
 import org.bukkit.GameMode;
+import org.bukkit.NamespacedKey;
 import org.bukkit.entity.Player;
 import org.bukkit.plugin.Plugin;
 import org.bukkit.plugin.java.JavaPlugin;
@@ -21,7 +22,7 @@ public class NoClip extends JavaPlugin {
     public void onEnable() {
 
         // Create the shared state holder before anything can consume it.
-        manager = new NoClipManager();
+        manager = new NoClipManager(new NamespacedKey(this, "enabled"));
         // Register the instantiation of the plugin (triggers loading with messages in
         // console).
         getServer().getPluginManager().registerEvents(new Listeners(manager), (Plugin) this);
@@ -42,11 +43,17 @@ public class NoClip extends JavaPlugin {
     // reboot).
     public void onDisable() {
 
+        if (manager == null)
+            return;
+
         // Iterate a snapshot copy so removal cannot upset the loop.
         for (UUID playerId : manager.activePlayers()) {
 
             // The player may already be gone; only touch the ones still online.
             final Player player = Bukkit.getPlayer(playerId);
+            // Stop the live session before any gamemode listeners can mistake
+            // shutdown cleanup for an intentional exit.
+            manager.suspendNoClip(player);
             if (player != null && player.isOnline()) {
 
                 // Get player out of block-phase (spectator) mode.
@@ -54,13 +61,6 @@ public class NoClip extends JavaPlugin {
 
             }
 
-            // Remove player from active list.
-            if (player != null) {
-
-                manager.exitNoClip(player);
-
-            }
-
         }
 
     }

src/main/java/plugin/NoClipManager.java

@@ -7,7 +7,9 @@ import java.util.HashSet;
 import java.util.Set;
 import java.util.UUID;
 import org.bukkit.GameMode;
+import org.bukkit.NamespacedKey;
 import org.bukkit.entity.Player;
+import org.bukkit.persistence.PersistentDataType;
 
 // Owns the set of players in NoClip mode. Keyed by UUID so a username change
 // cannot orphan or hijack an entry. Main-thread only, like all Bukkit state.
@@ -16,6 +18,22 @@ public class NoClipManager {
 
     // Players currently in NoClip mode.
     private final Set<UUID> active = new HashSet<>();
+    // Saved with the player, independently of the live phasing session. Spawn-OG
+    // decides when it may be restored; loading player data never enables /nc.
+    private final NamespacedKey intentKey;
+
+    public NoClipManager(NamespacedKey intentKey) {
+
+        this.intentKey = intentKey;
+
+    }
+
+    public boolean hasNoClipIntent(Player player) {
+
+        return player != null && Byte.valueOf((byte) 1)
+                .equals(player.getPersistentDataContainer().get(intentKey, PersistentDataType.BYTE));
+
+    }
 
     // Reports whether the player is currently in NoClip mode.
     public boolean isNoClipping(UUID playerId) {
@@ -24,9 +42,8 @@ public class NoClipManager {
 
     }
 
-    // Puts the player into NoClip mode. The caller is responsible for the
-    // gamemode: the player must already be in creative or spectator, and must
-    // hold noclip.use. Returns false when either precondition fails.
+    // The caller must put the player in creative before entry. Active phasing
+    // may re-enter from spectator. Requires noclip.use in either case.
     public boolean enterNoClip(Player player) {
 
         // Guard against offline handles passed in by other plugins.
@@ -36,9 +53,11 @@ public class NoClipManager {
 
         }
 
-        // NoClip phases between creative and spectator, so it can only engage
-        // from one of those modes.
-        if (player.getGameMode() != GameMode.CREATIVE && player.getGameMode() != GameMode.SPECTATOR) {
+        // Enter through creative (/gmic) first. Plain spectator must not bypass
+        // creative's inventory and region rules; active phasing is idempotent.
+        if (player.getGameMode() != GameMode.CREATIVE
+                && !(player.getGameMode() == GameMode.SPECTATOR && isNoClipping(player.getUniqueId())))
+        {
 
             return false;
 
@@ -53,6 +72,7 @@ public class NoClipManager {
 
         // Keeps track of all players in NoClip mode. Re-entry is idempotent.
         active.add(player.getUniqueId());
+        player.getPersistentDataContainer().set(intentKey, PersistentDataType.BYTE, (byte) 1);
         return true;
 
     }
@@ -67,7 +87,18 @@ public class NoClipManager {
 
         }
 
-        // State removal only; callers decide about gamemode and teleports.
+        player.getPersistentDataContainer().remove(intentKey);
+        return suspendNoClip(player);
+
+    }
+
+    // Disconnect/shutdown stops phasing without forgetting the toggle. Callers
+    // decide about gamemode and teleports, as with an explicit exit.
+    public boolean suspendNoClip(Player player) {
+
+        if (player == null)
+            return false;
+
         return active.remove(player.getUniqueId());
 
     }

src/test/java/plugin/NoClipManagerTest.java

@@ -0,0 +1,147 @@
+package plugin;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.anyByte;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicReference;
+import org.bukkit.GameMode;
+import org.bukkit.NamespacedKey;
+import org.bukkit.entity.Player;
+import org.bukkit.event.entity.PlayerDeathEvent;
+import org.bukkit.event.player.PlayerGameModeChangeEvent;
+import org.bukkit.event.player.PlayerQuitEvent;
+import org.bukkit.persistence.PersistentDataContainer;
+import org.bukkit.persistence.PersistentDataType;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class NoClipManagerTest {
+
+    private final NamespacedKey key = NamespacedKey.fromString("noclip-og:enabled");
+    private final Player player = mock(Player.class);
+    private final NoClipManager manager = new NoClipManager(key);
+    private final AtomicReference<Byte> saved = new AtomicReference<>();
+
+    @BeforeEach
+    void setUp() {
+
+        when(player.getUniqueId()).thenReturn(UUID.randomUUID());
+        when(player.isOnline()).thenReturn(true);
+        when(player.getGameMode()).thenReturn(GameMode.CREATIVE);
+        when(player.hasPermission("noclip.use")).thenReturn(true);
+        final PersistentDataContainer data = mock(PersistentDataContainer.class);
+        when(player.getPersistentDataContainer()).thenReturn(data);
+        when(data.get(key, PersistentDataType.BYTE)).thenAnswer(invocation -> saved.get());
+        doAnswer(invocation -> {
+
+            saved.set(invocation.getArgument(2));
+            return null;
+
+        }).when(data).set(eq(key), eq(PersistentDataType.BYTE), anyByte());
+        doAnswer(invocation -> {
+
+            saved.set(null);
+            return null;
+
+        }).when(data).remove(key);
+
+    }
+
+    @Test
+    void quitAndReloadRememberToggleWithoutStartingPhasing() {
+
+        assertTrue(manager.enterNoClip(player));
+        final PlayerQuitEvent quit = mock(PlayerQuitEvent.class);
+        when(quit.getPlayer()).thenReturn(player);
+        new Listeners(manager).onPlayerQuit(quit);
+        assertFalse(manager.isNoClipping(player.getUniqueId()));
+        assertTrue(manager.hasNoClipIntent(player));
+
+        final NoClipManager reloaded = new NoClipManager(key);
+        assertTrue(reloaded.hasNoClipIntent(player));
+        assertFalse(reloaded.isNoClipping(player.getUniqueId()));
+        assertTrue(reloaded.enterNoClip(player));
+        assertTrue(reloaded.isNoClipping(player.getUniqueId()));
+
+    }
+
+    @Test
+    void explicitExitForgetsToggleEvenWhenSuspended() {
+
+        manager.enterNoClip(player);
+        manager.suspendNoClip(player);
+        manager.exitNoClip(player);
+        assertFalse(new NoClipManager(key).hasNoClipIntent(player));
+
+    }
+
+    @Test
+    void deathForgetsToggle() {
+
+        manager.enterNoClip(player);
+        final PlayerDeathEvent death = mock(PlayerDeathEvent.class);
+        when(death.getEntity()).thenReturn(player);
+        new Listeners(manager).onPlayerDeath(death);
+        assertFalse(manager.isNoClipping(player.getUniqueId()));
+        assertFalse(manager.hasNoClipIntent(player));
+
+    }
+
+    @Test
+    void creativeSpectatorPhasingRetainsToggleButGmicOffClearsIt() {
+
+        manager.enterNoClip(player);
+        final PlayerGameModeChangeEvent change = mock(PlayerGameModeChangeEvent.class);
+        when(change.getPlayer()).thenReturn(player);
+        when(change.getNewGameMode()).thenReturn(GameMode.SPECTATOR);
+        final Listeners listeners = new Listeners(manager);
+        listeners.onGameModeChange(change);
+        assertTrue(manager.isNoClipping(player.getUniqueId()));
+        assertTrue(manager.hasNoClipIntent(player));
+        when(change.getNewGameMode()).thenReturn(GameMode.SURVIVAL);
+        listeners.onGameModeChange(change);
+        assertFalse(manager.isNoClipping(player.getUniqueId()));
+        assertFalse(manager.hasNoClipIntent(player));
+
+    }
+
+    @Test
+    void loginNormalizationDoesNotEraseSuspendedIntent() {
+
+        manager.enterNoClip(player);
+        manager.suspendNoClip(player);
+        final PlayerGameModeChangeEvent change = mock(PlayerGameModeChangeEvent.class);
+        when(change.getPlayer()).thenReturn(player);
+        when(change.getNewGameMode()).thenReturn(GameMode.SURVIVAL);
+        new Listeners(manager).onGameModeChange(change);
+        assertTrue(manager.hasNoClipIntent(player));
+
+    }
+
+    @Test
+    void entryRequiresCreativeAndPermissionButActivePhasingIsIdempotent() {
+
+        when(player.getGameMode()).thenReturn(GameMode.SPECTATOR);
+        assertFalse(manager.enterNoClip(player));
+        when(player.getGameMode()).thenReturn(GameMode.SURVIVAL);
+        assertFalse(manager.enterNoClip(player));
+        when(player.getGameMode()).thenReturn(GameMode.CREATIVE);
+        when(player.hasPermission("noclip.use")).thenReturn(false);
+        assertFalse(manager.enterNoClip(player));
+        assertFalse(manager.hasNoClipIntent(player));
+        when(player.hasPermission("noclip.use")).thenReturn(true);
+        assertTrue(manager.enterNoClip(player));
+        when(player.getGameMode()).thenReturn(GameMode.SPECTATOR);
+        assertTrue(manager.enterNoClip(player));
+        when(player.isOnline()).thenReturn(false);
+        assertFalse(manager.enterNoClip(player));
+
+    }
+
+}