Fixed splatter attribute & moved messages to action bar

- Changed AttributeModifier to an enchantment, because the former was
  arbitrary removed by Minecraft or Spigot.
- Used action bar messages instead of chat messages at various places.

src/main/java/fr/moribus/imageonmap/commands/maptool/NewCommand.java

@@ -23,11 +23,15 @@ import fr.moribus.imageonmap.commands.IoMCommand;
 import fr.moribus.imageonmap.image.ImageRendererExecutor;
 import fr.moribus.imageonmap.image.ImageUtils;
 import fr.moribus.imageonmap.map.ImageMap;
+import fr.moribus.imageonmap.map.PosterMap;
 import fr.zcraft.zlib.components.commands.CommandException;
 import fr.zcraft.zlib.components.commands.CommandInfo;
 import fr.zcraft.zlib.components.i18n.I;
 import fr.zcraft.zlib.components.worker.WorkerCallback;
 import fr.zcraft.zlib.tools.PluginLogger;
+import fr.zcraft.zlib.tools.text.ActionBar;
+import fr.zcraft.zlib.tools.text.MessageSender;
+import org.bukkit.ChatColor;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 
@@ -72,14 +76,16 @@ public class NewCommand  extends IoMCommand
             }
         }
         
-        info(I.t("Rendering..."));
+        ActionBar.sendPermanentMessage(player, ChatColor.DARK_GREEN + I.t("Rendering..."));
         ImageRendererExecutor.render(url, scaling, player.getUniqueId(), width, height, new WorkerCallback<ImageMap>()
         {
             @Override
             public void finished(ImageMap result)
             {
-                player.sendMessage(I.t("{cst}Rendering finished!"));
+                ActionBar.removeMessage(player);
-                if(result.give(player))
+                MessageSender.sendActionBarMessage(player, ChatColor.DARK_GREEN + I.t("Rendering finished!"));
+
+                if (result.give(player) && (result instanceof PosterMap && !((PosterMap) result).hasColumnData()))
                 {
                     info(I.t("The rendered map was too big to fit in your inventory."));
                     info(I.t("Use '/maptool getremaining' to get the remaining maps."));

src/main/java/fr/moribus/imageonmap/ui/MapItemManager.java

@@ -22,16 +22,12 @@ import fr.moribus.imageonmap.map.ImageMap;
 import fr.moribus.imageonmap.map.MapManager;
 import fr.moribus.imageonmap.map.PosterMap;
 import fr.moribus.imageonmap.map.SingleMap;
-import fr.zcraft.zlib.components.attributes.Attributes;
 import fr.zcraft.zlib.components.i18n.I;
 import fr.zcraft.zlib.core.ZLib;
 import fr.zcraft.zlib.tools.PluginLogger;
 import fr.zcraft.zlib.tools.items.ItemStackBuilder;
 import fr.zcraft.zlib.tools.items.ItemUtils;
-import fr.zcraft.zlib.tools.reflection.NMSException;
+import org.bukkit.*;
-import org.bukkit.Color;
-import org.bukkit.GameMode;
-import org.bukkit.Material;
 import org.bukkit.entity.ItemFrame;
 import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
@@ -51,132 +47,106 @@ import java.util.UUID;
 public class MapItemManager implements Listener
 {
     static private HashMap<UUID, Queue<ItemStack>> mapItemCache;
-    
+
     static public void init()
     {
-        mapItemCache = new HashMap();
+        mapItemCache = new HashMap<>();
         ZLib.registerEvents(new MapItemManager());
     }
-    
+
     static public void exit()
     {
-        if(mapItemCache != null) mapItemCache.clear();
+        if (mapItemCache != null) mapItemCache.clear();
         mapItemCache = null;
     }
-    
+
     static public boolean give(Player player, ImageMap map)
     {
-        if(map instanceof PosterMap) return give(player, (PosterMap) map);
+        if (map instanceof PosterMap) return give(player, (PosterMap) map);
-        else if(map instanceof SingleMap) return give(player, (SingleMap) map);
+        else if (map instanceof SingleMap) return give(player, (SingleMap) map);
         return false;
     }
-    
+
     static public boolean give(Player player, SingleMap map)
     {
         return give(player, createMapItem(map));
     }
-    
+
     static public boolean give(Player player, PosterMap map)
     {
-        if(!map.hasColumnData())
+        if (!map.hasColumnData())
             return giveParts(player, map);
         return give(player, SplatterMapManager.makeSplatterMap(map));
     }
-    
+
     static public boolean giveParts(Player player, PosterMap map)
     {
         boolean inventoryFull = false;
-        
+
         ItemStack mapPartItem;
-        for(int i = 0, c = map.getMapCount(); i < c; i++)
+        for (int i = 0, c = map.getMapCount(); i < c; i++)
         {
-            if(map.hasColumnData())
+            mapPartItem = map.hasColumnData() ? createMapItem(map, map.getColumnAt(i), map.getRowAt(i)) : createMapItem(map, i);
-            {
-                mapPartItem = createMapItem(map, map.getColumnAt(i), map.getRowAt(i));
-            }
-            else
-            {
-                mapPartItem = createMapItem(map, i);
-            }
             inventoryFull = give(player, mapPartItem) || inventoryFull;
         }
-        
+
         return inventoryFull;
     }
-    
+
     static public int giveCache(Player player)
     {
         Queue<ItemStack> cache = getCache(player);
         Inventory inventory = player.getInventory();
         int givenItemsCount = 0;
-        
+
-        while(inventory.firstEmpty() >= 0 && !cache.isEmpty())
+        while (inventory.firstEmpty() >= 0 && !cache.isEmpty())
         {
-            inventory.addItem(cache.poll());
+            give(player, cache.poll());
             givenItemsCount++;
         }
-        
+
         return givenItemsCount;
     }
-    
+
     static private boolean give(final Player player, final ItemStack item)
     {
-        final int freeSlot = player.getInventory().firstEmpty();
+        boolean given = ItemUtils.give(player, item);
-        if (freeSlot != -1)
-        {
-            player.getInventory().setItem(freeSlot, item);
 
-            // If this is a splatter map, we have to re-add the splatter attribute, because
+        if (given)
-            // for some reason, the `addItem` or `setItem` methods removes the attribute in
-            // Minecraft 1.14+, breaking the auto-deploy feature.
-            if (SplatterMapManager.hasSplatterAttributes(item))
-            {
-                try
-                {
-                    Attributes.set(player.getInventory().getItem(freeSlot), new SplatterMapManager.Attribute());
-                }
-                catch (NMSException e)
-                {
-                    PluginLogger.error("Unable to add back attribute to splatter map");
-                }
-            }
-
-            return false;
-        }
-        else
         {
-            ItemUtils.drop(player.getLocation(), item);
+            player.playSound(player.getLocation(), Sound.ENTITY_ITEM_PICKUP, SoundCategory.PLAYERS, 1, 1);
-            return true;
         }
+
+        return !given;
     }
-    
+
     static public ItemStack createMapItem(SingleMap map)
     {
         return createMapItem(map.getMapsIDs()[0], map.getName(), false);
     }
-    
+
     static public ItemStack createMapItem(PosterMap map, int index)
     {
         return createMapItem(map.getMapIdAt(index), getMapTitle(map, index), true);
     }
-    
+
     static public ItemStack createMapItem(PosterMap map, int x, int y)
     {
-        return createMapItem(map.getMapIdAt(x, y),  getMapTitle(map, y, x), true);
+        return createMapItem(map.getMapIdAt(x, y), getMapTitle(map, y, x), true);
     }
-    
+
     static public String getMapTitle(PosterMap map, int row, int column)
     {
         /// The name of a map item given to a player, if splatter maps are not used. 0 = map name; 1 = row; 2 = column.
         return I.t("{0} (row {1}, column {2})", map.getName(), row + 1, column + 1);
     }
-    
+
     static public String getMapTitle(PosterMap map, int index)
     {
         /// The name of a map item given to a player, if splatter maps are not used. 0 = map name; 1 = index.
         return I.t("{0} (part {1})", map.getName(), index + 1);
     }
-    
+
     static public ItemStack createMapItem(int mapID, String text, boolean isMapPart)
     {
         final ItemStack mapItem = new ItemStackBuilder(Material.FILLED_MAP)
@@ -196,22 +166,20 @@ public class MapItemManager implements Listener
      * Returns the item to place to display the (col;row) part of the given poster.
      *
      * @param map The map to take the part from.
-     * @param x The x coordinate of the part to display. Starts at 0.
+     * @param x   The x coordinate of the part to display. Starts at 0.
-     * @param y The y coordinate of the part to display. Starts at 0.
+     * @param y   The y coordinate of the part to display. Starts at 0.
-     *
      * @return The map.
-     *
      * @throws ArrayIndexOutOfBoundsException If x;y is not inside the map.
      */
     static public ItemStack createSubMapItem(ImageMap map, int x, int y)
     {
-        if(map instanceof PosterMap && ((PosterMap) map).hasColumnData())
+        if (map instanceof PosterMap && ((PosterMap) map).hasColumnData())
         {
             return MapItemManager.createMapItem((PosterMap) map, x, y);
         }
         else
         {
-            if(x != 0 || y != 0)
+            if (x != 0 || y != 0)
             {
                 throw new ArrayIndexOutOfBoundsException(); // Coherence
             }
@@ -219,27 +187,27 @@ public class MapItemManager implements Listener
             return createMapItem(map.getMapsIDs()[0], map.getName(), false);
         }
     }
-    
+
     static public int getCacheSize(Player player)
     {
         return getCache(player).size();
     }
-    
+
     static private Queue<ItemStack> getCache(Player player)
     {
         Queue<ItemStack> cache = mapItemCache.get(player.getUniqueId());
-        if(cache == null)
+        if (cache == null)
         {
             cache = new ArrayDeque<>();
             mapItemCache.put(player.getUniqueId(), cache);
         }
         return cache;
     }
-    
+
     static private String getMapTitle(ItemStack item)
     {
         ImageMap map = MapManager.getMap(item);
-        if(map instanceof SingleMap)
+        if (map instanceof SingleMap)
         {
             return map.getName();
         }
@@ -247,75 +215,76 @@ public class MapItemManager implements Listener
         {
             PosterMap poster = (PosterMap) map;
             int index = poster.getIndex(MapManager.getMapIdFromItemStack(item));
-            if(poster.hasColumnData())
+            if (poster.hasColumnData())
                 return getMapTitle(poster, poster.getRowAt(index), poster.getColumnAt(index));
-            
+
             return getMapTitle(poster, index);
         }
     }
-    
+
     static private void onItemFramePlace(ItemFrame frame, Player player, PlayerInteractEntityEvent event)
     {
         final ItemStack mapItem = player.getInventory().getItemInMainHand();
 
-        if(frame.getItem().getType() != Material.AIR) return;
+        if (frame.getItem().getType() != Material.AIR) return;
-        if(!MapManager.managesMap(mapItem)) return;
+        if (!MapManager.managesMap(mapItem)) return;
+
         event.setCancelled(true);
 
-        if(SplatterMapManager.hasSplatterAttributes(mapItem))
+        if (SplatterMapManager.hasSplatterAttributes(mapItem))
         {
-            if(!SplatterMapManager.placeSplatterMap(frame, player))
+            if (!SplatterMapManager.placeSplatterMap(frame, player))
                 return;
         }
         else
         {
-            frame.setItem(player.getInventory().getItemInMainHand());
+            frame.setItem(mapItem);
         }
-        
+
         ItemUtils.consumeItem(player);
     }
-    
+
     static private void onItemFrameRemove(ItemFrame frame, Player player, EntityDamageByEntityEvent event)
     {
         ItemStack item = frame.getItem();
-        if(frame.getItem().getType() != Material.FILLED_MAP) return;
+        if (frame.getItem().getType() != Material.FILLED_MAP) return;
-        
+
-        if(player.isSneaking())
+        if (player.isSneaking())
         {
             PosterMap poster = SplatterMapManager.removeSplatterMap(frame);
-            if(poster != null)
+            if (poster != null)
             {
                 event.setCancelled(true);
-                
+
-                if(player.getGameMode() != GameMode.CREATIVE || !SplatterMapManager.hasSplatterMap(player, poster))
+                if (player.getGameMode() != GameMode.CREATIVE || !SplatterMapManager.hasSplatterMap(player, poster))
                     poster.give(player);
-                
+
                 return;
             }
         }
-        
+
-        if(!MapManager.managesMap(frame.getItem())) return;
+        if (!MapManager.managesMap(frame.getItem())) return;
-        
+
         frame.setItem(new ItemStackBuilder(item)
                 .title(getMapTitle(item))
                 .hideAttributes()
                 .item());
-        
+
     }
-    
+
     @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
     static public void onEntityDamage(EntityDamageByEntityEvent event)
     {
-        if(!(event.getEntity() instanceof ItemFrame)) return;
+        if (!(event.getEntity() instanceof ItemFrame)) return;
-        if(!(event.getDamager() instanceof Player)) return;
+        if (!(event.getDamager() instanceof Player)) return;
-        
+
-        onItemFrameRemove((ItemFrame)event.getEntity(), (Player)event.getDamager(), event);
+        onItemFrameRemove((ItemFrame) event.getEntity(), (Player) event.getDamager(), event);
     }
-    
+
     @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
     static public void onEntityInteract(PlayerInteractEntityEvent event)
     {
-        if(!(event.getRightClicked() instanceof ItemFrame)) return;
+        if (!(event.getRightClicked() instanceof ItemFrame)) return;
-        onItemFramePlace((ItemFrame)event.getRightClicked(), event.getPlayer(), event);
+        onItemFramePlace((ItemFrame) event.getRightClicked(), event.getPlayer(), event);
     }
 }

src/main/java/fr/moribus/imageonmap/ui/SplatterMapManager.java

@@ -23,12 +23,15 @@ import fr.moribus.imageonmap.image.MapInitEvent;
 import fr.moribus.imageonmap.map.ImageMap;
 import fr.moribus.imageonmap.map.MapManager;
 import fr.moribus.imageonmap.map.PosterMap;
-import fr.zcraft.zlib.components.attributes.Attributes;
 import fr.zcraft.zlib.components.i18n.I;
+import fr.zcraft.zlib.components.nbt.NBT;
+import fr.zcraft.zlib.components.nbt.NBTCompound;
 import fr.zcraft.zlib.components.nbt.NBTException;
+import fr.zcraft.zlib.components.nbt.NBTList;
 import fr.zcraft.zlib.tools.PluginLogger;
 import fr.zcraft.zlib.tools.items.ItemStackBuilder;
 import fr.zcraft.zlib.tools.reflection.NMSException;
+import fr.zcraft.zlib.tools.text.MessageSender;
 import fr.zcraft.zlib.tools.world.FlatLocation;
 import org.bukkit.ChatColor;
 import org.bukkit.Color;
@@ -39,8 +42,6 @@ import org.bukkit.inventory.Inventory;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.meta.MapMeta;
 
-import java.util.UUID;
-
 abstract public class SplatterMapManager 
 {
     private SplatterMapManager() {}
@@ -69,21 +70,73 @@ abstract public class SplatterMapManager
         meta.setColor(Color.GREEN);
         splatter.setItemMeta(meta);
 
+        return addSplatterAttribute(splatter);
+    }
+
+    /**
+     * To identify image on maps for the auto-splattering to work, we mark the items
+     * using an enchantment maps are not supposed to have (Mending).
+     *
+     * Then we check if the map is enchanted at all to know if it's a splatter map.
+     * This ensure compatibility with old splatter maps from 3.x, where zLib's glow
+     * effect was used.
+     *
+     * An AttributeModifier (using zLib's attributes system) is not used, because
+     * Minecraft (or Spigot) removes them from maps in 1.14+, so that wasn't stable
+     * enough (and the glowing effect of enchantments is prettier).
+     *
+     * @param itemStack The item stack to mark as a splatter map.
+     * @return The modified item stack. The instance may be different if the
+     * passed item stack is not a craft item stack; that's why the instance is
+     * returned.
+     */
+    static public ItemStack addSplatterAttribute(final ItemStack itemStack)
+    {
         try
         {
-            Attributes.set(splatter, new Attribute());
+            final NBTCompound nbt = NBT.fromItemStack(itemStack);
+            final NBTList enchantments = new NBTList();
+            final NBTCompound protection = new NBTCompound();
+
+            protection.put("id", "minecraft:mending");
+            protection.put("lvl", 1);
+            enchantments.add(protection);
+
+            nbt.put("Enchantments", enchantments);
+
+            return NBT.addToItemStack(itemStack, nbt, false);
         }
         catch (NBTException | NMSException e)
         {
             PluginLogger.error("Unable to set Splatter Map attribute on item", e);
+            return itemStack;
         }
-
-        return splatter;
     }
-    
+
+    /**
+     * Checks if an item have the splatter attribute set (i.e. if the item is
+     * enchanted in any way).
+     *
+     * @param itemStack The item to check.
+     * @return True if the attribute was detected.
+     */
     static public boolean hasSplatterAttributes(ItemStack itemStack)
     {
-        return Attribute.hasAttribute(itemStack);
+        try
+        {
+            final NBTCompound nbt = NBT.fromItemStack(itemStack);
+            if (!nbt.containsKey("Enchantments")) return false;
+
+            final Object enchantments = nbt.get("Enchantments");
+            if (!(enchantments instanceof NBTList)) return false;
+
+            return !((NBTList) enchantments).isEmpty();
+        }
+        catch (NMSException e)
+        {
+            PluginLogger.error("Unable to get Splatter Map attribute on item", e);
+            return false;
+        }
     }
     
     static public boolean isSplatterMap(ItemStack itemStack)
@@ -119,14 +172,14 @@ abstract public class SplatterMapManager
         wall.loc1 = startLocation;
         wall.loc2 = endLocation;
         
-        if(!wall.isValid())
+        if (!wall.isValid())
         {
-            player.sendMessage(I.t("{ce}There is not enough space to place this map ({0} × {1}).", poster.getColumnCount(), poster.getRowCount()));
+            MessageSender.sendActionBarMessage(player, I.t("{ce}There is not enough space to place this map ({0} × {1}).", poster.getColumnCount(), poster.getRowCount()));
             return false;
         }
         
         int i = 0;
-        for(ItemFrame frame : wall.frames)
+        for (ItemFrame frame : wall.frames)
         {
             int id = poster.getMapIdAtReverseY(i);
             frame.setItem(new ItemStackBuilder(Material.FILLED_MAP).nbt(ImmutableMap.of("map", id)).craftItem());
@@ -154,29 +207,4 @@ abstract public class SplatterMapManager
         
         return poster;
     }
-
-
-    static public class Attribute extends fr.zcraft.zlib.components.attributes.Attribute
-    {
-        static public final UUID SPLATTER_MAP_UUID = UUID.fromString("260ae1b3-4b24-40a0-a30d-67ad84b7bdb2");
-
-        Attribute()
-        {
-            setUUID(SPLATTER_MAP_UUID);
-            setCustomData("splatter-map");
-        }
-
-        static boolean hasAttribute(final ItemStack item)
-        {
-            try
-            {
-                return Attributes.get(item, SPLATTER_MAP_UUID) != null;
-            }
-            catch (NMSException | NBTException ex)
-            {
-                PluginLogger.warning("Error while retrieving SplatterMap Attribute data", ex);
-                return false;
-            }
-        }
-    }
 }

src/main/resources/i18n/en_US.po

@@ -108,8 +108,8 @@ msgid "Rendering..."
 msgstr "Rendering..."
 
 #: src/main/java/fr/moribus/imageonmap/commands/maptool/NewCommand.java:67
-msgid "{cst}Rendering finished!"
+msgid "Rendering finished!"
-msgstr "{cst}Rendering finished!"
+msgstr "Rendering finished!"
 
 #: src/main/java/fr/moribus/imageonmap/commands/maptool/NewCommand.java:70
 msgid "The rendered map was too big to fit in your inventory."

src/main/resources/i18n/fr_FR.po

@@ -371,8 +371,8 @@ msgid "Rendering..."
 msgstr "Rendu en cours..."
 
 #: src/main/java/fr/moribus/imageonmap/commands/maptool/NewCommand.java:69
-msgid "{cst}Rendering finished!"
+msgid "Rendering finished!"
-msgstr "{cst}Rendu achevé !"
+msgstr "Rendu achevé !"
 
 #: src/main/java/fr/moribus/imageonmap/commands/maptool/NewCommand.java:72
 msgid "The rendered map was too big to fit in your inventory."