Rework per-class permissions handling.

This removes the map of attachments in ArenaImpl and makes the permissions "replacement" happen via the Bukkit API's PermissionAttachmentInfo. Bukkit already keeps track of which attachments we've added to a player, so removing them is just a matter of looping through and finding those. This reduces the coupling between the Arena interface and UpgradeWave (which is the only interface that used the methods removed from the Arena interface).

src/main/java/com/garbagemule/MobArena/ArenaImpl.java

@@ -48,6 +48,7 @@ import org.bukkit.inventory.InventoryView;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.PlayerInventory;
 import org.bukkit.permissions.PermissionAttachment;
+import org.bukkit.permissions.PermissionAttachmentInfo;
 import org.bukkit.potion.PotionEffect;
 
 import java.util.ArrayDeque;
@@ -60,7 +61,6 @@ import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.PriorityBlockingQueue;
 import java.util.logging.Level;
@@ -102,7 +102,6 @@ public class ArenaImpl implements Arena
     // Classes stuff
     private ArenaClass defaultClass;
     private Map<String,ArenaClass> classes;
-    private Map<Player,PermissionAttachment> attachments;
     
     // Blocks and pets
     private PriorityBlockingQueue<Repairable> repairQueue;
@@ -173,7 +172,6 @@ public class ArenaImpl implements Arena
 
         // Classes, items and permissions
         this.classes      = plugin.getArenaMaster().getClasses();
-        this.attachments  = new HashMap<>();
         this.limitManager = new ClassLimitManager(this, classes, makeSection(section, "class-limits"));
 
         String defaultClassName = settings.getString("default-class", null);
@@ -521,7 +519,7 @@ public class ArenaImpl implements Arena
             p.teleport(region.getArenaWarp());
             movingPlayers.remove(p);
 
-            assignClassPermissions(p);
+            addClassPermissions(p);
             arenaPlayerMap.get(p).resetStats();
 
             Thing price = arenaPlayerMap.get(p).getArenaClass().getPrice();
@@ -772,7 +770,7 @@ public class ArenaImpl implements Arena
             clearInv(p);
         }
         
-        removeClassPermissions(p);
+        removePermissionAttachments(p);
         removePotionEffects(p);
         
         boolean refund = inLobby(p);
@@ -872,7 +870,7 @@ public class ArenaImpl implements Arena
     @Override
     @SuppressWarnings("deprecation")
     public void revivePlayer(Player p) {
-        removeClassPermissions(p);
+        removePermissionAttachments(p);
         removePotionEffects(p);
         
         discardPlayer(p);
@@ -1168,9 +1166,7 @@ public class ArenaImpl implements Arena
         removePotionEffects(p);
         arenaPlayer.setArenaClass(arenaClass);
         arenaClass.grantItems(p);
-
-        PermissionAttachment pa = arenaClass.grantLobbyPermissions(plugin, p);
-        replacePermissions(p, pa);
+        arenaClass.grantLobbyPermissions(plugin, p);
 
         autoReady(p);
     }
@@ -1185,6 +1181,7 @@ public class ArenaImpl implements Arena
         }
         
         InventoryManager.clearInventory(p);
+        removePermissionAttachments(p);
         removePotionEffects(p);
         arenaPlayer.setArenaClass(arenaClass);
         
@@ -1243,24 +1240,11 @@ public class ArenaImpl implements Arena
         inv.setBoots(boots);
         inv.setItemInOffHand(offhand);
 
-        PermissionAttachment pa = arenaClass.grantLobbyPermissions(plugin, p);
-        replacePermissions(p, pa);
+        arenaClass.grantLobbyPermissions(plugin, p);
 
         autoReady(p);
     }
 
-    private void replacePermissions(Player p, PermissionAttachment rep) {
-        PermissionAttachment old = attachments.get(p);
-        if (old != null) {
-            old.remove();
-            p.recalculatePermissions();
-        }
-        if (rep != null) {
-            attachments.put(p, rep);
-            p.recalculatePermissions();
-        }
-    }
-    
     private void autoReady(Player p) {
         if (settings.getBoolean("auto-ready", false)) {
             if (autoStartTimer.getRemaining() <= 0) {
@@ -1296,43 +1280,25 @@ public class ArenaImpl implements Arena
         messenger.tell(p, Msg.LOBBY_CLASS_PICKED, this.classes.get(className).getConfigName());
     }
 
-    @Override
-    public void assignClassPermissions(Player p)
-    {
-        PermissionAttachment pa = arenaPlayerMap.get(p).getArenaClass().grantPermissions(plugin, p);
-        replacePermissions(p, pa);
+    private void addClassPermissions(Player player) {
+        ArenaPlayer arenaPlayer = arenaPlayerMap.get(player);
+        if (arenaPlayer == null) {
+            return;
+        }
+        ArenaClass arenaClass = arenaPlayer.getArenaClass();
+        if (arenaClass == null) {
+            return;
+        }
+        removePermissionAttachments(player);
+        arenaClass.grantPermissions(plugin, player);
     }
 
-    @Override
-    public void removeClassPermissions(Player p)
-    {
-        PermissionAttachment pa = attachments.remove(p);
-        if (pa == null) return;
-        
-        try {
-            p.removeAttachment(pa);
-        }
-        catch (Exception e) {
-            for (Entry<String,Boolean> entry : pa.getPermissions().entrySet()) {
-                String perm = entry.getKey() + ":" + entry.getValue();
-                String name = p.getName();
-
-                plugin.getLogger().warning("[PERM01] Failed to remove permission attachment '" + perm + "' from player '" + name
-                                  + "'.\nThis should not be a big issue, but please verify that the player doesn't have any permissions they shouldn't have.");
-            }
-        }
-        p.recalculatePermissions();
-    }
-
-    @Override
-    public void addPermission(Player p, String perm, boolean value) {
-        PermissionAttachment pa = attachments.get(p);
-        if (pa == null) {
-            pa = p.addAttachment(plugin);
-            attachments.put(p, pa);
-        }
-        pa.setPermission(perm, value);
-        p.recalculatePermissions();
+    private void removePermissionAttachments(Player player) {
+        player.getEffectivePermissions().stream()
+            .filter(info -> info.getAttachment() != null)
+            .filter(info -> info.getAttachment().getPlugin().equals(plugin))
+            .map(PermissionAttachmentInfo::getAttachment)
+            .forEach(PermissionAttachment::remove);
     }
     
     private void removePotionEffects(Player p) {

src/main/java/com/garbagemule/MobArena/framework/Arena.java

@@ -192,12 +192,6 @@ public interface Arena
     
     void assignRandomClass(Player p);
 
-    void assignClassPermissions(Player p);
-    
-    void removeClassPermissions(Player p);
-
-    void addPermission(Player p, String perm, boolean value);
-    
     
     
     /*////////////////////////////////////////////////////////////////////

src/main/java/com/garbagemule/MobArena/waves/types/UpgradeWave.java

@@ -8,6 +8,7 @@ import com.garbagemule.MobArena.waves.Wave;
 import com.garbagemule.MobArena.waves.enums.WaveType;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.ItemStack;
+import org.bukkit.permissions.PermissionAttachment;
 
 import java.util.ArrayList;
 import java.util.HashMap;
@@ -182,7 +183,8 @@ public class UpgradeWave extends AbstractWave
         public void upgrade(Arena arena, Player p) {
             if (perm == null) return;
 
-            arena.addPermission(p, perm, value);
+            PermissionAttachment attachment = p.addAttachment(arena.getPlugin());
+            attachment.setPermission(perm, value);
         }
     }
 }