Delegate workbench apply button and slot behaviour to item classes

src/main/java/electroblob/wizardry/item/IWorkbenchItem.java

@@ -0,0 +1,56 @@
+package electroblob.wizardry.item;
+
+import electroblob.wizardry.event.SpellBindEvent;
+import net.minecraft.entity.player.EntityPlayer;
+import net.minecraft.inventory.Slot;
+import net.minecraft.item.ItemStack;
+
+/**
+ * Items that implement this interface may be placed in the central slot of the arcane workbench as long as
+ * {@link IWorkbenchItem#canPlace(ItemStack)} returns true.The number of spell book slots displayed is also specified
+ * using {@link IWorkbenchItem#getSpellSlotCount(ItemStack)}.
+ * <p>
+ * Items that implement this interface define what happens if they are in the central slot of the arcane workbench and
+ * the apply button is pressed, in {@link IWorkbenchItem#onApplyButtonPressed(EntityPlayer, Slot, Slot, Slot, Slot[])}.
+ * This is a core part of the arcane workbench refactoring in version 4.2 and allows for custom spell casting items and
+ * chargeable armour without requiring that they extend {@link ItemWand} or {@link ItemWizardArmour}.
+ * @author Electroblob
+ * @since Wizardry 4.2
+ */
+public interface IWorkbenchItem {
+	
+	/** 
+	 * Returns true if the item can be placed in the central slot of an arcane workbench, false otherwise. Allows
+	 * for itemstack-sensitive behaviour. Returns true by default.
+	 * @param stack The stack that is being placed into the workbench.
+	 * @return True to allow the item to be placed into the workbench, false to prevent that from happening.
+	 */
+	default boolean canPlace(ItemStack stack){
+		return true;
+	}
+	
+	/**
+	 * Returns the number of spell book slots that should appear in the workbench when this item is placed into it,
+	 * based on the given itemstack.
+	 * @param stack The stack that is being placed into the workbench.
+	 * @return The number of spell book slots that should appear around this item when it is placed into the workbench.
+	 * Can be 0, but must not be negative.
+	 */
+	int getSpellSlotCount(ItemStack stack);
+	
+	/**
+	 * Called when this item is in the central slot of an arcane workbench and the apply button is pressed. Items must
+	 * implement this method to define what happens when the apply button is pressed. Note that {@link SpellBindEvent}
+	 * is fired <i>before</i> this method is called.
+	 * @param player The player that pressed the apply button.
+	 * @param centre The central slot in the arcane workbench. This slot will always contain a stack of the implementing
+	 * item, or in other words, <i>it is guaranteed that</i> {@code this == centre.getStack().getItem()}.
+	 * @param crystals The magic crystal slot of the arcane workbench.
+	 * @param upgrade The upgrade slot of the arcane workbench.
+	 * @param spellBooks An array of the <i>active</i> (visible) spell book slots in the arcane workbench. The length of
+	 * the array will be equal to the value returned by {@link IWorkbenchItem#getSpellSlotCount(ItemStack)}.
+	 * @return True if anything changed, false if not.
+	 */
+	boolean onApplyButtonPressed(EntityPlayer player, Slot centre, Slot crystals, Slot upgrade, Slot[] spellBooks);
+	
+}

src/main/java/electroblob/wizardry/item/ItemBlankScroll.java

@@ -0,0 +1,56 @@
+package electroblob.wizardry.item;
+
+import electroblob.wizardry.WizardData;
+import electroblob.wizardry.constants.Constants;
+import electroblob.wizardry.registry.Spells;
+import electroblob.wizardry.registry.WizardryItems;
+import electroblob.wizardry.registry.WizardryTabs;
+import electroblob.wizardry.spell.Spell;
+import net.minecraft.entity.player.EntityPlayer;
+import net.minecraft.inventory.Slot;
+import net.minecraft.item.Item;
+import net.minecraft.item.ItemStack;
+
+public class ItemBlankScroll extends Item implements IWorkbenchItem {
+
+	public ItemBlankScroll(){
+		this.setCreativeTab(WizardryTabs.WIZARDRY);
+	}
+
+	@Override
+	public int getSpellSlotCount(ItemStack stack){
+		return 1;
+	}
+
+	@Override
+	public boolean onApplyButtonPressed(EntityPlayer player, Slot centre, Slot crystals, Slot upgrade, Slot[] spellBooks){
+		
+		if(!spellBooks[0].getStack().isEmpty() && !crystals.getStack().isEmpty()){
+			
+			Spell spell = Spell.get(spellBooks[0].getStack().getItemDamage());
+			WizardData data = WizardData.get(player);
+
+			// Spells can only be bound to scrolls if the player has already cast them (prevents casting of master
+			// spells without getting a master wand)
+			// This restriction does not apply in creative mode
+			if(spell != Spells.none && player.capabilities.isCreativeMode || (data != null
+					&& data.hasSpellBeenDiscovered(spell))){
+				
+				int cost = spell.cost;
+				// Continuous spell scrolls require enough mana to cast them for the duration defined in ItemScroll.
+				if(spell.isContinuous) cost *= ItemScroll.CASTING_TIME / 20;
+				
+				if(crystals.getStack().getCount() * Constants.MANA_PER_CRYSTAL > cost){
+					// Rounds up to the nearest whole crystal
+					crystals.decrStackSize(cost / Constants.MANA_PER_CRYSTAL + 1);
+					centre.putStack(new ItemStack(WizardryItems.scroll, 1, spell.id()));
+					return true;
+				}
+				
+			}
+		}
+		
+		return false;
+	}
+
+}

src/main/java/electroblob/wizardry/item/ItemWand.java

@@ -13,6 +13,7 @@ import electroblob.wizardry.event.SpellCastEvent;
 import electroblob.wizardry.event.SpellCastEvent.Source;
 import electroblob.wizardry.packet.PacketCastSpell;
 import electroblob.wizardry.packet.WizardryPacketHandler;
+import electroblob.wizardry.registry.Spells;
 import electroblob.wizardry.registry.WizardryAdvancementTriggers;
 import electroblob.wizardry.registry.WizardryItems;
 import electroblob.wizardry.registry.WizardryPotions;
@@ -30,6 +31,7 @@ import net.minecraft.entity.EntityLiving;
 import net.minecraft.entity.EntityLivingBase;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.inventory.EntityEquipmentSlot;
+import net.minecraft.inventory.Slot;
 import net.minecraft.item.EnumAction;
 import net.minecraft.item.Item;
 import net.minecraft.item.ItemStack;
@@ -59,7 +61,10 @@ import net.minecraftforge.fml.relauncher.SideOnly;
  * 
  * @since Wizardry 1.0
  */
-public class ItemWand extends Item {
+public class ItemWand extends Item implements IWorkbenchItem {
+	
+	/** The number of spell slots a wand has with no attunement upgrades applied. */
+	public static final int BASE_SPELL_SLOTS = 5;
 
 	public Tier tier;
 	public Element element;
@@ -424,4 +429,142 @@ public class ItemWand extends Item {
 
 		return false;
 	}
+
+	@Override
+	public int getSpellSlotCount(ItemStack stack){
+		return BASE_SPELL_SLOTS + WandHelper.getUpgradeLevel(stack, WizardryItems.attunement_upgrade);
+	}
+
+	@Override
+	public boolean onApplyButtonPressed(EntityPlayer player, Slot centre, Slot crystals, Slot upgrade, Slot[] spellBooks){
+		
+		boolean changed = false;
+		
+		// Upgrades wand if necessary. Damage is copied, preserving remaining durability,
+		// and also the entire NBT tag compound.
+		if(upgrade.getStack().getItem() == WizardryItems.arcane_tome){
+
+			// Checks the wand upgrade is for the tier above the wand's tier.
+			// It is guaranteed that: this == centre.getStack().getItem()
+			if(upgrade.getStack().getItemDamage() - 1 == this.tier.ordinal()){
+				
+				Tier tier = Tier.values()[upgrade.getStack().getItemDamage()];
+				
+				ItemStack newWand = new ItemStack(WizardryUtilities.getWand(tier, this.element));
+				newWand.setTagCompound(centre.getStack().getTagCompound());
+				// This needs to be done after copying the tag compound so the max damage for the new wand
+				// takes storage upgrades into account.
+				newWand.setItemDamage(newWand.getMaxDamage() - (centre.getStack().getMaxDamage() - centre.getStack().getItemDamage()));
+				
+				centre.putStack(newWand);
+				upgrade.decrStackSize(1);
+				
+				if(tier == Tier.APPRENTICE) WizardryAdvancementTriggers.apprentice.triggerFor(player);
+				if(tier == Tier.MASTER) WizardryAdvancementTriggers.master.triggerFor(player);
+				
+				changed = true;
+			}
+
+		}else if(WandHelper.isWandUpgrade(upgrade.getStack().getItem())){
+
+			// Special upgrades
+			Item specialUpgrade = upgrade.getStack().getItem();
+
+			if(WandHelper.getTotalUpgrades(centre.getStack()) < this.tier.upgradeLimit
+					&& WandHelper.getUpgradeLevel(centre.getStack(), specialUpgrade) < Constants.UPGRADE_STACK_LIMIT){
+
+				// Used to preserve existing mana when upgrading storage rather than creating free mana.
+				int prevMana = centre.getStack().getMaxDamage() - centre.getStack().getItemDamage();
+
+				WandHelper.applyUpgrade(centre.getStack(), specialUpgrade);
+
+				// Special behaviours for specific upgrades
+				if(specialUpgrade == WizardryItems.storage_upgrade){
+					
+					centre.getStack().setItemDamage(centre.getStack().getMaxDamage() - prevMana);
+					
+				}else if(specialUpgrade == WizardryItems.attunement_upgrade){
+
+					int newSlotCount = BASE_SPELL_SLOTS + WandHelper.getUpgradeLevel(centre.getStack(),
+							WizardryItems.attunement_upgrade);
+					
+					Spell[] spells = WandHelper.getSpells(centre.getStack());
+					Spell[] newSpells = new Spell[newSlotCount];
+
+					for(int i = 0; i < newSpells.length; i++){
+						newSpells[i] = i < spells.length && spells[i] != null ? spells[i] : Spells.none;
+					}
+
+					WandHelper.setSpells(centre.getStack(), newSpells);
+
+					int[] cooldowns = WandHelper.getCooldowns(centre.getStack());
+					int[] newCooldowns = new int[newSlotCount];
+
+					if(cooldowns.length > 0){
+						for(int i = 0; i < cooldowns.length; i++){
+							newCooldowns[i] = cooldowns[i];
+						}
+					}
+
+					WandHelper.setCooldowns(centre.getStack(), newCooldowns);
+				}
+
+				upgrade.decrStackSize(1);
+				WizardryAdvancementTriggers.special_upgrade.triggerFor(player);
+
+				if(WandHelper.getTotalUpgrades(centre.getStack()) == Tier.MASTER.upgradeLimit){
+					WizardryAdvancementTriggers.max_out_wand.triggerFor(player);
+				}
+				
+				changed = true;
+			}
+		}
+
+		// Reads NBT spell id array to variable, edits this, then writes it back to NBT.
+		// Original spells are preserved; if a slot is left empty the existing spell binding will remain.
+		// Accounts for spells which cannot be applied because they are above the wand's tier; these spells
+		// will not bind but the existing spell in that slot will remain and other applicable spells will
+		// be bound as normal, along with any upgrades and crystals.
+		Spell[] spells = WandHelper.getSpells(centre.getStack());
+		
+		if(spells.length <= 0){
+			// Base value here because if the spell array doesn't exist, the wand can't possibly have attunement upgrades
+			spells = new Spell[BASE_SPELL_SLOTS];
+		}
+		
+		for(int i = 0; i < spells.length; i++){
+			if(spellBooks[i].getStack() != ItemStack.EMPTY){
+				
+				Spell spell = Spell.get(spellBooks[i].getStack().getItemDamage());
+				// If the wand is powerful enough for the spell and it's not already bound to that slot
+				if(!(spell.tier.level > this.tier.level) && spells[i] != spell){
+					spells[i] = spell;
+					changed = true;
+				}
+			}
+		}
+		
+		WandHelper.setSpells(centre.getStack(), spells);
+
+		// Charges wand by appropriate amount
+		if(crystals.getStack() != ItemStack.EMPTY){
+			
+			int chargeDepleted = centre.getStack().getItemDamage();
+			
+			if(crystals.getStack().getCount() * Constants.MANA_PER_CRYSTAL < chargeDepleted){
+				
+				centre.getStack().setItemDamage(chargeDepleted - crystals.getStack().getCount() * Constants.MANA_PER_CRYSTAL);
+				crystals.decrStackSize(crystals.getStack().getCount());
+				changed = true;
+				
+			}else if(chargeDepleted != 0){
+
+				centre.getStack().setItemDamage(0);
+				crystals.decrStackSize((int)Math.ceil(((double)chargeDepleted) / Constants.MANA_PER_CRYSTAL));
+				changed = true;
+			}
+		}
+		
+		return changed;
+	}
 }

src/main/java/electroblob/wizardry/item/ItemWizardArmour.java

@@ -11,6 +11,7 @@ import electroblob.wizardry.block.BlockStatue;
 import electroblob.wizardry.constants.Constants;
 import electroblob.wizardry.constants.Element;
 import electroblob.wizardry.registry.WizardryAdvancementTriggers;
+import electroblob.wizardry.registry.WizardryItems;
 import electroblob.wizardry.registry.WizardryTabs;
 import net.minecraft.client.model.ModelBiped;
 import net.minecraft.client.util.ITooltipFlag;
@@ -20,9 +21,11 @@ import net.minecraft.entity.SharedMonsterAttributes;
 import net.minecraft.entity.ai.attributes.AttributeModifier;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.inventory.EntityEquipmentSlot;
+import net.minecraft.inventory.Slot;
 import net.minecraft.item.EnumAction;
 import net.minecraft.item.ItemArmor;
 import net.minecraft.item.ItemStack;
+import net.minecraft.nbt.NBTTagCompound;
 import net.minecraft.util.EnumHandSide;
 import net.minecraft.world.World;
 import net.minecraftforge.event.entity.living.LivingEvent.LivingUpdateEvent;
@@ -32,7 +35,7 @@ import net.minecraftforge.fml.relauncher.Side;
 import net.minecraftforge.fml.relauncher.SideOnly;
 
 @Mod.EventBusSubscriber
-public class ItemWizardArmour extends ItemArmor {
+public class ItemWizardArmour extends ItemArmor implements IWorkbenchItem {
 
 	//VanillaCopy, ItemArmor has this set to private for some reason.
     public static final UUID[] ARMOR_MODIFIERS = new UUID[] {UUID.fromString("845DB27C-C624-495F-8C9F-6020A9A58B6B"), UUID.fromString("D8499B04-0E66-4726-AB29-64469D734E0D"), UUID.fromString("9F3D476D-C118-4544-8365-64846904B48E"), UUID.fromString("2AD3F246-FEE1-4E67-B886-69FD380BB150")};
@@ -211,4 +214,52 @@ public class ItemWizardArmour extends ItemArmor {
 		}
 	}
 
+	@Override
+	public int getSpellSlotCount(ItemStack stack){
+		return 0; // Doesn't have any spell slots!
+	}
+
+	@Override
+	public boolean onApplyButtonPressed(EntityPlayer player, Slot centre, Slot crystals, Slot upgrade, Slot[] spellBooks){
+		
+		boolean changed = false;
+		
+		// Applies legendary upgrade
+		if(upgrade.getStack().getItem() == WizardryItems.armour_upgrade){
+			
+			if(!centre.getStack().hasTagCompound()){
+				centre.getStack().setTagCompound(new NBTTagCompound());
+			}
+			
+			if(!centre.getStack().getTagCompound().hasKey("legendary")){
+				
+				centre.getStack().getTagCompound().setBoolean("legendary", true);
+				upgrade.decrStackSize(1);
+				WizardryAdvancementTriggers.legendary.triggerFor(player);
+				changed = true;
+			}
+		}
+		
+		// Charges armour by appropriate amount
+		if(crystals.getStack() != ItemStack.EMPTY){
+			
+			int chargeDepleted = centre.getStack().getItemDamage();
+			
+			if(crystals.getStack().getCount() * Constants.MANA_PER_CRYSTAL < chargeDepleted){
+				
+				centre.getStack().setItemDamage(chargeDepleted - crystals.getStack().getCount() * Constants.MANA_PER_CRYSTAL);
+				crystals.decrStackSize(crystals.getStack().getCount());
+				changed = true;
+				
+			}else if(chargeDepleted != 0){
+
+				centre.getStack().setItemDamage(0);
+				crystals.decrStackSize((int)Math.ceil(((double)chargeDepleted) / Constants.MANA_PER_CRYSTAL));
+				changed = true;
+			}
+		}
+		
+		return changed;
+	}
+
 }

src/main/java/electroblob/wizardry/tileentity/ContainerArcaneWorkbench.java

@@ -3,30 +3,22 @@ package electroblob.wizardry.tileentity;
 import java.util.HashSet;
 import java.util.Set;
 
-import electroblob.wizardry.WizardData;
 import electroblob.wizardry.Wizardry;
-import electroblob.wizardry.constants.Constants;
-import electroblob.wizardry.constants.Tier;
 import electroblob.wizardry.event.SpellBindEvent;
+import electroblob.wizardry.item.IWorkbenchItem;
 import electroblob.wizardry.item.ItemArcaneTome;
 import electroblob.wizardry.item.ItemArmourUpgrade;
 import electroblob.wizardry.item.ItemSpellBook;
-import electroblob.wizardry.item.ItemWand;
-import electroblob.wizardry.item.ItemWizardArmour;
-import electroblob.wizardry.registry.Spells;
-import electroblob.wizardry.registry.WizardryAdvancementTriggers;
 import electroblob.wizardry.registry.WizardryItems;
-import electroblob.wizardry.spell.Spell;
 import electroblob.wizardry.util.WandHelper;
-import electroblob.wizardry.util.WizardryUtilities;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.inventory.Container;
 import net.minecraft.inventory.IInventory;
 import net.minecraft.inventory.Slot;
 import net.minecraft.item.Item;
 import net.minecraft.item.ItemStack;
-import net.minecraft.nbt.NBTTagCompound;
 import net.minecraft.util.ResourceLocation;
+import net.minecraft.util.math.MathHelper;
 import net.minecraftforge.common.MinecraftForge;
 
 public class ContainerArcaneWorkbench extends Container {
@@ -40,29 +32,26 @@ public class ContainerArcaneWorkbench extends Container {
 			"gui/empty_slot_upgrade");
 
 	public static final int CRYSTAL_SLOT = 8;
-	public static final int WAND_SLOT = 9;
+	public static final int CENTRE_SLOT = 9;
 	public static final int UPGRADE_SLOT = 10;
-
+	
-	private static final int[][][] SPELL_BOOK_SLOT_COORDS = {
+	public static final int SLOT_RADIUS = 42;
-			{{80, 22}, {121, 51}, {106, 98}, {54, 98}, {39, 51}, {-999, -999}, {-999, -999}, {-999, -999}},
-			{{80, 22}, {117, 43}, {117, 85}, {80, 106}, {43, 85}, {43, 43}, {-999, -999}, {-999, -999}},
-			{{80, 22}, {113, 38}, {121, 74}, {98, 102}, {62, 102}, {39, 74}, {47, 38}, {-999, -999}},
-			{{80, 22}, {111, 33}, {122, 64}, {111, 95}, {80, 106}, {49, 95}, {38, 64}, {49, 33}}};
 
 	public ContainerArcaneWorkbench(IInventory inventory, TileEntityArcaneWorkbench tileentity){
 
 		this.tileentity = tileentity;
 
-		ItemStack wand = tileentity.getStackInSlot(WAND_SLOT);
+		ItemStack wand = tileentity.getStackInSlot(CENTRE_SLOT);
 
 		for(int i = 0; i < 8; i++){
-			this.addSlotToContainer(new SlotItemList(tileentity, i, -999, -999, 1, WizardryItems.spell_book));
+			Slot slot = new SlotItemList(tileentity, i, -999, -999, 1, WizardryItems.spell_book);
+			this.addSlotToContainer(slot);
 		}
 
 		this.addSlotToContainer(new SlotItemList(tileentity, CRYSTAL_SLOT, 8, 88, 64, WizardryItems.magic_crystal))
 				.setBackgroundName(EMPTY_SLOT_CRYSTAL.toString());
 
-		this.addSlotToContainer(new SlotWandArmour(tileentity, WAND_SLOT, 80, 64, this));
+		this.addSlotToContainer(new SlotWorkbenchItem(tileentity, CENTRE_SLOT, 80, 64, this));
 
 		Set<Item> upgrades = new HashSet<Item>(WandHelper.getSpecialUpgrades()); // Can't be done statically.
 		upgrades.add(WizardryItems.arcane_tome);
@@ -81,102 +70,98 @@ public class ContainerArcaneWorkbench extends Container {
 			}
 		}
 
-		this.onSlotChanged(WAND_SLOT, wand, null);
+		this.onSlotChanged(CENTRE_SLOT, wand, null);
 	}
 
 	@Override
 	public boolean canInteractWith(EntityPlayer player){
 		return this.tileentity.isUsableByPlayer(player);
 	}
+	
+	/**
+	 * Shows the given slot in the container GUI at the given position. Intended to do the opposite of
+	 * {@link ContainerArcaneWorkbench#hideSlot(int, EntityPlayer)}.
+	 * @param index The index of the slot to show.
+	 * @param x The x position to put the slot in.
+	 * @param y The y position to put the slot in.
+	 */
+	private void showSlot(int index, int x, int y){
+		
+		Slot slot = this.getSlot(index);
+		slot.xPos = x;
+		slot.yPos = y;
+	}
+	
+	/**
+	 * Hides the given slot from the container GUI (moves it off the screen) and returns its contents to the given
+	 * player. If some or all of the items do not fit in the player's inventory, or if the player is null, they are
+	 * dropped on the floor.
+	 * @param index The index of the slot to hide.
+	 * @param player The player that is using this container.
+	 */
+	private void hideSlot(int index, EntityPlayer player){
+		
+		Slot slot = this.getSlot(index);
+		
+		// 'Removes' the slot from the container (moves it off the screen)
+		slot.xPos = -999;
+		slot.yPos = -999;
+
+		ItemStack stack = slot.getStack();
+		// This doesn't cause an infinite loop because slot i can never be a SlotWandArmour. In effect, it's
+		// exactly the same as shift-clicking the slot, so why re-invent the wheel?
+		ItemStack remainder = this.transferStackInSlot(player, index);
+
+		if(remainder == ItemStack.EMPTY && stack != ItemStack.EMPTY){
+			slot.putStack(ItemStack.EMPTY);
+			// The second parameter is never used...
+			if(player != null) player.dropItem(stack, false);
+		}
+	}
 
 	/** Called from the central wand/armour slot when its item is changed or removed. */
 	// In case I forget again and think it should have @Override: I wrote this!
 	public void onSlotChanged(int slotNumber, ItemStack stack, EntityPlayer player){
 
-		if(slotNumber == WAND_SLOT){
+		if(slotNumber == CENTRE_SLOT){
 
-			if(!(stack.getItem() instanceof ItemWand) && stack.getItem() != WizardryItems.blank_scroll){
+			if(stack.isEmpty()){
-				// If the stack has been removed
+				// If the stack has been removed, hide all the spell book slots
 				for(int i = 0; i < CRYSTAL_SLOT; i++){
-					Slot slot1 = this.getSlot(i);
+					this.hideSlot(i, player);
-					// 'Removes' the slot from the container (moves it off the screen)
-					slot1.xPos = -100;
-					slot1.yPos = -100;
-
-					ItemStack stack1 = slot1.getStack();
-					// This doesn't cause an infinite loop because slot i can never be a SlotWandArmour. In effect, it's
-					// exactly the same as shift-clicking the slot, so why re-invent the wheel?
-					ItemStack remainder = this.transferStackInSlot(player, i);
-
-					if(remainder == ItemStack.EMPTY && stack1 != ItemStack.EMPTY){
-						slot1.putStack(ItemStack.EMPTY);
-						// The second parameter is never used...
-						if(player != null) player.dropItem(stack1, false);
-					}
 				}
 
 			}else{
-
+				
-				if(stack.getItem() == WizardryItems.blank_scroll){
+				if(stack.getItem() instanceof IWorkbenchItem){ // Should always be true here.
-					// If a blank scroll is added
+					
-					// The first slot is shown
+					int spellSlots = ((IWorkbenchItem)stack.getItem()).getSpellSlotCount(stack);
-					this.getSlot(0).xPos = SPELL_BOOK_SLOT_COORDS[0][0][0];
+					
-					this.getSlot(0).yPos = SPELL_BOOK_SLOT_COORDS[0][0][1];
+					int centreX = this.getSlot(CENTRE_SLOT).xPos;
-
+					int centreY = this.getSlot(CENTRE_SLOT).yPos;
-					// The rest of the slots are hidden
+					
-					for(int i = 1; i < CRYSTAL_SLOT; i++){
+					// Show however many spell book slots are necessary
-
+					for(int i = 0; i < spellSlots; i++){
-						Slot slot1 = this.getSlot(i);
+						
-
+						float angle = i * (2 * (float)Math.PI)/spellSlots;
-						slot1.xPos = -100;
+						int x = centreX + (int)(Math.round(SLOT_RADIUS * MathHelper.sin(angle)));
-						slot1.yPos = -100;
+						// -cos because +y is downwards
-
+						int y = centreY + (int)(Math.round(SLOT_RADIUS * -MathHelper.cos(angle)));
-						ItemStack stack1 = slot1.getStack();
+						
-						// This doesn't cause an infinite loop because slot i can never be a SlotWandArmour. In effect,
+						showSlot(i, x, y);
-						// it's
-						// exactly the same as shift-clicking the slot, so why re-invent the wheel?
-						ItemStack remainder = this.transferStackInSlot(player, i);
-
-						if(remainder == ItemStack.EMPTY && stack1 != ItemStack.EMPTY){
-							slot1.putStack(ItemStack.EMPTY);
-							// The second parameter is never used...
-							if(player != null) player.dropItem(stack1, false);
-						}
 					}
-
+					
-				}else{
+					// Hide the rest
-
+					for(int i = spellSlots; i < CRYSTAL_SLOT; i++){
-					for(int i = 0; i < CRYSTAL_SLOT; i++){
+						hideSlot(i, player);
-
-						int n = WandHelper.getUpgradeLevel(stack, WizardryItems.attunement_upgrade);
-						int[] coords = SPELL_BOOK_SLOT_COORDS[n][i];
-
-						Slot slot1 = this.getSlot(i);
-						// Puts the slot back in the correct position
-						slot1.xPos = coords[0];
-						slot1.yPos = coords[1];
-
-						if(slot1.xPos < 0 || slot1.yPos < 0){
-
-							ItemStack stack1 = slot1.getStack();
-							// This doesn't cause an infinite loop because slot i can never be a SlotWandArmour. In
-							// effect, it's
-							// exactly the same as shift-clicking the slot, so why re-invent the wheel?
-							ItemStack remainder = this.transferStackInSlot(player, i);
-
-							if(remainder == ItemStack.EMPTY && stack1 != ItemStack.EMPTY){
-								slot1.putStack(ItemStack.EMPTY);
-								// The second parameter is never used...
-								if(player != null) player.dropItem(stack1, false);
-							}
-						}
 					}
+					
 				}
 			}
 		}
 
 		// FIXME: It only seems to be syncing correctly when a stack is put into the slot, not taken out.
-		// Was this broken in 1.7.10 as well?
+		// This is because markDirty isn't called in the tileentity, I think.
 		this.tileentity.sync();
 	}
 
@@ -209,17 +194,16 @@ public class ContainerArcaneWorkbench extends Container {
 				}else if(stack.getItem() == WizardryItems.magic_crystal){
 					minSlotId = CRYSTAL_SLOT;
 					maxSlotId = CRYSTAL_SLOT;
-				}else if(stack.getItem() instanceof ItemWand || stack.getItem() instanceof ItemWizardArmour
+				}else if(stack.getItem() instanceof IWorkbenchItem){
-						|| stack.getItem() == WizardryItems.blank_scroll){
+					minSlotId = CENTRE_SLOT;
-					minSlotId = WAND_SLOT;
+					maxSlotId = CENTRE_SLOT;
-					maxSlotId = WAND_SLOT;
 				}else if(stack.getItem() instanceof ItemArcaneTome || stack.getItem() instanceof ItemArmourUpgrade
 						|| WandHelper.isWandUpgrade(stack.getItem())){
 					minSlotId = UPGRADE_SLOT;
 					maxSlotId = UPGRADE_SLOT;
 				}else{
-					return ItemStack.EMPTY; // If none of the above cases were true, then the item won't fit in the
+					// If none of the above cases were true, then the item won't fit in the workbench.
-											// workbench.
+					return ItemStack.EMPTY;
 				}
 
 				if(!this.mergeItemStack(stack, minSlotId, maxSlotId + 1, false)){
@@ -261,204 +245,22 @@ public class ContainerArcaneWorkbench extends Container {
 	 * Called (via {@link electroblob.wizardry.packet.PacketControlInput PacketControlInput}) when the apply button in
 	 * the arcane workbench GUI is pressed.
 	 */
-	// All operations on the items contained in the inventory simply call the corresponding methods in the tileentity.
 	// As of 2.1, for the sake of events and neatness of code, this was moved here from TileEntityArcaneWorkbench.
+	// As of 4.2, the spell binding/charging/upgrading code was delegated (via IWorkbenchItem) to the items themselves.
 	public void onApplyButtonPressed(EntityPlayer player){
 
-		ItemStack wand = this.getSlot(WAND_SLOT).getStack();
-		ItemStack[] spellBooks = new ItemStack[CRYSTAL_SLOT];
-		for(int i = 0; i < spellBooks.length; i++){
-			spellBooks[i] = this.getSlot(i).getStack();
-		}
-		ItemStack crystals = this.getSlot(CRYSTAL_SLOT).getStack();
-		ItemStack upgrade = this.getSlot(UPGRADE_SLOT).getStack();
-
 		if(MinecraftForge.EVENT_BUS.post(new SpellBindEvent(player, this))) return;
-
+		
-		// Since the workbench now accepts armour as well as wands, this check is needed.
+		Slot centre = this.getSlot(CENTRE_SLOT);
-		if(wand.getItem() instanceof ItemWand){
+		
-
+		if(centre.getStack().getItem() instanceof IWorkbenchItem){ // Should always be true, but no harm in checking.
-			// Upgrades wand if necessary. Damage is copied, preserving remaining durability,
+			
-			// and also the entire NBT tag compound.
+			Slot[] spellBooks = this.inventorySlots.subList(0, 8).toArray(new Slot[8]);
-			if(upgrade.getItem() == WizardryItems.arcane_tome){
+			
-
+			if(((IWorkbenchItem)centre.getStack().getItem())
-				ItemStack newWand;
+				.onApplyButtonPressed(player, centre, this.getSlot(CRYSTAL_SLOT), this.getSlot(UPGRADE_SLOT), spellBooks)){
-
+				
-				switch(Tier.values()[upgrade.getItemDamage()]){
+				// TODO: Sound and possibly animation for spell binding
-
-				case APPRENTICE:
-					if(((ItemWand)wand.getItem()).tier == Tier.BASIC){
-						newWand = new ItemStack(WizardryUtilities.getWand(Tier.values()[upgrade.getItemDamage()],
-								((ItemWand)wand.getItem()).element));
-						newWand.setTagCompound(wand.getTagCompound());
-						// This needs to be done after copying the tag compound so the max damage for the new wand
-						// takes storage
-						// upgrades into account.
-						newWand.setItemDamage(newWand.getMaxDamage() - (wand.getMaxDamage() - wand.getItemDamage()));
-						this.putStackInSlot(WAND_SLOT, newWand);
-						this.putStackInSlot(UPGRADE_SLOT, ItemStack.EMPTY);
-						WizardryAdvancementTriggers.apprentice.triggerFor(player);
-					}
-					break;
-
-				case ADVANCED:
-					if(((ItemWand)wand.getItem()).tier == Tier.APPRENTICE){
-						newWand = new ItemStack(WizardryUtilities.getWand(Tier.values()[upgrade.getItemDamage()],
-								((ItemWand)wand.getItem()).element));
-						newWand.setTagCompound(wand.getTagCompound());
-						newWand.setItemDamage(newWand.getMaxDamage() - (wand.getMaxDamage() - wand.getItemDamage()));
-						this.putStackInSlot(WAND_SLOT, newWand);
-						this.putStackInSlot(UPGRADE_SLOT, ItemStack.EMPTY);
-					}
-					break;
-
-				case MASTER:
-					if(((ItemWand)wand.getItem()).tier == Tier.ADVANCED){
-						newWand = new ItemStack(WizardryUtilities.getWand(Tier.values()[upgrade.getItemDamage()],
-								((ItemWand)wand.getItem()).element));
-						newWand.setTagCompound(wand.getTagCompound());
-						newWand.setItemDamage(newWand.getMaxDamage() - (wand.getMaxDamage() - wand.getItemDamage()));
-						this.putStackInSlot(WAND_SLOT, newWand);
-						this.putStackInSlot(UPGRADE_SLOT, ItemStack.EMPTY);
-						WizardryAdvancementTriggers.master.triggerFor(player);
-					}
-					break;
-
-				default:
-					break;
-				}
-
-				// This needs to happen so the charging works on the new wand, not the old one.
-				wand = this.getSlot(WAND_SLOT).getStack();
-
-			}else if(WandHelper.isWandUpgrade(upgrade.getItem())){
-
-				// Special upgrades
-
-				// Used to preserve existing mana when upgrading storage rather than creating free mana.
-				int prevMana = wand.getMaxDamage() - wand.getItemDamage();
-
-				if(WandHelper.getTotalUpgrades(wand) < ((ItemWand)wand.getItem()).tier.upgradeLimit
-						&& WandHelper.getUpgradeLevel(wand, upgrade.getItem()) < Constants.UPGRADE_STACK_LIMIT){
-
-					WandHelper.applyUpgrade(wand, upgrade.getItem());
-
-					// Special behaviours for specific upgrades
-					if(upgrade.getItem() == WizardryItems.storage_upgrade){
-						wand.setItemDamage(wand.getMaxDamage() - prevMana);
-					}
-					if(upgrade.getItem() == WizardryItems.attunement_upgrade){
-
-						Spell[] spells = WandHelper.getSpells(wand);
-						Spell[] newSpells = new Spell[5
-								+ WandHelper.getUpgradeLevel(wand, WizardryItems.attunement_upgrade)];
-
-						for(int i = 0; i < newSpells.length; i++){
-							// Prevents both NPEs and AIOOBEs
-							newSpells[i] = i < spells.length && spells[i] != null ? spells[i] : Spells.none;
-						}
-
-						WandHelper.setSpells(wand, newSpells);
-
-						int[] cooldown = WandHelper.getCooldowns(wand);
-						int[] newCooldown = new int[5
-								+ WandHelper.getUpgradeLevel(wand, WizardryItems.attunement_upgrade)];
-
-						if(cooldown.length > 0){
-							for(int i = 0; i < cooldown.length; i++){
-								newCooldown[i] = cooldown[i];
-							}
-						}
-
-						WandHelper.setCooldowns(wand, newCooldown);
-					}
-
-					this.getSlot(UPGRADE_SLOT).decrStackSize(1);
-					WizardryAdvancementTriggers.special_upgrade.triggerFor(player);
-
-					if(WandHelper.getTotalUpgrades(wand) == Tier.MASTER.upgradeLimit){
-						WizardryAdvancementTriggers.max_out_wand.triggerFor(player);
-					}
-				}
-			}
-
-			// Reads NBT spell id array to variable, edits this, then writes it back to NBT.
-			// Original spells are preserved; if a slot is left empty the existing spell binding will remain.
-			// Accounts for spells which cannot be applied because they are above the wand's tier; these spells
-			// will not bind but the existing spell in that slot will remain and other applicable spells will
-			// be bound as normal, along with any upgrades and crystals.
-			Spell[] spells = WandHelper.getSpells(wand);
-			if(spells.length <= 0){
-				// 5 here because if the spell array doesn't exist, the wand can't possibly have attunement upgrades
-				spells = new Spell[5];
-			}
-			for(int i = 0; i < spells.length; i++){
-				if(spellBooks[i] != ItemStack.EMPTY && !(Spell
-						.get(spellBooks[i].getItemDamage()).tier.level > ((ItemWand)wand.getItem()).tier.level)){
-					spells[i] = Spell.get(spellBooks[i].getItemDamage());
-				}
-			}
-			WandHelper.setSpells(wand, spells);
-
-			// Charges wand by appropriate amount
-			if(crystals != ItemStack.EMPTY){
-				int chargeDepleted = wand.getItemDamage();
-				// System.out.println("Charge depleted: " + chargeDepleted);
-				// System.out.println("Crystals found: " + crystals.getCount());
-				if(crystals.getCount() * Constants.MANA_PER_CRYSTAL < chargeDepleted){
-					// System.out.println("charging");
-					wand.setItemDamage(chargeDepleted - crystals.getCount() * Constants.MANA_PER_CRYSTAL);
-					this.getSlot(CRYSTAL_SLOT).decrStackSize(crystals.getCount());
-				}else if(chargeDepleted != 0){
-					// System.out.println((int)Math.ceil(((double)chargeDepleted)/50));
-					this.getSlot(CRYSTAL_SLOT)
-							.decrStackSize((int)Math.ceil(((double)chargeDepleted) / Constants.MANA_PER_CRYSTAL));
-					wand.setItemDamage(0);
-				}
-			}
-		}
-
-		// Armour
-		else if(wand.getItem() instanceof ItemWizardArmour){
-			// Applies legendary upgrade
-			if(upgrade.getItem() == WizardryItems.armour_upgrade){
-				if(!wand.hasTagCompound()){
-					wand.setTagCompound(new NBTTagCompound());
-				}
-				if(!wand.getTagCompound().hasKey("legendary")){
-					wand.getTagCompound().setBoolean("legendary", true);
-					this.putStackInSlot(UPGRADE_SLOT, ItemStack.EMPTY);
-					WizardryAdvancementTriggers.legendary.triggerFor(player);
-				}
-			}
-			// Charges armour by appropriate amount
-			if(crystals != ItemStack.EMPTY){
-				int chargeDepleted = wand.getItemDamage();
-				if(crystals.getCount() * Constants.MANA_PER_CRYSTAL < chargeDepleted){
-					wand.setItemDamage(chargeDepleted - crystals.getCount() * Constants.MANA_PER_CRYSTAL);
-					this.getSlot(CRYSTAL_SLOT).decrStackSize(crystals.getCount());
-				}else if(chargeDepleted != 0){
-					this.getSlot(CRYSTAL_SLOT)
-							.decrStackSize((int)Math.ceil(((double)chargeDepleted) / Constants.MANA_PER_CRYSTAL));
-					wand.setItemDamage(0);
-				}
-			}
-		}
-
-		// Scrolls
-		else if(wand.getItem() == WizardryItems.blank_scroll){
-			// Spells can only be bound to scrolls if the player has already cast them (prevents casting of master
-			// spells without getting a master wand)
-			// This restriction does not apply in creative mode
-			if(spellBooks[0] != ItemStack.EMPTY
-					&& (player.capabilities.isCreativeMode || (WizardData.get(player) != null
-							&& WizardData.get(player).hasSpellBeenDiscovered(Spell.get(spellBooks[0].getItemDamage()))))
-					&& crystals != ItemStack.EMPTY && crystals.getCount()
-							* Constants.MANA_PER_CRYSTAL > Spell.get(spellBooks[0].getItemDamage()).cost){
-
-				this.getSlot(CRYSTAL_SLOT).decrStackSize((int)Math
-						.ceil(((double)Spell.get(spellBooks[0].getItemDamage()).cost) / Constants.MANA_PER_CRYSTAL));
-				this.putStackInSlot(WAND_SLOT, new ItemStack(WizardryItems.scroll, 1, spellBooks[0].getItemDamage()));
 			}
 		}
 	}

src/main/java/electroblob/wizardry/tileentity/SlotWorkbenchItem.java

@@ -1,8 +1,6 @@
 package electroblob.wizardry.tileentity;
 
-import electroblob.wizardry.item.ItemWand;
+import electroblob.wizardry.item.IWorkbenchItem;
-import electroblob.wizardry.item.ItemWizardArmour;
-import electroblob.wizardry.registry.WizardryItems;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.inventory.IInventory;
 import net.minecraft.inventory.Slot;
@@ -14,12 +12,12 @@ import net.minecraft.item.ItemStack;
  * @author Electroblob
  * @since Wizardry 1.0
  */
-public class SlotWandArmour extends Slot {
+public class SlotWorkbenchItem extends Slot {
 
 	private ContainerArcaneWorkbench container;
 
-	public SlotWandArmour(IInventory par1iInventory, int index, int x, int y, ContainerArcaneWorkbench container){
+	public SlotWorkbenchItem(IInventory inventory, int index, int x, int y, ContainerArcaneWorkbench container){
-		super(par1iInventory, index, x, y);
+		super(inventory, index, x, y);
 		this.container = container;
 	}
 
@@ -41,8 +39,7 @@ public class SlotWandArmour extends Slot {
 	}
 
 	@Override
-	public boolean isItemValid(ItemStack itemstack){
+	public boolean isItemValid(ItemStack stack){
-		return (itemstack.getItem() instanceof ItemWand || itemstack.getItem() instanceof ItemWizardArmour
+		return stack.getItem() instanceof IWorkbenchItem && ((IWorkbenchItem)stack.getItem()).canPlace(stack);
-				|| itemstack.getItem() == WizardryItems.blank_scroll);
 	}
 }

src/main/java/electroblob/wizardry/tileentity/TileEntityArcaneWorkbench.java

@@ -56,7 +56,7 @@ public class TileEntityArcaneWorkbench extends TileEntity implements IInventory,
 	@Override
 	public void update(){
 
-		ItemStack itemstack = this.getStackInSlot(ContainerArcaneWorkbench.WAND_SLOT);
+		ItemStack itemstack = this.getStackInSlot(ContainerArcaneWorkbench.CENTRE_SLOT);
 
 		// Decrements wand damage (increases mana) every 1.5 seconds if it has a condenser upgrade
 		if(itemstack.getItem() instanceof ItemWand && !this.world.isRemote && itemstack.isItemDamaged()
@@ -93,33 +93,42 @@ public class TileEntityArcaneWorkbench extends TileEntity implements IInventory,
 	}
 
 	@Override
-	public ItemStack decrStackSize(int slot, int amt){
+	public ItemStack decrStackSize(int slot, int amount){
+		
 		ItemStack stack = getStackInSlot(slot);
+		
 		if(!stack.isEmpty()){
-			if(stack.getCount() <= amt){
+			if(stack.getCount() <= amount){
 				setInventorySlotContents(slot, ItemStack.EMPTY);
 			}else{
-				stack = stack.splitStack(amt);
+				stack = stack.splitStack(amount);
 				if(stack.getCount() == 0){
 					setInventorySlotContents(slot, ItemStack.EMPTY);
 				}
 			}
+			this.markDirty();
 		}
+		
 		return stack;
 	}
 
 	@Override
 	public ItemStack removeStackFromSlot(int slot){
+		
 		ItemStack stack = getStackInSlot(slot);
+		
 		if(!stack.isEmpty()){
 			setInventorySlotContents(slot, ItemStack.EMPTY);
 		}
+		
 		return stack;
 	}
 
 	@Override
 	public void setInventorySlotContents(int slot, ItemStack stack){
+		
 		inventory.set(slot, stack);
+		
 		if(!stack.isEmpty() && stack.getCount() > getInventoryStackLimit()){
 			stack.setCount(getInventoryStackLimit());
 		}
@@ -166,7 +175,7 @@ public class TileEntityArcaneWorkbench extends TileEntity implements IInventory,
 		}else if(slotNumber == ContainerArcaneWorkbench.CRYSTAL_SLOT){
 			return itemstack.getItem() == WizardryItems.magic_crystal;
 
-		}else if(slotNumber == ContainerArcaneWorkbench.WAND_SLOT){
+		}else if(slotNumber == ContainerArcaneWorkbench.CENTRE_SLOT){
 			return (itemstack.getItem() instanceof ItemWand || itemstack.getItem() instanceof ItemWizardArmour
 					|| itemstack.getItem() == WizardryItems.blank_scroll);