Minor cleanup of pocket computer upgrades

- General improvements of the docs.
 - Move all ItemStack code to the ItemPocketComputer class
 - Make PocketAPI execute on the server thread, rather than the computer
   thread

src/main/java/dan200/computercraft/api/pocket/IPocketAccess.java

@@ -15,25 +15,25 @@
 public interface IPocketAccess
 {
     /**
-     * Gets the holding entity of this item
+     * Gets the entity holding this item.
      *
-     * @return The holding entity, may be {@code null}.
+     * @return The holding entity. This may be {@code null}.
      */
     @Nullable
     Entity getEntity();
 
     /**
-     * Get the colour of the modem light.
+     * Get the colour of the pocket computer's light.
      *
      * See {@link #setLight(int)} for the values this may return.
      *
-     * @return The colour of the modem light.
+     * @return The colour of the pocket computer's light.
      * @see #setLight(int)
      */
     int getLight();
 
     /**
-     * Set the colour of the modem light. Use {@link 0} to turn it off.
+     * Set the colour of the pocket computer's light. Use {@link 0} to turn it off.
      *
      * Colours take the form of an integer between 0 and 15, using the opposite order to those in
      * {@link <a href="http://www.computercraft.info/wiki/Colors_(API)#Colors">The colors API</a>}  - so 0 being black,
@@ -45,31 +45,32 @@ public interface IPocketAccess
     void setLight( int value );
 
     /**
-     * Get the upgrade specific NBT
+     * Get the upgrade-specific NBT.
      *
-     * @return The upgrade's NBT
+     * This is persisted between computer reboots and chunk loads.
+     *
+     * @return The upgrade's NBT.
      * @see #updateUpgradeNBTData()
      */
     @Nonnull
     NBTTagCompound getUpgradeNBTData();
 
     /**
-     * Mark the upgrade specific NBT as dirty
+     * Mark the upgrade-specific NBT as dirty.
      *
      * @see #getUpgradeNBTData()
      */
     void updateUpgradeNBTData();
 
     /**
-     * Remove the current peripheral and create a new one. You
-     * may wish to do this if the methods available change.
+     * Remove the current peripheral and create a new one. You may wish to do this if the methods available change.
      */
     void invalidatePeripheral();
 
     /**
-     * Get a list of all upgrades for the pocket computer
+     * Get a list of all upgrades for the pocket computer.
      *
-     * @return A collection of all upgrade names
+     * @return A collection of all upgrade names.
      */
     @Nonnull
     Map<ResourceLocation, IPeripheral> getUpgrades();

src/main/java/dan200/computercraft/api/pocket/IPocketUpgrade.java

@@ -2,9 +2,7 @@
 
 import dan200.computercraft.api.ComputerCraftAPI;
 import dan200.computercraft.api.peripheral.IPeripheral;
-import dan200.computercraft.api.turtle.ITurtleAccess;
 import dan200.computercraft.api.turtle.ITurtleUpgrade;
-import dan200.computercraft.api.turtle.TurtleSide;
 import net.minecraft.item.ItemStack;
 import net.minecraft.util.ResourceLocation;
 import net.minecraft.world.World;
@@ -14,18 +12,20 @@
 
 /**
  * Additional peripherals for pocket computers.
- * <p>
+ *
  * This is similar to {@link dan200.computercraft.api.turtle.ITurtleUpgrade}.
  */
 public interface IPocketUpgrade
 {
 
     /**
-     * Gets a unique identifier representing this type of turtle upgrade. eg: "computercraft:wireless_modem" or "my_mod:my_upgrade".
-     * You should use a unique resource domain to ensure this upgrade is uniquely identified.
-     * The peripheral will fail registration if an already used ID is specified.
+     * Gets a unique identifier representing this type of turtle upgrade. eg: "computercraft:wireless_modem" or
+     * "my_mod:my_upgrade".
      *
-     * @return The id
+     * You should use a unique resource domain to ensure this upgrade is uniquely identified. The upgrade will fail
+     * registration if an already used ID is specified.
+     *
+     * @return The upgrade's id.
      * @see IPocketUpgrade#getUpgradeID()
      * @see ComputerCraftAPI#registerPocketUpgrade(IPocketUpgrade)
      */
@@ -33,54 +33,59 @@ public interface IPocketUpgrade
     ResourceLocation getUpgradeID();
 
     /**
-     * Return a String to describe this type of turtle in turtle item names.
-     * An example of built-in adjectives is "Wireless".
+     * Return an unlocalised string to describe the type of pocket computer this upgrade provides.
      *
-     * @return The unlocalised adjective
+     * An example of a built-in adjectives is "Wireless" - this is converted to "Wireless Pocket Computer".
+     *
+     * @return The unlocalised adjective.
      * @see ITurtleUpgrade#getUnlocalisedAdjective()
      */
     @Nonnull
     String getUnlocalisedAdjective();
 
     /**
-     * Return an item stack representing the type of item that a turtle must be crafted
-     * with to create a turtle which holds this upgrade. This item stack is also used
-     * to determine the upgrade given by turtle.equip()
+     * Return an item stack representing the type of item that a pocket computer must be crafted with to create a
+     * pocket computer which holds this upgrade. This item stack is also used to determine the upgrade given by
+     * {@code pocket.equip()}/{@code pocket.unequip()}.
      *
      * @return The item stack used for crafting. This can be {@code null} if crafting is disabled.
-     * @see ITurtleUpgrade#getCraftingItem()
      */
     @Nullable
     ItemStack getCraftingItem();
 
     /**
      * Creates a peripheral for the pocket computer.
-     * <p>
-     * The peripheral created will be stored for the lifetime of the upgrade, will have update() called
-     * once-per-tick, and will be attached, detached and have methods called in the same manner as a Computer peripheral.
      *
-     * @param access The access object for the pocket item stack
+     * The peripheral created will be stored for the lifetime of the upgrade, will be passed an argument to
+     * {@link #update(IPocketAccess, IPeripheral)} and will be attached, detached and have methods called in the same
+     * manner as an ordinary peripheral.
+     *
+     * @param access The access object for the pocket item stack.
      * @return The newly created peripheral.
-     * @see ITurtleUpgrade#createPeripheral(ITurtleAccess, TurtleSide)
+     * @see #update(IPocketAccess, IPeripheral)
      */
     @Nullable
     IPeripheral createPeripheral( @Nonnull IPocketAccess access );
 
     /**
-     * Called when the pocket computer item stack updates
+     * Called when the pocket computer item stack updates.
      *
-     * @param access     The access object for the pocket item stack
-     * @param peripheral The peripheral for this upgrade
+     * @param access     The access object for the pocket item stack.
+     * @param peripheral The peripheral for this upgrade.
+     * @see #createPeripheral(IPocketAccess)
      */
     void update( @Nonnull IPocketAccess access, @Nullable IPeripheral peripheral );
 
     /**
-     * Called when the pocket computer is right clicked on something
+     * Called when the pocket computer is right clicked.
      *
-     * @param world      The world the computer is in
-     * @param access     The access object for the pocket item stack
-     * @param peripheral The peripheral for this upgrade
-     * @return {@code true} to stop the gui from opening, otherwise false.
+     * @param world      The world the computer is in.
+     * @param access     The access object for the pocket item stack.
+     * @param peripheral The peripheral for this upgrade.
+     * @return {@code true} to stop the GUI from opening, otherwise false. You should always provide some code path
+     * which returns {@code false}, such as requiring the player to be sneaking - otherwise they will be unable to
+     * access the GUI.
+     * @see #createPeripheral(IPocketAccess)
      */
     boolean onRightClick( @Nonnull World world, @Nonnull IPocketAccess access, @Nullable IPeripheral peripheral );
 }

src/main/java/dan200/computercraft/shared/pocket/apis/PocketAPI.java

@@ -8,11 +8,11 @@
 
 import dan200.computercraft.ComputerCraft;
 import dan200.computercraft.api.lua.ILuaContext;
+import dan200.computercraft.api.lua.ILuaTask;
 import dan200.computercraft.api.lua.LuaException;
 import dan200.computercraft.api.pocket.IPocketUpgrade;
 import dan200.computercraft.core.apis.ILuaAPI;
 import dan200.computercraft.shared.pocket.core.PocketServerComputer;
-import dan200.computercraft.shared.pocket.items.ItemPocketComputer;
 import dan200.computercraft.shared.util.InventoryUtil;
 import dan200.computercraft.shared.util.WorldUtil;
 import net.minecraft.entity.player.EntityPlayer;
@@ -61,84 +61,91 @@ public String[] getMethodNames()
     }
 
     @Override
-    public Object[] callMethod( ILuaContext context, int method, Object[] arguments ) throws LuaException
+    public Object[] callMethod( ILuaContext context, int method, Object[] arguments ) throws LuaException, InterruptedException
     {
         switch( method )
         {
             case 0:
-            {
                 // equipBack
-                if( !(m_computer.getEntity() instanceof EntityPlayer) )
+                return context.executeMainThreadTask( new ILuaTask()
                 {
-                    throw new LuaException( "Cannot find player" );
-                }
-
-                ItemStack pocketStack = m_computer.getStack();
-                EntityPlayer player = (EntityPlayer) m_computer.getEntity();
-                InventoryPlayer inventory = player.inventory;
-
-                IPocketUpgrade previousUpgrade = m_computer.getUpgrade();
-
-                // Attempt to find the upgrade, starting in the main segment, and then looking in the opposite
-                // one. We start from the position the item is currently in and loop round to the start.
-                IPocketUpgrade newUpgrade = findUpgrade( inventory.mainInventory, inventory.currentItem, previousUpgrade );
-                if( newUpgrade == null ) newUpgrade = findUpgrade( inventory.offHandInventory, 0, previousUpgrade );
-                if( newUpgrade == null ) throw new LuaException( "Cannot find a valid upgrade" );
-
-                // Remove the current upgrade
-                if( previousUpgrade != null )
-                {
-                    ItemStack stack = previousUpgrade.getCraftingItem();
-                    if( stack != null )
+                    @Override
+                    public Object[] execute() throws LuaException
                     {
-                        stack = InventoryUtil.storeItems( stack, inventory, 0, 36, inventory.currentItem );
-                        if( stack != null )
+                        if( !(m_computer.getEntity() instanceof EntityPlayer) )
                         {
-                            WorldUtil.dropItemStack( stack, player.worldObj, player.posX, player.posY, player.posZ );
+                            throw new LuaException( "Cannot find player" );
                         }
+
+                        EntityPlayer player = (EntityPlayer) m_computer.getEntity();
+                        InventoryPlayer inventory = player.inventory;
+
+                        IPocketUpgrade previousUpgrade = m_computer.getUpgrade();
+
+                        // Attempt to find the upgrade, starting in the main segment, and then looking in the opposite
+                        // one. We start from the position the item is currently in and loop round to the start.
+                        IPocketUpgrade newUpgrade = findUpgrade( inventory.mainInventory, inventory.currentItem, previousUpgrade );
+                        if( newUpgrade == null )
+                        {
+                            newUpgrade = findUpgrade( inventory.offHandInventory, 0, previousUpgrade );
+                        }
+                        if( newUpgrade == null ) throw new LuaException( "Cannot find a valid upgrade" );
+
+                        // Remove the current upgrade
+                        if( previousUpgrade != null )
+                        {
+                            ItemStack stack = previousUpgrade.getCraftingItem();
+                            if( stack != null )
+                            {
+                                stack = InventoryUtil.storeItems( stack, inventory, 0, 36, inventory.currentItem );
+                                if( stack != null )
+                                {
+                                    WorldUtil.dropItemStack( stack, player.worldObj, player.posX, player.posY, player.posZ );
+                                }
+                            }
+                        }
+
+                        // Set the new upgrade
+                        m_computer.setUpgrade( newUpgrade );
+
+                        return null;
                     }
-                }
-
-                // Set the new upgrade
-                ItemPocketComputer.setUpgrade( pocketStack, newUpgrade );
-                m_computer.setUpgrade( newUpgrade );
-
-                inventory.markDirty();
-
-                return null;
-            }
+                } );
 
             case 1:
-            {
                 // unequipBack
-                if( !(m_computer.getEntity() instanceof EntityPlayer) )
+                return context.executeMainThreadTask( new ILuaTask()
                 {
-                    throw new LuaException( "Cannot find player" );
-                }
-
-                ItemStack pocketStack = m_computer.getStack();
-                EntityPlayer player = (EntityPlayer) m_computer.getEntity();
-                InventoryPlayer inventory = player.inventory;
-
-                IPocketUpgrade previousUpgrade = m_computer.getUpgrade();
-
-                if( previousUpgrade == null ) throw new LuaException( "Nothing to unequip" );
-
-                ItemPocketComputer.setUpgrade( pocketStack, null );
-                m_computer.setUpgrade( null );
-
-                ItemStack stack = previousUpgrade.getCraftingItem();
-                if( stack != null )
-                {
-                    stack = InventoryUtil.storeItems( stack, inventory, 0, 36, inventory.currentItem );
-                    if( stack != null )
+                    @Override
+                    public Object[] execute() throws LuaException
                     {
-                        WorldUtil.dropItemStack( stack, player.worldObj, player.posX, player.posY, player.posZ );
-                    }
-                }
+                        if( !(m_computer.getEntity() instanceof EntityPlayer) )
+                        {
+                            throw new LuaException( "Cannot find player" );
+                        }
 
-                return null;
-            }
+                        EntityPlayer player = (EntityPlayer) m_computer.getEntity();
+                        InventoryPlayer inventory = player.inventory;
+
+                        IPocketUpgrade previousUpgrade = m_computer.getUpgrade();
+
+                        if( previousUpgrade == null ) throw new LuaException( "Nothing to unequip" );
+
+                        m_computer.setUpgrade( null );
+
+                        ItemStack stack = previousUpgrade.getCraftingItem();
+                        if( stack != null )
+                        {
+                            stack = InventoryUtil.storeItems( stack, inventory, 0, 36, inventory.currentItem );
+                            if( stack != null )
+                            {
+                                WorldUtil.dropItemStack( stack, player.worldObj, player.posX, player.posY, player.posZ );
+                            }
+                        }
+
+                        return null;
+                    }
+                } );
             default:
                 return null;
         }

src/main/java/dan200/computercraft/shared/pocket/core/PocketServerComputer.java

@@ -13,7 +13,6 @@
 import net.minecraft.nbt.NBTTagCompound;
 import net.minecraft.util.ResourceLocation;
 import net.minecraft.world.World;
-import net.minecraftforge.common.util.Constants;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -26,10 +25,9 @@ public class PocketServerComputer extends ServerComputer implements IPocketAcces
     private Entity m_entity;
     private ItemStack m_stack;
 
-    public PocketServerComputer( World world, int computerID, String label, int instanceID, ComputerFamily family, ItemStack stack, Entity entity )
+    public PocketServerComputer( World world, int computerID, String label, int instanceID, ComputerFamily family )
     {
         super( world, computerID, label, instanceID, family, ComputerCraft.terminalWidth_pocketComputer, ComputerCraft.terminalHeight_pocketComputer );
-        update( entity, stack );
     }
 
     @Nullable
@@ -63,30 +61,7 @@ public void setLight( int value )
     @Override
     public NBTTagCompound getUpgradeNBTData()
     {
-        NBTTagCompound tag;
-        if( m_stack.hasTagCompound() )
-        {
-            tag = m_stack.getTagCompound();
-        }
-        else
-        {
-            tag = new NBTTagCompound();
-            m_stack.setTagCompound( tag );
-        }
-
-        if( tag.hasKey( "upgrade_info", Constants.NBT.TAG_COMPOUND ) )
-        {
-            return tag.getCompoundTag( "upgrade_info" );
-        }
-        else
-        {
-            NBTTagCompound sub = new NBTTagCompound();
-
-            tag.setTag( "upgrade_info", sub );
-            updateUpgradeNBTData();
-
-            return sub;
-        }
+        return ComputerCraft.Items.pocketComputer.getUpgradeInfo( m_stack );
     }
 
     @Override
@@ -120,39 +95,47 @@ public Map<ResourceLocation, IPeripheral> getUpgrades()
         }
     }
 
-    public ItemStack getStack()
-    {
-        return m_stack;
-    }
-
     public IPocketUpgrade getUpgrade()
     {
         return m_upgrade;
     }
 
-    public void update( Entity entity, ItemStack stack )
-    {
-        if( m_entity != null ) setPosition( entity.getPosition() );
-        m_entity = entity;
-        m_stack = stack;
-    }
-
-    public synchronized void setUpgrade( IPocketUpgrade upgrade )
+    /**
+     * Set the upgrade for this pocket computer, also updating the item stack.
+     *
+     * Note this method is not thread safe - it must be called from the server thread.
+     *
+     * @param upgrade The new upgrade to set it to, may be {@code null}.
+     */
+    public void setUpgrade( IPocketUpgrade upgrade )
     {
         if( this.m_upgrade == upgrade ) return;
 
-        // Clear the old upgrade NBT
-        if( m_stack.hasTagCompound() )
+        synchronized (this)
         {
-            NBTTagCompound tag = m_stack.getTagCompound();
-            if( tag.hasKey( "upgrade_info", 10 ) )
-            {
-                tag.removeTag( "upgrade_info" );
-                updateUpgradeNBTData();
-            }
+            ComputerCraft.Items.pocketComputer.setUpgrade( m_stack, upgrade );
+            if( m_entity instanceof EntityPlayer ) ((EntityPlayer) m_entity).inventory.markDirty();
+
+            this.m_upgrade = upgrade;
+            invalidatePeripheral();
+        }
+    }
+
+    public synchronized void updateValues( Entity entity, ItemStack stack, IPocketUpgrade upgrade )
+    {
+        if( entity != null )
+        {
+            setWorld( entity.getEntityWorld() );
+            setPosition( entity.getPosition() );
         }
 
-        this.m_upgrade = upgrade;
-        invalidatePeripheral();
+        m_entity = entity;
+        m_stack = stack;
+
+        if( this.m_upgrade != upgrade )
+        {
+            this.m_upgrade = upgrade;
+            invalidatePeripheral();
+        }
     }
 }

src/main/java/dan200/computercraft/shared/pocket/items/ItemPocketComputer.java

@@ -104,10 +104,12 @@ public void onUpdate( ItemStack stack, World world, Entity entity, int slotNum,
             PocketServerComputer computer = createServerComputer( world, inventory, entity, stack );
             if( computer != null )
             {
+                IPocketUpgrade upgrade = getUpgrade( stack );
+
                 // Ping computer
                 computer.keepAlive();
                 computer.setWorld( world );
-                computer.update( entity, stack );
+                computer.updateValues( entity, stack, upgrade );
 
                 // Sync ID
                 int id = computer.getID();
@@ -131,8 +133,7 @@ public void onUpdate( ItemStack stack, World world, Entity entity, int slotNum,
                     }
                 }
 
-                // Update modem
-                IPocketUpgrade upgrade = getUpgrade( stack );
+                // Update pocket upgrade
                 if( upgrade != null )
                 {
                     upgrade.update( computer, computer.getPeripheral( 2 ) );
@@ -165,6 +166,7 @@ public ActionResult<ItemStack> onItemRightClick( ItemStack stack, World world, E
                 IPocketUpgrade upgrade = getUpgrade( stack );
                 if( upgrade != null )
                 {
+                    computer.updateValues( player, stack, upgrade );
                     stop = upgrade.onRightClick( world, computer, computer.getPeripheral( 2 ) );
                 }
             }
@@ -258,12 +260,10 @@ private PocketServerComputer createServerComputer( final World world, IInventory
                 computerID,
                 getLabel( stack ),
                 instanceID,
-                getFamily( stack ),
-                stack,
-                entity
+                getFamily( stack )
             );
+            computer.updateValues( entity, stack, getUpgrade( stack ) );
             computer.addAPI( new PocketAPI( computer ) );
-            computer.setUpgrade( getUpgrade( stack ) );
             ComputerCraft.serverComputerRegistry.add( instanceID, computer );
             if( inventory != null )
             {
@@ -461,7 +461,7 @@ public int getLightState( ItemStack stack )
         return 0;
     }
 
-    public static IPocketUpgrade getUpgrade( ItemStack stack )
+    public IPocketUpgrade getUpgrade( ItemStack stack )
     {
         NBTTagCompound compound = stack.getTagCompound();
         if( compound != null )
@@ -484,7 +484,7 @@ else if( compound.hasKey( "upgrade", Constants.NBT.TAG_ANY_NUMERIC ) )
         return null;
     }
 
-    public static void setUpgrade( ItemStack stack, IPocketUpgrade upgrade )
+    public void setUpgrade( ItemStack stack, IPocketUpgrade upgrade )
     {
         NBTTagCompound compound = stack.getTagCompound();
         if( compound == null ) stack.setTagCompound( compound = new NBTTagCompound() );
@@ -500,4 +500,25 @@ public static void setUpgrade( ItemStack stack, IPocketUpgrade upgrade )
 
         compound.removeTag( "upgrade_info" );
     }
+
+    public NBTTagCompound getUpgradeInfo( ItemStack stack )
+    {
+        NBTTagCompound tag = stack.getTagCompound();
+        if( tag == null )
+        {
+            tag = new NBTTagCompound();
+            stack.setTagCompound( tag );
+        }
+
+        if( tag.hasKey( "upgrade_info", Constants.NBT.TAG_COMPOUND ) )
+        {
+            return tag.getCompoundTag( "upgrade_info" );
+        }
+        else
+        {
+            NBTTagCompound sub = new NBTTagCompound();
+            tag.setTag( "upgrade_info", sub );
+            return sub;
+        }
+    }
 }