Fix logic

src/main/java/spawnog/SpawnOG.java

@@ -88,7 +88,7 @@ public final class SpawnOG extends JavaPlugin {
         GamemodeAuthority gamemodeAuthority = GameModeInventoriesAuthority.find(this);
         GamemodePolicy gamemodePolicy = new GamemodePolicy(this, regionLookup(), gamemodeAuthority);
         loginMigrationService = new LoginMigrationService(this, migrationStore, returnLocationStore,
-                flightSnapshotStore, gamemodePolicy, managedWorlds, fallProtection);
+                flightSnapshotStore, gamemodePolicy, gamemodeAuthority, managedWorlds, fallProtection);
         getServer().getPluginManager().registerEvents(new SpawnListener(this, loginMigrationService, managedWorlds),
                 this);
 

src/main/java/spawnog/commands/PendingConfirmations.java

@@ -5,10 +5,8 @@ import java.util.Map;
 import java.util.UUID;
 import java.util.function.LongSupplier;
 
-// The two-step /spawnback state: the first invocation arms a warning, the
-// second within the window executes. Keyed by the return record's token, so a
-// warning issued for one rescue can never confirm a different one recorded in
-// between. Clock injected so the expiry logic is testable off the server.
+// Two-step /spawnback state: first call arms a warning, second in the window
+// executes. Keyed by record token; clock injected so expiry is testable.
 public final class PendingConfirmations {
 
     private final LongSupplier clock;
@@ -20,9 +18,8 @@ public final class PendingConfirmations {
 
     }
 
-    // True when a live warning for exactly this token is on record, consuming
-    // it. False arms (or re-arms) the warning for this token instead: a first
-    // call, an expired window, and a token mismatch all warn anew.
+    // True consumes a live warning for exactly this token. False arms anew: a
+    // first call, an expired window, and a token mismatch all warn again.
     public boolean confirm(UUID playerId, String token, long windowMillis) {
 
         long now = clock.getAsLong();

src/main/java/spawnog/commands/SpawnBackCommand.java

@@ -28,9 +28,8 @@ import spawnog.login.ReturnLocationStore;
 import spawnog.login.ReturnLocationStore.ReturnPoint;
 import spawnog.teleport.FallProtection;
 
-// Sends a player back to wherever a login safety migration took them from. The
-// position is knowingly unsafe, so the first /spawnback only warns and the
-// second one within the window executes; there is no subcommand.
+// Sends a player back to where a login safety migration took them from. The
+// spot is knowingly unsafe: first /spawnback warns, second in the window runs.
 public final class SpawnBackCommand implements CommandExecutor, TabCompleter, Listener {
 
     private static final int DEFAULT_CONFIRM_SECONDS = 30;
@@ -105,17 +104,17 @@ public final class SpawnBackCommand implements CommandExecutor, TabCompleter, Li
                     coordinates(point), Placeholder.unparsed("reason", point.reason()),
                     Placeholder.unparsed("seconds", String.valueOf(confirmSeconds)));
             // A flyer headed for a spot that no longer permits their flight is
-            // told about the drop before they commit to it.
-            if (point.mode() != FlightMode.NONE && !flightRestorer.canRestore(point.mode(), player, point.location()))
+            // warned of the drop; a sanctioned live spectator cannot fall.
+            if (point.mode() != FlightMode.NONE
+                    && !flightRestorer.canResumeInPlace(point.mode(), player, point.location()))
                 send(player, "locale.spawnbackFallWarning",
                         "<gold>You can no longer fly there, so you will fall; a one-time fall protection will cover the landing.</gold>");
             return true;
 
         }
 
-        // A plain return may land in a spot built over since the rescue, so it
-        // is lifted to the nearest clear position above; fall protection below
-        // covers drops. Flight returns keep the exact spot they left mid-air.
+        // A plain return may be built over since the rescue, so it is lifted to
+        // the nearest clear spot above; flight returns keep the exact position.
         Location recorded = point.location().clone();
         Location destination = recorded;
         if (point.mode() == FlightMode.NONE) {
@@ -144,9 +143,10 @@ public final class SpawnBackCommand implements CommandExecutor, TabCompleter, Li
 
                 }
 
-                // Re-checked live at the destination: a permission or region
-                // right lost since the warning means protection, not flight.
-                boolean restored = flightRestorer.restore(point.mode(), player);
+                // Re-checked live at the destination: a right lost since the
+                // warning means protection; a sanctioned spectator is left as is.
+                boolean restored = flightRestorer.canResumeInPlace(point.mode(), player, player.getLocation())
+                        && flightRestorer.resumeInPlace(point.mode(), player);
                 if (!restored) {
 
                     fallProtection.grant(player);

src/main/java/spawnog/flight/FlightIntentStore.java

@@ -9,9 +9,7 @@ import spawnog.SpawnOG;
 import spawnog.login.YamlStore;
 
 // Remembers who has /fly switched on, so the choice survives relogs and
-// restarts instead of dying with the session. Only the intent is stored;
-// whether flight is actually granted is re-decided against the region rules
-// and permissions every time it is applied.
+// restarts. Only intent is stored; each grant is re-judged against the rules.
 public final class FlightIntentStore extends YamlStore {
 
     public FlightIntentStore(SpawnOG plugin) {

src/main/java/spawnog/flight/FlightMode.java

@@ -2,9 +2,8 @@ package spawnog.flight;
 
 import org.bukkit.GameMode;
 
-// The kind of flight a player was using when they left, which decides how
-// /spawnback puts them back into the air. Classified once from the quit
-// snapshot and carried through the return record.
+// The kind of flight a player left with, deciding how /spawnback puts them
+// back into the air. Classified once at quit and carried through the record.
 public enum FlightMode {
 
     // Regional /fly: survival flight granted by RegionFlightService.
@@ -18,9 +17,8 @@ public enum FlightMode {
     // No flight to restore: a grounded rescue or a record predating modes.
     NONE;
 
-    // NoClip wins because /nc drives the gamemode itself. Survival flight only
-    // counts as FLY when /fly was actually toggled; flight without that intent
-    // is NONE, so /spawnback never invents a toggle the player did not make.
+    // NoClip wins because /nc drives the gamemode itself. Survival flight is
+    // FLY only with a real /fly toggle, so a toggle is never invented.
     public static FlightMode classify(GameMode gamemode, boolean noclip, boolean flyIntent) {
 
         if (noclip)

src/main/java/spawnog/flight/FlightQuitListener.java

@@ -9,11 +9,8 @@ import org.bukkit.event.player.PlayerQuitEvent;
 import spawnog.SpawnOG;
 import spawnog.login.NoClipAuthority;
 
-// Captures the flight snapshot on the way out. LOWEST so it reads the state
-// before RegionFlightService grounds the player and before NoClip-OG's MONITOR
-// cleanup erases its own record of them. Kicks funnel into the same quit event,
-// which is also why nothing here (or in the services behind it) may act on
-// PlayerKickEvent: that fires earlier and would corrupt the snapshot.
+// Captures the flight snapshot at LOWEST, before grounding and NoClip cleanup.
+// Kicks funnel into this quit event; a PlayerKickEvent handler would corrupt it.
 public final class FlightQuitListener implements Listener {
 
     private final SpawnOG plugin;
@@ -52,9 +49,8 @@ public final class FlightQuitListener implements Listener {
 
     }
 
-    // Replays the quit capture for everyone still online, so a /reload or a
-    // shutdown that skips quit events records flyers the same way a disconnect
-    // does. Runs before the flight service revokes anything.
+    // Replays the quit capture for everyone online, so a /reload or shutdown
+    // that skips quit events records flyers before the flight service revokes.
     public void captureAll() {
 
         plugin.getServer().getOnlinePlayers().forEach(this::capture);

src/main/java/spawnog/flight/FlightRestorer.java

@@ -8,11 +8,8 @@ import spawnog.login.GamemodeAuthority;
 import spawnog.login.GamemodePolicy;
 import spawnog.login.NoClipAuthority;
 
-// The single decision table for handing a rescued flyer their flight back:
-// /spawnback and the login bypass path both ask it, so the warning, the
-// restore, and the in-place resume can never disagree. Every check is judged
-// at the destination, and a nofly region outranks every mode of flight unless
-// the player holds the regional flight bypass.
+// The single decision table for handing a rescued flyer flight back, shared by
+// /spawnback and login so they never disagree; a nofly region outranks all but bypass.
 public final class FlightRestorer {
 
     public static final String NOCLIP_PERMISSION = "noclip.use";
@@ -38,9 +35,8 @@ public final class FlightRestorer {
 
     }
 
-    // Whether the player's prior mode of flight would be handed back at the
-    // destination. /spawnback asks this before warning, so a player headed for
-    // a drop is told about it up front.
+    // Whether the prior mode of flight would be handed back at the destination.
+    // Asked before warning, so a player headed for a drop is told up front.
     public boolean canRestore(FlightMode mode, Player player, Location destination) {
 
         if (mode == null || player == null || destination == null)
@@ -61,9 +57,7 @@ public final class FlightRestorer {
     }
 
     // Puts the player back into their prior mode of flight where they stand.
-    // Permissions are re-checked live, so a right lost since the warning is a
-    // refusal here rather than an unearned grant. False means the caller owes
-    // the player fall protection.
+    // Rights re-checked live; false means the caller owes fall protection.
     public boolean restore(FlightMode mode, Player player) {
 
         if (player == null || !canRestore(mode, player, player.getLocation()))
@@ -80,9 +74,8 @@ public final class FlightRestorer {
 
     }
 
-    // Like canRestore, except a sanctioned spectator is left alone. Only a
-    // player still IN spectator qualifies: force-gamemode rewrites the mode
-    // between quit and login, and an ex-spectator takes the rescue instead.
+    // Like canRestore, except a sanctioned spectator is left alone. Only a live
+    // spectator qualifies: force-gamemode rewrites the mode between quit and login.
     public boolean canResumeInPlace(FlightMode mode, Player player, Location location) {
 
         if (mode == FlightMode.SPECTATOR)
@@ -102,9 +95,8 @@ public final class FlightRestorer {
 
     }
 
-    // The narrowest flight that covers a descent when no mode can be restored:
-    // a landing loan the flight service revokes on touchdown. For the rescue
-    // paths that leave a player airborne with nothing else holding them up.
+    // The narrowest flight covering a descent when nothing can be restored: a
+    // landing loan the flight service revokes on touchdown.
     public boolean loanFlight(Player player) {
 
         return regionFlightService != null && regionFlightService.resumeFlight(player);
@@ -118,9 +110,8 @@ public final class FlightRestorer {
 
     }
 
-    // Creative flight needs GameModeInventories-OG's sanction for the gamemode
-    // and the region rules' sanction for the flight: gamemodeinventories.anywhere
-    // licenses creative everywhere, but a nofly region still grounds it.
+    // Creative flight needs GMI's sanction for the gamemode and the region
+    // rules' for the flight: anywhere licenses creative, a nofly still grounds it.
     private boolean creativeFlightPermitted(Player player, Location destination) {
 
         if (regionFlightService == null || gamemodeAuthority == null)
@@ -133,9 +124,8 @@ public final class FlightRestorer {
 
     }
 
-    // Creative through the authority so the inventory swap runs. Ability then
-    // state are re-asserted, not trusted: an already-creative player skips the
-    // vanilla ability reset, and setFlying throws while allow-flight is off.
+    // Creative through the authority so the inventory swap runs. Abilities are
+    // re-asserted, not trusted; setFlying throws while allow-flight is off.
     private boolean restoreCreative(Player player) {
 
         if (!gamemodeAuthority.changeGameMode(player, GameMode.CREATIVE))

src/main/java/spawnog/flight/FlightSnapshotStore.java

@@ -11,17 +11,8 @@ import org.bukkit.entity.Player;
 import spawnog.SpawnOG;
 import spawnog.login.YamlStore;
 
-// Remembers everything about a player who left mid-flight: where, in which
-// gamemode, and by which kind of flight. The next login reads this to decide
-// between resuming the flight in place and rescuing the player to spawn, and
-// the /spawnback record is written from it.
-//
-// Last quit wins: a quit while flying writes the record, a grounded quit
-// erases it, so a stale snapshot can never make a later login look airborne.
-//
-// Persisted rather than kept in memory because a shutdown grounds players the
-// same way a disconnect does, and the player may not come back until long
-// after.
+// Persistent record of who left mid-flight: where, gamemode, and kind of
+// flight. Last quit wins: a grounded quit erases any stale airborne record.
 public final class FlightSnapshotStore extends YamlStore {
 
     public FlightSnapshotStore(SpawnOG plugin) {
@@ -36,9 +27,8 @@ public final class FlightSnapshotStore extends YamlStore {
         Location location = player.getLocation();
         data.set(path + ".player-name", player.getName());
         data.set(path + ".recorded-at", Instant.now().toString());
-        // Plain numbers rather than a serialized Location: deserializing one
-        // whose world is unloaded throws, and that would take every other
-        // player's record down with it.
+        // Plain numbers, not a serialized Location: one unloaded world would
+        // throw and take every other player's record down with it.
         data.set(path + ".location.world", location.getWorld() == null ? null : location.getWorld().getName());
         data.set(path + ".location.x", location.getX());
         data.set(path + ".location.y", location.getY());
@@ -114,9 +104,8 @@ public final class FlightSnapshotStore extends YamlStore {
 
     }
 
-    // gamemode and location may be null for older or unloaded-world records;
-    // mode() tolerates both. An absent fly-intent reads false, so a truncated
-    // record classifies as NONE, never as a /fly grant the player lacked.
+    // gamemode and location may be null on older or unloaded-world records. An
+    // absent fly-intent reads false, so truncation classifies as NONE, never FLY.
     public record FlightSnapshot(GameMode gamemode, Location location, boolean allowFlight, boolean flying,
             boolean noclip, boolean flyIntent)
     {

src/main/java/spawnog/flight/RegionFlightService.java

@@ -55,10 +55,8 @@ public final class RegionFlightService implements Listener {
     private final FallProtection fallProtection;
     private final Map<String, RuleKind> regionRules = new HashMap<>();
     private final Map<UUID, FlightOverride> flightOverrides = new HashMap<>();
-    // Players flying on a grant that belongs to no region, handed out so a
-    // /spawnback return does not drop them from the height that got them
-    // rescued. Tracked separately from the region overrides because it is
-    // revoked by landing rather than by leaving a region.
+    // Flyers on a grant belonging to no region, so a /spawnback return does not
+    // drop them. Revoked by landing rather than by leaving a region.
     private final Set<UUID> landingGrants = new HashSet<>();
     private final Set<UUID> scheduledRefreshes = new HashSet<>();
     private int particleStep;
@@ -141,19 +139,15 @@ public final class RegionFlightService implements Listener {
 
     }
 
-    // Puts a player back into the air after a plugin-driven teleport when they
-    // were flying beforehand. False means the player was left airborne without
-    // flight, so the caller can say why. Refuses when the player lacks flight
-    // permission or the destination forbids flight.
+    // Puts a player back into the air after a plugin teleport when they flew
+    // before. False: left airborne without flight, so the caller can say why.
     public boolean resumeFlight(Player player) {
 
         if (player == null || !player.isOnline())
             return false;
 
-        // Reconcile against where the player now stands before granting
-        // anything. The override from the region they were teleported out of is
-        // still in the map, and leaving it there lets the next refresh restore
-        // it and take the flight straight back a tick later.
+        // Reconcile where the player now stands first: the departed region's
+        // override would otherwise be restored and take the flight back a tick later.
         refresh(player);
 
         UUID playerId = player.getUniqueId();
@@ -173,9 +167,8 @@ public final class RegionFlightService implements Listener {
         if (!hasFlightPermission(player))
             return false;
 
-        // Every grant is recorded, so leaving the region, a nofly rule, and
-        // disconnecting can all take it back. An unrecorded one would survive
-        // into the player's saved abilities and become permanent free flight.
+        // Every grant is recorded so region exit, nofly, and disconnect can take
+        // it back; unrecorded it would persist as permanent free flight.
         if (rule != null && rule.kind() == RuleKind.FLY) {
 
             player.setAllowFlight(true);
@@ -205,17 +198,14 @@ public final class RegionFlightService implements Listener {
 
     }
 
-    // Evaluates the rules at an explicit position, because during a move or
-    // teleport event the player entity still reports the pre-move location and
-    // judging there would leave the rules one step behind at region borders.
+    // Judges at an explicit position: during move/teleport events the entity
+    // still reports the pre-move spot, one step behind at region borders.
     public void refresh(Player player, org.bukkit.Location at) {
 
         if (player == null || !player.isOnline() || at == null)
             return;
-        // While a login migration is in flight, do not reschedule: the completion
-        // listener registered in the constructor refreshes once it resolves.
-        // Re-polling here previously spun the scheduler within a single tick and
-        // froze the server, because the migration could only resolve on a later tick.
+        // While a login migration is pending, do not reschedule: its completion
+        // listener refreshes. Re-polling once spun the scheduler and froze the server.
         if (loginMigrationService.isPending(player))
             return;
         if (releaseGamemodeFlyer(player))
@@ -224,10 +214,8 @@ public final class RegionFlightService implements Listener {
         UUID playerId = player.getUniqueId();
         RegionRule rule = ruleAt(at);
 
-        // The landing loan ends once the player is down, and at once inside a
-        // region that denies them flight. Revoked before the branches below read
-        // the ability, so a nofly region records the player's own flight state
-        // as the original rather than the loan and cannot hand it back later.
+        // The loan ends on touchdown or inside a denying region. Revoked before
+        // the branches read abilities, so a nofly never records the loan as owned.
         if (landingGrants.contains(playerId) && (LocationSafety.isSupported(at)
                 || rule != null && rule.kind() == RuleKind.NOFLY && !hasBypass(player)))
             revokeLandingGrant(player);
@@ -315,19 +303,16 @@ public final class RegionFlightService implements Listener {
         if (releaseGamemodeFlyer(player))
             return;
 
-        // The airborne evidence is FlightQuitListener's snapshot, captured at
-        // LOWEST before this runs, so grounding here loses nothing.
-        //
-        // The loan before the override, so a loan is never mistaken for an
-        // ability the player owned and written back into their saved abilities.
+        // The LOWEST-priority quit snapshot already holds the airborne evidence,
+        // so grounding loses nothing. Loan before override: a loan is never saved as
+        // owned.
         revokeLandingGrant(player);
         restoreOverride(player, flightOverrides.get(player.getUniqueId()));
 
     }
 
-    // Creative and spectator flight is gamemode-derived and never regional:
-    // bookkeeping is dropped, not applied, so a stale override can neither
-    // ground a gamemode flyer nor leak a loaned ability back to survival.
+    // Creative and spectator flight is gamemode-derived, never regional:
+    // bookkeeping is dropped, not applied, so overrides neither ground nor leak.
     private boolean releaseGamemodeFlyer(Player player) {
 
         GameMode gamemode = player.getGameMode();
@@ -400,17 +385,15 @@ public final class RegionFlightService implements Listener {
                 && event.getFrom().getBlockX() == event.getTo().getBlockX()
                 && event.getFrom().getBlockY() == event.getTo().getBlockY()
                 && event.getFrom().getBlockZ() == event.getTo().getBlockZ();
-        // Judged where the player is going, not where they still stand.
-        // Teleports never reach this handler (PlayerTeleportEvent has its own
-        // HandlerList); onTeleport's scheduled refresh covers them instead.
+        // Judged where the player is going. Teleports never reach this handler
+        // (own HandlerList); onTeleport's scheduled refresh covers them.
         if (!sameBlock)
             refresh(player, event.getTo());
 
     }
 
-    // No PlayerKickEvent handler on purpose: a kick fires before the quit event
-    // that FlightQuitListener snapshots from, so grounding a kicked player early
-    // would erase the airborne evidence. The quit event always follows.
+    // No PlayerKickEvent handler on purpose: a kick precedes the quit event the
+    // snapshot reads from; grounding early would erase the airborne evidence.
     @EventHandler
     public void onQuit(PlayerQuitEvent event) {
 
@@ -448,9 +431,8 @@ public final class RegionFlightService implements Listener {
 
     }
 
-    // Grants regional flight to a player standing inside a fly region they hold
-    // permission for, recording the grant and the intent so it is revoked and
-    // re-armed like any /fly toggle. The restore path for a rescued /fly flyer.
+    // Grants regional flight inside a permitted fly region, recording grant and
+    // intent like a /fly toggle. The restore path for a rescued /fly flyer.
     public boolean grantRegionFlight(Player player) {
 
         if (player == null || !player.isOnline() || !hasFlightPermission(player))
@@ -482,17 +464,15 @@ public final class RegionFlightService implements Listener {
 
     }
 
-    // Re-grants flight inside a fly region for a player whose stored intent says
-    // /fly was left on. The live ability is dropped on quit (and by login
-    // normalization), so without this the toggle would not survive a relog.
+    // Re-grants flight in a fly region when stored intent says /fly was on; the
+    // live ability dies on quit, so the toggle would not survive a relog.
     private void rearmPersistedFlight(Player player, RegionRule rule) {
 
         if (!flightIntentStore.contains(player.getUniqueId()) || !hasFlightPermission(player))
             return;
 
-        // Another authority (creative mode, a bypass, another plugin) already
-        // allows flight, so there is no override to track; but an airborne
-        // player still needs the flying flag itself put back.
+        // Another authority already allows flight, so there is no override to
+        // track; an airborne player still needs the flying flag put back.
         if (!player.getAllowFlight()) {
 
             player.setAllowFlight(true);
@@ -597,9 +577,8 @@ public final class RegionFlightService implements Listener {
         UUID playerId = player.getUniqueId();
         if (!scheduledRefreshes.add(playerId))
             return;
-        // Paper's scheduler executes zero-delay tasks scheduled from within a task
-        // in the same tick, so a one-tick delay is required to guarantee the
-        // current tick always finishes before the refresh runs.
+        // Paper runs zero-delay tasks scheduled from within a task in the same
+        // tick, so a one-tick delay guarantees the current tick finishes first.
         plugin.getServer().getScheduler().runTaskLater(plugin, () -> {
 
             scheduledRefreshes.remove(playerId);

src/main/java/spawnog/integration/MyWorldsSpawnBridge.java

@@ -9,11 +9,8 @@ import org.bukkit.event.Listener;
 import org.bukkit.event.world.WorldLoadEvent;
 import org.bukkit.plugin.java.JavaPlugin;
 
-// Pushes Spawn-OG's global spawn into MyWorlds so both agree on where world
-// spawn is. Without this, MyWorlds stamps its own worlds.yml spawn onto the world
-// on every load and anything that teleports to World#getSpawnLocation (new
-// players, /world tp, minigame teardown fallbacks) ignores /setspawn.
-// Reflection keeps MyWorlds a soft dependency: My_Worlds is the TrueOG fork name.
+// Pushes the global spawn into MyWorlds so World#getSpawnLocation matches
+// /setspawn. Reflection keeps it soft; My_Worlds is the TrueOG fork name.
 public final class MyWorldsSpawnBridge implements Listener {
 
     private static final String[] PLUGIN_NAMES = { "MyWorlds", "My_Worlds" };

src/main/java/spawnog/listener/SpawnListener.java

@@ -72,9 +72,8 @@ public class SpawnListener implements Listener {
         Player p = e.getPlayer();
         Location destination = preferred(p, e);
 
-        // Bed, home, and configured spawn all keep their say, but only inside
-        // the worlds Spawn-OG governs. A bed left in a minigame world, or a
-        // death in one, would otherwise respawn the player outside the SMP.
+        // Bed, home, and configured spawn keep their say only inside governed
+        // worlds; a minigame bed or death must not respawn the player outside the SMP.
         if (!managedWorlds.contains(destination == null ? null : destination.getWorld()))
             destination = lastManagedSpawn(p);
 
@@ -83,9 +82,8 @@ public class SpawnListener implements Listener {
 
     }
 
-    // Where the player would land without the managed-world guarantee: their bed
-    // if the server found one usable, then their Essentials home, then the
-    // configured spawn.
+    // Without the managed-world guarantee: usable bed first, then Essentials
+    // home, then the configured spawn.
     private Location preferred(Player p, PlayerRespawnEvent e) {
 
         boolean atHome = plugin.getConfig().getBoolean("respawn-at-home", true);

src/main/java/spawnog/login/GameModeInventoriesAuthority.java

@@ -8,9 +8,8 @@ import org.bukkit.plugin.RegisteredServiceProvider;
 
 import spawnog.SpawnOG;
 
-// GameModeInventories-OG's published policy, taken from the services manager it
-// registers on enable. Only constructed when that plugin is enabled, so its
-// classes are never resolved on servers running without it.
+// GameModeInventories-OG's published policy from the services manager. Only
+// constructed when that plugin is enabled, so its classes never resolve without it.
 public final class GameModeInventoriesAuthority implements GamemodeAuthority {
 
     public static final String PLUGIN_NAME = "GameModeInventories-OG";

src/main/java/spawnog/login/GamemodeAuthority.java

@@ -8,16 +8,12 @@ import org.bukkit.entity.Player;
 // that rule when it is installed; Spawn-OG's own rules stand in when it is not.
 public interface GamemodeAuthority {
 
-    // Whether the player may hold the gamemode at the location, which is not
-    // always where they stand: login safety asks about the destination it is
-    // about to move them to. Null when the authority cannot answer right now,
-    // so the caller falls back.
+    // Whether the player may hold the gamemode at the location (often a rescue
+    // destination, not where they stand). Null when the authority cannot answer.
     Boolean mayUse(Player player, GameMode gamemode, Location location);
 
-    // Switches the player's gamemode through the authority's own machinery, so
-    // its side effects (inventory swaps, region-listener exemptions) apply.
-    // False when no authority is running or the switch was cancelled; Spawn-OG
-    // never sets a sanctioned gamemode behind the authority's back.
+    // Switches gamemode through the authority so its side effects (inventory
+    // swaps, exemptions) apply; false when absent or the switch was cancelled.
     default boolean changeGameMode(Player player, GameMode gamemode) {
 
         return false;

src/main/java/spawnog/login/GamemodePolicy.java

@@ -12,13 +12,8 @@ import org.bukkit.entity.Player;
 import spawnog.SpawnOG;
 import spawnog.region.RegionLookup;
 
-// Decides who may log in outside survival. Everyone else is returned to
-// survival, because a non-survival login in an SMP world is either a leftover
-// autopsy state or an escape from survival rules.
-//
-// GameModeInventories-OG owns that rule at runtime, so this defers to the
-// authority it publishes rather than keeping a second copy that can drift. The
-// local rules below stand in on servers running Spawn-OG without it.
+// Decides who may log in outside survival. GameModeInventories-OG owns the
+// rule at runtime and is asked first; the local rules below stand in without it.
 public final class GamemodePolicy {
 
     private static final String ANYWHERE_PERMISSION = "gamemodeinventories.anywhere";
@@ -43,9 +38,8 @@ public final class GamemodePolicy {
 
     }
 
-    // True when the player is allowed to hold the gamemode they logged in with
-    // at the location. Judged against wherever they will actually end up, so a
-    // rescue teleport asks about its destination rather than the login spot.
+    // True when the player may hold the gamemode at the location, judged where
+    // they will actually end up: a rescue asks about its destination.
     public boolean mayUse(Player player, GameMode gamemode, Location location) {
 
         if (gamemode == GameMode.SURVIVAL)

src/main/java/spawnog/login/LocationSafety.java

@@ -9,10 +9,8 @@ import org.bukkit.World;
 import org.bukkit.block.Block;
 import org.bukkit.block.BlockFace;
 
-// Decides whether a login location would hurt or kill the player who logs in
-// there. The bar is deliberately low: only positions that damage the player on
-// arrival count as unsafe. Standing in water, tall grass, on a slab, or on any
-// other block that is not a full cube is normal survival play and stays safe.
+// Judges whether a login spot would hurt the player on arrival. The bar is low
+// on purpose: water, grass, slabs, and other partial blocks all stay safe.
 public final class LocationSafety {
 
     // A drop this long or shorter is survivable, so a player suspended above one is
@@ -64,9 +62,8 @@ public final class LocationSafety {
 
     }
 
-    // Whether something underfoot will hold the player: ground, a block they are
-    // standing on, or liquid. Tells a finished landing from a fall still in
-    // progress, which is when a granted fall protection is no longer owed.
+    // Whether something underfoot holds the player (ground or liquid), telling
+    // a finished landing, when protection is no longer owed, from a live fall.
     public static boolean isSupported(Location feet) {
 
         if (feet == null)
@@ -83,9 +80,8 @@ public final class LocationSafety {
 
     }
 
-    // The lowest position at or above the given one whose feet and head blocks
-    // neither suffocate nor burn on arrival. Landing is not judged: the caller
-    // covers falls. Null when the world is missing or the column is blocked.
+    // The lowest position at or above the given one that neither suffocates nor
+    // burns. Landing is not judged (caller covers falls); null when blocked.
     public static Location clearAbove(Location location) {
 
         if (location == null)

src/main/java/spawnog/login/LoginMigrationService.java

@@ -41,19 +41,22 @@ public final class LoginMigrationService {
     private final ReturnLocationStore returnLocationStore;
     private final FlightSnapshotStore flightSnapshotStore;
     private final GamemodePolicy gamemodePolicy;
+    // Null without GameModeInventories-OG; creative is then never promoted back
+    // after a rescue, because only its machinery swaps inventories safely.
+    private final GamemodeAuthority gamemodeAuthority;
     private final ManagedWorlds managedWorlds;
     private final FallProtection fallProtection;
     private final MiniMessage miniMessage = MiniMessage.miniMessage();
     private final Map<UUID, PendingLogin> pendingLogins = new HashMap<>();
     private final List<Consumer<Player>> completionListeners = new ArrayList<>();
-    // Decides whether and how a rescued flyer gets their flight back. Set after
-    // construction because it is built around the flight service, which takes
-    // this service in its own constructor.
+    // Decides how a rescued flyer gets flight back. Set after construction
+    // because it is built around the flight service, which takes this service.
     private FlightRestorer flightRestorer;
 
     public LoginMigrationService(SpawnOG plugin, AutopsyMigrationStore migrationStore,
             ReturnLocationStore returnLocationStore, FlightSnapshotStore flightSnapshotStore,
-            GamemodePolicy gamemodePolicy, ManagedWorlds managedWorlds, FallProtection fallProtection)
+            GamemodePolicy gamemodePolicy, GamemodeAuthority gamemodeAuthority, ManagedWorlds managedWorlds,
+            FallProtection fallProtection)
     {
 
         this.plugin = plugin;
@@ -61,6 +64,7 @@ public final class LoginMigrationService {
         this.returnLocationStore = returnLocationStore;
         this.flightSnapshotStore = flightSnapshotStore;
         this.gamemodePolicy = gamemodePolicy;
+        this.gamemodeAuthority = gamemodeAuthority;
         this.managedWorlds = managedWorlds;
         this.fallProtection = fallProtection;
 
@@ -71,9 +75,8 @@ public final class LoginMigrationService {
         if (!plugin.getConfig().getBoolean("login-safety.enabled", true))
             return false;
 
-        // A snapshot is only believed when the login still looks like the quit
-        // it recorded; a mismatch, a minigame arena, or an unmanaged world
-        // discards it on the spot so it can never ambush a later login.
+        // A snapshot is believed only while the login still looks like the quit:
+        // a mismatch, minigame arena, or unmanaged world discards it on the spot.
         FlightSnapshot snapshot = flightSnapshotStore.peek(player.getUniqueId());
         if (snapshot != null) {
 
@@ -93,9 +96,8 @@ public final class LoginMigrationService {
 
     }
 
-    // A relog in flight: either the player still has the right to that flight
-    // where they hang and holds the bypass, in which case it is resumed on the
-    // spot, or they are pulled to spawn with /spawnback as the way back.
+    // A relog in flight: resumed on the spot when still permitted there and the
+    // bypass is held, otherwise a pull to spawn with /spawnback as the way back.
     private boolean handleAirborneLogin(Player player, Location plannedDestination, FlightSnapshot snapshot) {
 
         FlightMode mode = snapshot.mode();
@@ -134,14 +136,12 @@ public final class LoginMigrationService {
 
         }
 
-        // Corroboration has already pinned the snapshot to this login: the
-        // recorded spot is where the player hangs right now, and the recorded
-        // gamemode is the one they carry.
+        // Corroboration pinned the snapshot to this login: the recorded spot and
+        // gamemode are the ones the player carries right now.
         GameMode originalGamemode = snapshot.gamemode();
         Location origin = snapshot.location().clone();
-        // Judged at the rescue destination, same as a grounded login: a
-        // creative flyer rescued into the spawn creative region keeps creative
-        // instead of being dropped to survival.
+        // Judged at the rescue destination, same as a grounded login: a creative
+        // flyer rescued into the spawn creative region keeps creative.
         boolean normalizeGamemode = !gamemodePolicy.mayUse(player, originalGamemode, destination)
                 && plugin.getConfig().getBoolean("login-safety.normalize-non-staff-gamemode", true);
         beginTransaction(player, destination, new PendingLogin(player.isInvulnerable(), snapshot.allowFlight(), true,
@@ -160,12 +160,8 @@ public final class LoginMigrationService {
         boolean mayKeepGamemode = gamemodePolicy.mayUse(player, originalGamemode, player.getLocation());
         boolean normalizeGamemode = managedWorld && !mayKeepGamemode
                 && plugin.getConfig().getBoolean("login-safety.normalize-non-staff-gamemode", true);
-        // A sanctioned spectator is working, not an abandoned autopsy state.
-        // A minigame puts its own players into spectator legitimately, so a disconnect
-        // from an arena looks exactly
-        // like the abandoned season 1 state this migration exists to rescue. Migrating
-        // one of those would burn the
-        // player's one-shot flag and record the arena as their /spawnback location.
+        // Minigames put players into spectator legitimately, so an arena
+        // disconnect must not burn the one-shot flag or record the arena.
         boolean minigameArena = MinigameArenas.contains(player);
         boolean autopsyMigration = managedWorld && !staff && !mayKeepGamemode && !minigameArena
                 && originalGamemode == GameMode.SPECTATOR
@@ -182,8 +178,7 @@ public final class LoginMigrationService {
             destination = globalSpawn();
 
         // The player ends up at the destination, so their right to the login
-        // gamemode is judged there too: a creative login rescued into the spawn
-        // creative region keeps creative instead of being dropped to survival.
+        // gamemode is judged there too: permitted there means no normalization.
         if (destination != null && !mayKeepGamemode && gamemodePolicy.mayUse(player, originalGamemode, destination)) {
 
             normalizeGamemode = false;
@@ -347,6 +342,10 @@ public final class LoginMigrationService {
 
         }
 
+        // Force-gamemode demotes non-survival quitters before login, so skipping
+        // normalization is not enough: the gamemode must be actively handed back.
+        promoteGamemode(player, pending);
+
         if (pending.snapshot() != null) {
 
             Location from = pending.originalLocation();
@@ -405,9 +404,37 @@ public final class LoginMigrationService {
 
     }
 
-    // Writes the /spawnback record and offers it. False means the player was
-    // told the way back could not be saved, and the caller must not spend
-    // whatever evidence would let a later login try again.
+    // Re-promotes a rescued player to the gamemode they left with, judged live
+    // where they stand. Creative only via the authority (inventory swap).
+    private void promoteGamemode(Player player, PendingLogin pending) {
+
+        GameMode original = pending.originalGamemode();
+        if (original != GameMode.CREATIVE && original != GameMode.SPECTATOR)
+            return;
+        if (player.getGameMode() == original || pending.normalizeGamemode() || pending.autopsyMigration())
+            return;
+        if (!gamemodePolicy.mayUse(player, original, player.getLocation()))
+            return;
+
+        boolean promoted;
+        if (gamemodeAuthority != null)
+            promoted = gamemodeAuthority.changeGameMode(player, original);
+        else if (original == GameMode.SPECTATOR) {
+
+            player.setGameMode(original);
+            promoted = player.getGameMode() == original;
+
+        } else
+            promoted = false;
+
+        if (!promoted)
+            plugin.getLogger().warning("Unable to return " + player.getName() + " to " + original.name()
+                    + " after their rescue; they remain in " + player.getGameMode().name() + ".");
+
+    }
+
+    // Writes and offers the /spawnback record. False: the player was told it
+    // could not be saved; the caller must keep the evidence for a later retry.
     private boolean offerReturn(Player player, PendingLogin pending) {
 
         String reason;
@@ -436,10 +463,8 @@ public final class LoginMigrationService {
 
     }
 
-    // The failure mode when a rescue cannot run or finish: the player keeps
-    // their gamemode and place, gets the narrowest protection that stops the
-    // situation from hurting them, and everything expires on its own. Never
-    // parks anyone in spectator, because nothing would ever take them out.
+    // Failure mode when a rescue cannot finish: keep gamemode and place, grant
+    // the narrowest self-expiring protection, never park anyone in spectator.
     private void protectInPlace(Player player, FlightSnapshot snapshot, boolean originalInvulnerable) {
 
         // A flyer is given back the flight they already had, as a loan the
@@ -479,9 +504,8 @@ public final class LoginMigrationService {
 
     }
 
-    // Same spot and still off the ground, or the snapshot is stale. Gamemode
-    // equality is NOT required (force-gamemode rewrites quitters to survival),
-    // and live flight bits alone are not either (/fly is revoked on quit).
+    // Same spot and still off the ground, or stale. Gamemode equality is NOT
+    // required (force-gamemode), nor live flight bits (/fly is revoked on quit).
     private boolean corroborates(Player player, FlightSnapshot snapshot) {
 
         Location recorded = snapshot.location();

src/main/java/spawnog/login/MinigameArenas.java

@@ -7,15 +7,8 @@ import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 import org.bukkit.plugin.Plugin;
 
-// Reports whether a player is inside a minigame arena.
-//
-// Autopsy migration exists for OG:SMP season 1 players who were left in spectator mode on the SMP. A minigame puts
-// its own players into spectator legitimately -- Spleef does it to everyone it eliminates, and Splegg runs whole
-// matches in worlds it owns -- so without this check a routine minigame disconnect looks identical to the state the
-// migration is meant to rescue. Migrating one of those players burns their one-shot migration flag and records the
-// arena as their /spawnback location.
-//
-// Every lookup is reflective and optional: no minigame is a dependency, and an absent one simply answers "no".
+// Reports whether a player is inside a minigame arena. Minigames use spectator
+// legitimately; without this an arena disconnect looks like an abandoned autopsy.
 public final class MinigameArenas {
 
     private static final String SPLEEF_PLUGIN = "Spleef-OG";
@@ -45,10 +38,8 @@ public final class MinigameArenas {
             if (Boolean.TRUE.equals(api.getMethod("isInSpleef", Player.class).invoke(null, player)))
                 return true;
 
-            // Survives a crash, where the live session is gone but the saved pre-match
-            // state is not. That is
-            // exactly the login on which a minigame casualty would otherwise be mistaken
-            // for an SMP one.
+            // Survives a crash: the live session is gone but the saved pre-match
+            // state is not, exactly when a casualty would be mistaken for SMP.
             Method pending = api.getMethod("hasPendingRecovery", java.util.UUID.class);
             return Boolean.TRUE.equals(pending.invoke(null, player.getUniqueId()));
 

src/main/java/spawnog/login/NoClipAuthority.java

@@ -7,9 +7,8 @@ import org.bukkit.entity.Player;
 import plugin.NoClip;
 import spawnog.SpawnOG;
 
-// NoClip-OG's manager, reached as a direct compile-time dependency on the
-// submodule under libs/. Only constructed when that plugin is enabled, so its
-// classes are never resolved on servers running without it.
+// NoClip-OG's manager, a compile-time dependency on the submodule under libs/.
+// Only constructed when enabled, so its classes never resolve without it.
 public final class NoClipAuthority {
 
     public static final String PLUGIN_NAME = "NoClip-OG";

src/main/java/spawnog/login/ReturnLocationStore.java

@@ -10,15 +10,8 @@ import org.bukkit.World;
 import spawnog.SpawnOG;
 import spawnog.flight.FlightMode;
 
-// Remembers where a player stood before Spawn-OG pulled them to spawn, so they
-// can walk it back with /spawnback even though the position was flagged unsafe.
-// Persisted, because players usually read the migration message a session or
-// two later.
-//
-// <p>
-// Coordinates are stored as plain numbers rather than serialized Locations:
-// deserializing a Location whose world is unloaded throws, and that would take
-// every other player's record down with it.
+// Remembers where a player stood before a rescue so /spawnback can walk it
+// back. Plain numbers, not Locations: an unloaded world would throw on load.
 public final class ReturnLocationStore extends YamlStore {
 
     // Token for records written before tokens existed, so their pending
@@ -51,9 +44,8 @@ public final class ReturnLocationStore extends YamlStore {
         data.set(path + ".location.z", location.getZ());
         data.set(path + ".location.yaw", location.getYaw());
         data.set(path + ".location.pitch", location.getPitch());
-        // The mode of flight decides what /spawnback hands back; the raw
-        // gamemode and ability are kept alongside so staff reading this file
-        // can see what a player actually lost.
+        // The mode decides what /spawnback hands back; raw gamemode and ability
+        // are kept alongside so staff can see what a player actually lost.
         data.set(path + ".mode", mode == null ? FlightMode.NONE.name() : mode.name());
         data.set(path + ".gamemode", gamemode == null ? null : gamemode.name());
         data.set(path + ".allow-flight", allowFlight);
@@ -160,9 +152,8 @@ public final class ReturnLocationStore extends YamlStore {
 
     }
 
-    // mode is what /spawnback restores; gamemode and allowFlight are the raw
-    // pre-migration state, kept for diagnosis. token identifies this record so
-    // a warning and its confirmation are known to speak about the same rescue.
+    // mode is what /spawnback restores; gamemode/allowFlight are diagnostic.
+    // token ties a warning and its confirmation to the same rescue.
     public record ReturnPoint(Location location, String reason, String worldName, GameMode gamemode,
             boolean allowFlight, boolean flying, FlightMode mode, String token)
     {

src/main/java/spawnog/teleport/FallProtection.java

@@ -15,11 +15,8 @@ import org.bukkit.event.player.PlayerQuitEvent;
 
 import spawnog.login.LocationSafety;
 
-// Cancels the next fall for a player Spawn-OG put in the air: a /spawnback
-// return, or a landing forced by taking flight away. Deliberately kept apart
-// from regional flight so the protection does not depend on the player's
-// permissions, on the rules of wherever they land, or on WorldGuard being
-// installed at all.
+// Cancels the next fall for a player Spawn-OG put in the air. Kept apart from
+// regional flight so it works without permissions, regions, or WorldGuard.
 public final class FallProtection implements Listener {
 
     private final Set<UUID> immune = new HashSet<>();
@@ -71,9 +68,8 @@ public final class FallProtection implements Listener {
 
     }
 
-    // MONITOR so this runs after the flight service's NORMAL quit handler,
-    // which re-grants immunity while grounding flyers; clearing first would
-    // leak the entry and gift a free fall on the player's next session.
+    // MONITOR: runs after the flight service's NORMAL quit handler, which
+    // re-grants immunity while grounding flyers; clearing first would leak it.
     @EventHandler(priority = EventPriority.MONITOR)
     public void onQuit(PlayerQuitEvent event) {
 

src/main/java/spawnog/teleport/SpawnWarmupService.java

@@ -19,10 +19,8 @@ import org.bukkit.scheduler.BukkitTask;
 
 import spawnog.SpawnOG;
 
-// Owns the countdown between /spawn and the teleport that follows it. Moving
-// out of the block the countdown started in cancels the teleport; taking damage
-// does not, so a player being hit while standing still still gets pulled to
-// spawn.
+// Owns the countdown between /spawn and its teleport. Leaving the starting
+// block cancels it; damage does not, so a player under attack still gets pulled.
 public final class SpawnWarmupService implements Listener {
 
     public static final int WARMUP_SECONDS = 5;
@@ -51,9 +49,8 @@ public final class SpawnWarmupService implements Listener {
 
         BukkitTask task = plugin.getServer().getScheduler().runTaskLater(plugin, () -> {
 
-            // Drop the pending entry before teleporting: PlayerTeleportEvent extends
-            // PlayerMoveEvent, so leaving it in place would make the teleport cancel
-            // itself the moment it fires.
+            // Drop the pending entry first: PlayerTeleportEvent extends
+            // PlayerMoveEvent, so the teleport would otherwise cancel itself.
             pendingTeleports.remove(playerId);
             if (player.isOnline())
                 player.teleportAsync(destination);

src/main/java/spawnog/world/ManagedWorlds.java

@@ -14,10 +14,8 @@ import org.bukkit.event.player.PlayerJoinEvent;
 import org.bukkit.event.player.PlayerQuitEvent;
 import org.bukkit.plugin.java.JavaPlugin;
 
-// The SMP dimensions Spawn-OG governs, read from login-safety.worlds. Login
-// safety normalizes logins in them and respawns are kept inside them, so both
-// consult one list. Also remembers which of them each player was in last,
-// because a respawn that would land outside them has to pick one to come back to.
+// The SMP dimensions Spawn-OG governs (login-safety.worlds): one list for both
+// login normalization and respawn routing; remembers each player's last one.
 public final class ManagedWorlds implements Listener {
 
     private static final List<String> DEFAULTS = List.of("world", "world_nether", "world_the_end");