Fix inconsistencies with joining state

core/src/main/java/tc/oc/pgm/compass/CompassModule.java

@@ -74,7 +74,7 @@ public static class Factory implements MapModuleFactory<CompassModule> {
         }
       }
       ImmutableList<CompassTarget<?>> targets = compassTargets.build();
-      if (targets == null) return null;
+      if (targets.isEmpty()) return null;
       return new CompassModule(targets, orderStrategy, showDistance);
     }
   }

core/src/main/java/tc/oc/pgm/spawns/SpawnMatchModule.java

@@ -216,7 +216,7 @@ public void onPartyChange(final PlayerPartyChangeEvent event) {
     if (event.getOldParty() == null) {
       // Join match
       if (event.getNewParty().isParticipating()) {
-        transition(player, null, new Joining(this, player, getJoinPenalty(event)));
+        transition(player, null, new Joining(this, player, getJoinPenalty(event), false));
       } else {
         transition(player, null, new Observing(this, player, true, true));
       }

core/src/main/java/tc/oc/pgm/spawns/states/Alive.java

@@ -111,7 +111,7 @@ public void onEvent(PlayerJoinPartyEvent event) {
     super.onEvent(event);
 
     if (event.getNewParty() instanceof Competitor) {
-      transition(new Joining(smm, player, smm.getJoinPenalty(event)));
+      transition(new Joining(smm, player, smm.getJoinPenalty(event), true));
     } else {
       transition(new Observing(smm, player, true, true));
     }

core/src/main/java/tc/oc/pgm/spawns/states/Dead.java

@@ -37,6 +37,9 @@ public class Dead extends Spawning {
 
   public Dead(SpawnMatchModule smm, MatchPlayer player) {
     super(smm, player, player.getMatch().getTick().tick, 0);
+
+    // Allow stuff like /tp or /j
+    if (options.spectate) this.permission = new StatePermissions.Observer();
   }
 
   @Override
@@ -108,7 +111,10 @@ private void endRotting() {
   @Override
   public void onEvent(PlayerJoinPartyEvent event) {
     super.onEvent(event);
-    if (!(event.getNewParty() instanceof Competitor)) {
+
+    if (event.getNewParty() instanceof Competitor) {
+      transition(new Joining(smm, player, smm.getJoinPenalty(event), true));
+    } else {
       transition(new Observing(smm, player, true, false));
     }
   }

core/src/main/java/tc/oc/pgm/spawns/states/Joining.java

@@ -11,20 +11,21 @@
 /** Player is waiting to spawn after joining a team */
 public class Joining extends Spawning {
 
-  public Joining(SpawnMatchModule smm, MatchPlayer player) {
-    this(smm, player, 0);
-  }
+  private final boolean reset;
 
-  public Joining(SpawnMatchModule smm, MatchPlayer player, long minSpawnTick) {
+  public Joining(SpawnMatchModule smm, MatchPlayer player, long minSpawnTick, boolean reset) {
     super(smm, player, smm.getDeathTick(player), minSpawnTick);
     this.spawnRequested = true;
+    this.reset = reset;
+    this.permission = new StatePermissions.Observer();
   }
 
   @Override
   public void enterState() {
     player.setVisible(false);
-
     super.enterState();
+
+    if (reset) Observing.resetPlayer(smm, player, true, false);
   }
 
   @Override

core/src/main/java/tc/oc/pgm/spawns/states/Observing.java

@@ -1,23 +1,18 @@
 package tc.oc.pgm.spawns.states;
 
 import java.util.EnumSet;
-import java.util.List;
 import java.util.Set;
 import org.bukkit.GameMode;
 import org.bukkit.Location;
 import org.bukkit.Material;
 import org.bukkit.block.Block;
-import org.bukkit.event.Event;
 import org.bukkit.event.entity.EntityDamageEvent;
 import org.bukkit.event.inventory.InventoryClickEvent;
 import org.bukkit.event.inventory.InventoryType;
 import org.bukkit.event.player.PlayerRespawnEvent;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 import org.bukkit.material.Door;
-import org.bukkit.permissions.PermissionAttachment;
-import tc.oc.pgm.api.Config;
-import tc.oc.pgm.api.PGM;
 import tc.oc.pgm.api.match.event.MatchStartEvent;
 import tc.oc.pgm.api.party.Competitor;
 import tc.oc.pgm.api.player.MatchPlayer;
@@ -40,27 +35,28 @@ public class Observing extends State {
 
   private final boolean reset;
   private final boolean teleport;
-  private PermissionAttachment permissionAttachment;
 
   public Observing(SpawnMatchModule smm, MatchPlayer player, boolean reset, boolean teleport) {
     super(smm, player);
     this.reset = reset;
     this.teleport = teleport;
+    this.permission = new StatePermissions.Observer();
   }
 
   @Override
   public void enterState() {
     super.enterState();
+    player.setDead(false);
+    resetPlayer(smm, player, reset, teleport);
+    player.setVisible(true);
+    player.resetVisibility();
+  }
 
-    permissionAttachment = bukkit.addAttachment(PGM.get());
-    for (Config.Group group : PGM.get().getConfiguration().getGroups()) {
-      if (bukkit.hasPermission(group.getPermission())) {
-        permissionAttachment.setPermission(group.getObserverPermission(), true);
-      }
-    }
-
+  // Made static to allow for reuse
+  public static void resetPlayer(
+      SpawnMatchModule smm, MatchPlayer player, boolean reset, boolean teleport) {
+    var bukkit = player.getBukkit();
     if (reset) player.reset();
-    player.setDead(false);
     player.resetInteraction();
     bukkit.setGameMode(GameMode.CREATIVE);
     bukkit.setAllowFlight(true);
@@ -94,8 +90,6 @@ public void enterState() {
     }
 
     player.getBukkit().updateInventory();
-    player.setVisible(true);
-    player.resetVisibility();
 
     // The player is not standing on anything, turn their flying on
     if (bukkit.getAllowFlight()) {
@@ -106,24 +100,18 @@ public void enterState() {
     }
   }
 
-  @Override
-  public void leaveState(List<Event> events) {
-    super.leaveState(events);
-    if (permissionAttachment != null) bukkit.removeAttachment(permissionAttachment);
-  }
-
   @Override
   public void onEvent(MatchStartEvent event) {
     super.onEvent(event);
     if (player.isParticipating()) {
-      transition(new Joining(smm, player));
+      transition(new Joining(smm, player, 0, false));
     }
   }
 
   @Override
   public void onEvent(PlayerJoinPartyEvent event) {
     if (event.getNewParty() instanceof Competitor && event.getMatch().isRunning()) {
-      transition(new Joining(smm, player, smm.getJoinPenalty(event)));
+      transition(new Joining(smm, player, smm.getJoinPenalty(event), false));
     }
   }
 

core/src/main/java/tc/oc/pgm/spawns/states/Participating.java

@@ -1,37 +1,14 @@
 package tc.oc.pgm.spawns.states;
 
-import java.util.List;
-import org.bukkit.event.Event;
-import org.bukkit.permissions.PermissionAttachment;
-import tc.oc.pgm.api.Config;
-import tc.oc.pgm.api.PGM;
 import tc.oc.pgm.api.match.event.MatchFinishEvent;
 import tc.oc.pgm.api.player.MatchPlayer;
 import tc.oc.pgm.spawns.SpawnMatchModule;
 
 public class Participating extends State {
 
-  private PermissionAttachment permissionAttachment;
-
   public Participating(SpawnMatchModule smm, MatchPlayer player) {
     super(smm, player);
-  }
-
-  @Override
-  public void enterState() {
-    super.enterState();
-    permissionAttachment = bukkit.addAttachment(PGM.get());
-    for (Config.Group group : PGM.get().getConfiguration().getGroups()) {
-      if (bukkit.hasPermission(group.getPermission())) {
-        permissionAttachment.setPermission(group.getParticipantPermission(), true);
-      }
-    }
-  }
-
-  @Override
-  public void leaveState(List<Event> events) {
-    if (permissionAttachment != null) bukkit.removeAttachment(permissionAttachment);
-    super.leaveState(events);
+    this.permission = new StatePermissions.Participant();
   }
 
   @Override

core/src/main/java/tc/oc/pgm/spawns/states/State.java

@@ -22,6 +22,8 @@ public abstract class State {
   protected final MatchPlayer player;
   protected final Player bukkit;
 
+  protected StatePermissions permission = null;
+
   private boolean entered, exited;
 
   public State(SpawnMatchModule smm, MatchPlayer player) {
@@ -41,6 +43,8 @@ public void enterState() {
       throw new IllegalStateException("Tried to enter already entered state " + this);
     }
     entered = true;
+
+    if (permission != null) permission.givePermission(player);
   }
 
   /**
@@ -50,6 +54,8 @@ public void enterState() {
    *     nested within each other.
    */
   public void leaveState(List<Event> events) {
+    if (permission != null) permission.revokePermission(player);
+
     if (!entered) {
       throw new IllegalStateException("Tried to leave state before entering " + this);
     } else if (exited) {

core/src/main/java/tc/oc/pgm/spawns/states/StatePermissions.java

@@ -0,0 +1,44 @@
+package tc.oc.pgm.spawns.states;
+
+import java.util.function.Function;
+import org.bukkit.permissions.Permission;
+import org.bukkit.permissions.PermissionAttachment;
+import tc.oc.pgm.api.Config;
+import tc.oc.pgm.api.PGM;
+import tc.oc.pgm.api.player.MatchPlayer;
+
+public class StatePermissions {
+
+  private final Function<Config.Group, Permission> groupFunction;
+  private PermissionAttachment permissionAttachment;
+
+  private StatePermissions(Function<Config.Group, Permission> groupFunction) {
+    this.groupFunction = groupFunction;
+  }
+
+  public void givePermission(MatchPlayer player) {
+    var bukkit = player.getBukkit();
+    permissionAttachment = bukkit.addAttachment(PGM.get());
+    for (Config.Group group : PGM.get().getConfiguration().getGroups()) {
+      if (bukkit.hasPermission(group.getPermission())) {
+        permissionAttachment.setPermission(groupFunction.apply(group), true);
+      }
+    }
+  }
+
+  public void revokePermission(MatchPlayer player) {
+    if (permissionAttachment != null) player.getBukkit().removeAttachment(permissionAttachment);
+  }
+
+  public static class Observer extends StatePermissions {
+    public Observer() {
+      super(Config.Group::getObserverPermission);
+    }
+  }
+
+  public static class Participant extends StatePermissions {
+    public Participant() {
+      super(Config.Group::getParticipantPermission);
+    }
+  }
+}