Refactoring: icon refresh improvements

Plugin/src/main/java/me/filoghost/chestcommands/menu/AdvancedIconMenu.java

@@ -20,7 +20,6 @@ import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.Inventory;
 import org.bukkit.inventory.ItemStack;
-import org.bukkit.inventory.meta.ItemMeta;
 
 import me.filoghost.chestcommands.ChestCommands;
 import me.filoghost.chestcommands.Permissions;
@@ -73,11 +72,6 @@ public class AdvancedIconMenu extends BaseIconMenu<AdvancedIcon> {
 		super.open(player);
 	}
 	
-	@Override
-	protected boolean canViewIcon(Player player, AdvancedIcon icon) {
-		return icon.canViewIcon(player);
-	}
-	
 	public void openCheckingPermission(Player player) {
 		if (player.hasPermission(openPermission)) {
 			open(player);
@@ -93,31 +87,11 @@ public class AdvancedIconMenu extends BaseIconMenu<AdvancedIcon> {
 				continue;
 			}
 			
-			if (icon.hasViewPermission() || icon.hasVariables()) {
-				// Then we have to refresh it
-				ItemStack currentItem = inventory.getItem(i);
-				ItemStack newItem = refreshIcon(player, icon, currentItem);
-				inventory.setItem(i, newItem);
-			}
+			ItemStack newItemStack = icon.refreshItemStack(player, inventory.getItem(i));
+			inventory.setItem(i, newItemStack);
 		}
 	}
 
-	private ItemStack refreshIcon(Player player, AdvancedIcon icon, ItemStack currentItem) {
-		if (icon.canViewIcon(player)) {
-			if (currentItem == null) {
-				return icon.createItemStack(player);
-			} else {
-				// Performance, only update name and lore
-				ItemMeta meta = currentItem.getItemMeta();
-				meta.setDisplayName(icon.calculateName(player));
-				meta.setLore(icon.calculateLore(player));
-				currentItem.setItemMeta(meta);
-				return currentItem;
-			}
-		} else {
-			return null;
-		}
-	}
 
 	public void sendNoOpenPermissionMessage(CommandSender sender) {
 		String noPermMessage = ChestCommands.getLang().no_open_permission;

Plugin/src/main/java/me/filoghost/chestcommands/menu/BaseIconMenu.java

@@ -17,6 +17,8 @@ package me.filoghost.chestcommands.menu;
 import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.Inventory;
+import org.bukkit.inventory.ItemStack;
+
 import me.filoghost.chestcommands.api.Icon;
 import me.filoghost.chestcommands.menu.inventory.Grid;
 import me.filoghost.chestcommands.menu.inventory.MenuInventoryHolder;
@@ -70,17 +72,20 @@ public class BaseIconMenu<T extends Icon> {
 
 		for (int i = 0; i < inventoryGrid.getSize(); i++) {
 			T icon = inventoryGrid.getElementAtIndex(i);
-			if (icon != null && canViewIcon(player, icon)) {
-				inventory.setItem(i, icon.createItemStack(player));
+			if (icon == null) {
+				continue;
 			}
+			
+			ItemStack itemStack = icon.createItemStack(player);
+			if (itemStack == null) {
+				continue;
+			}
+			
+			inventory.setItem(i, itemStack);
 		}
 
 		player.openInventory(inventory);
 	}
-	
-	protected boolean canViewIcon(Player player, T icon) {
-		return true;
-	}
 
 	@Override
 	public String toString() {

Plugin/src/main/java/me/filoghost/chestcommands/menu/icon/AdvancedIcon.java

@@ -19,6 +19,8 @@ import java.util.List;
 import org.bukkit.Material;
 import org.bukkit.entity.Player;
 import org.bukkit.inventory.Inventory;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.meta.ItemMeta;
 
 import me.filoghost.chestcommands.action.Action;
 import me.filoghost.chestcommands.action.OpenMenuAction;
@@ -89,6 +91,42 @@ public class AdvancedIcon extends ConfigurableIconImpl {
 	public void setClickActions(List<Action> clickActions) {
 		this.clickActions = Utils.nullableCopy(clickActions);
 	}
+	
+	
+	@Override
+	public ItemStack createItemStack(Player viewer) {
+		if (canViewIcon(viewer)) {
+			return super.createItemStack(viewer);
+		} else {
+			return null;
+		}
+	}
+	
+	
+	public ItemStack refreshItemStack(Player viewer, ItemStack currentItemStack) {
+		if (!hasViewPermission() && !hasVariables()) {
+			// Icon is not dynamic, no need to refresh
+			return currentItemStack;
+		}
+		
+		if (!canViewIcon(viewer)) {
+			// Player can't view the icon
+			return null;
+		}
+		
+		if (currentItemStack == null) {
+			// Create item from scratch
+			return createItemStack(viewer);
+		} else {
+			// Performance: only update name and lore
+			ItemMeta meta = currentItemStack.getItemMeta();
+			meta.setDisplayName(calculateName(viewer));
+			meta.setLore(calculateLore(viewer));
+			currentItemStack.setItemMeta(meta);
+			return currentItemStack;
+		}
+	}
+	
 
 	@Override
 	public boolean onClick(Inventory inventory, Player player) {