Various improvements to peripheral invalidation

- Abstract peripheral ID and type checking into separate class
 - Update peripherals directly rather than marking as invalid then
   fetching from the network.
 - Update peripherals when adjacent tiles change

This does result in a slightly more ugly interface, but reduces the
amount of work needed to perform partial updates of peripherals, such as
those done by neighbouring tile updates.

src/main/java/dan200/computercraft/shared/peripheral/modem/TileCable.java

@@ -16,10 +16,7 @@ import dan200.computercraft.shared.peripheral.PeripheralType;
 import dan200.computercraft.shared.peripheral.common.BlockCable;
 import dan200.computercraft.shared.peripheral.common.BlockCableModemVariant;
 import dan200.computercraft.shared.peripheral.common.PeripheralItemFactory;
-import dan200.computercraft.shared.util.IDAssigner;
-import dan200.computercraft.shared.util.PeripheralUtil;
 import dan200.computercraft.shared.wired.CapabilityWiredElement;
-import net.minecraft.block.Block;
 import net.minecraft.block.state.IBlockState;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.item.ItemStack;
@@ -35,7 +32,6 @@ import net.minecraftforge.common.capabilities.Capability;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import java.io.File;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -79,16 +75,6 @@ public class TileCable extends TileModemBase
             return new Vec3d( pos.getX() + 0.5, pos.getY() + 0.5, pos.getZ() + 0.5 );
         }
 
-        @Nonnull
-        @Override
-        public Map<String, IPeripheral> getPeripherals()
-        {
-            IPeripheral peripheral = m_entity.getConnectedPeripheral();
-            return peripheral != null
-                ? Collections.singletonMap( m_entity.getConnectedPeripheralName(), peripheral )
-                : Collections.emptyMap();
-        }
-
         @Override
         protected void attachPeripheral( String name, IPeripheral peripheral )
         {
@@ -105,9 +91,9 @@ public class TileCable extends TileModemBase
     // Members
 
     private boolean m_peripheralAccessAllowed;
-    private int m_attachedPeripheralID;
+    private WiredModemLocalPeripheral m_peripheral = new WiredModemLocalPeripheral();
 
-    private boolean m_destroyed;
+    private boolean m_destroyed = false;
 
     private boolean m_hasDirection = false;
     private boolean m_connectionsFormed = false;
@@ -115,14 +101,6 @@ public class TileCable extends TileModemBase
     private WiredModemElement m_cable;
     private IWiredNode m_node;
 
-    public TileCable()
-    {
-        m_peripheralAccessAllowed = false;
-        m_attachedPeripheralID = -1;
-
-        m_destroyed = false;
-    }
-
     @Override
     protected ModemPeripheral createPeripheral()
     {
@@ -133,7 +111,7 @@ public class TileCable extends TileModemBase
             @Override
             protected boolean canSeePeripheral( @Nonnull String peripheralName )
             {
-                return !peripheralName.equals( getConnectedPeripheralName() );
+                return !peripheralName.equals( m_peripheral.getConnectedName() );
             }
 
             @Nonnull
@@ -294,10 +272,29 @@ public class TileCable extends TileModemBase
                     modemChanged();
                     connectionsChanged();
 
-                    break;
+                    return;
                 }
             }
         }
+
+        if( !world.isRemote && m_peripheralAccessAllowed )
+        {
+            if( m_peripheral.attach( world, getPos(), dir ) ) updateConnectedPeripherals();
+        }
+    }
+
+    @Override
+    public void onNeighbourTileEntityChange( @Nonnull BlockPos neighbour )
+    {
+        super.onNeighbourTileEntityChange( neighbour );
+        if( !world.isRemote && m_peripheralAccessAllowed )
+        {
+            EnumFacing facing = getDirection();
+            if( getPos().offset( facing ).equals( neighbour ) )
+            {
+                if( m_peripheral.attach( world, getPos(), facing ) ) updateConnectedPeripherals();
+            }
+        }
     }
 
     public AxisAlignedBB getModemBounds()
@@ -400,9 +397,9 @@ public class TileCable extends TileModemBase
             if( !getWorld().isRemote )
             {
                 // On server, we interacted if a peripheral was found
-                String oldPeriphName = getConnectedPeripheralName();
+                String oldPeriphName = m_peripheral.getConnectedName();
                 togglePeripheralAccess();
-                String periphName = getConnectedPeripheralName();
+                String periphName = m_peripheral.getConnectedName();
 
                 if( !Objects.equal( periphName, oldPeriphName ) )
                 {
@@ -437,7 +434,7 @@ public class TileCable extends TileModemBase
         // Read properties
         super.readFromNBT( nbttagcompound );
         m_peripheralAccessAllowed = nbttagcompound.getBoolean( "peripheralAccess" );
-        m_attachedPeripheralID = nbttagcompound.getInteger( "peripheralID" );
+        m_peripheral.readNBT( nbttagcompound, "" );
     }
 
     @Nonnull
@@ -447,7 +444,7 @@ public class TileCable extends TileModemBase
         // Write properties
         nbttagcompound = super.writeToNBT( nbttagcompound );
         nbttagcompound.setBoolean( "peripheralAccess", m_peripheralAccessAllowed );
-        nbttagcompound.setInteger( "peripheralID", m_attachedPeripheralID );
+        m_peripheral.writeNBT( nbttagcompound, "" );
         return nbttagcompound;
     }
 
@@ -476,8 +473,13 @@ public class TileCable extends TileModemBase
             if( !m_connectionsFormed )
             {
                 m_connectionsFormed = true;
+
                 connectionsChanged();
-                if( m_peripheralAccessAllowed ) m_node.invalidate();
+                if( m_peripheralAccessAllowed )
+                {
+                    m_peripheral.attach( world, pos, m_dir );
+                    updateConnectedPeripherals();
+                }
             }
         }
     }
@@ -515,15 +517,11 @@ public class TileCable extends TileModemBase
         if( getWorld().isRemote ) return;
 
         PeripheralType type = getPeripheralType();
-        if( type == PeripheralType.Cable )
-        {
-            m_attachedPeripheralID = -1;
-        }
-
         if( type != PeripheralType.WiredModemWithCable && m_peripheralAccessAllowed )
         {
             m_peripheralAccessAllowed = false;
-            m_node.invalidate();
+            m_peripheral.detach();
+            m_node.updatePeripherals( Collections.emptyMap() );
             markDirty();
             updateAnim();
         }
@@ -534,61 +532,34 @@ public class TileCable extends TileModemBase
     {
         if( !m_peripheralAccessAllowed )
         {
+            m_peripheral.attach( world, getPos(), getDirection() );
+            if( !m_peripheral.hasPeripheral() ) return;
+
             m_peripheralAccessAllowed = true;
-            if( getConnectedPeripheral() == null )
-            {
-                m_peripheralAccessAllowed = false;
-                return;
-            }
+            m_node.updatePeripherals( m_peripheral.toMap() );
         }
         else
         {
+            m_peripheral.detach();
+
             m_peripheralAccessAllowed = false;
+            m_node.updatePeripherals( Collections.emptyMap() );
         }
 
         updateAnim();
-        m_node.invalidate();
     }
 
-    private String getConnectedPeripheralName()
+    private void updateConnectedPeripherals()
     {
-        IPeripheral periph = getConnectedPeripheral();
-        if( periph != null )
+        Map<String, IPeripheral> peripherals = m_peripheral.toMap();
+        if( peripherals.isEmpty() )
         {
-            String type = periph.getType();
-            if( m_attachedPeripheralID < 0 )
-            {
-                m_attachedPeripheralID = IDAssigner.getNextIDFromFile( new File(
-                    ComputerCraft.getWorldDir( getWorld() ),
-                    "computer/lastid_" + type + ".txt"
-                ) );
-            }
-            return type + "_" + m_attachedPeripheralID;
+            // If there are no peripherals then disable access and update the display state.
+            m_peripheralAccessAllowed = false;
+            updateAnim();
         }
-        return null;
-    }
 
-    private IPeripheral getConnectedPeripheral()
-    {
-        if( m_peripheralAccessAllowed )
-        {
-            if( getPeripheralType() == PeripheralType.WiredModemWithCable )
-            {
-                EnumFacing facing = getDirection();
-                BlockPos neighbour = getPos().offset( facing );
-                IPeripheral peripheral = getPeripheral( getWorld(), neighbour, facing.getOpposite() );
-                return peripheral == null || peripheral instanceof WiredModemPeripheral ? null : peripheral;
-            }
-        }
-        return null;
-    }
-
-    public static IPeripheral getPeripheral( World world, BlockPos pos, EnumFacing facing )
-    {
-        Block block = world.getBlockState( pos ).getBlock();
-        if( block == ComputerCraft.Blocks.wiredModemFull || block == ComputerCraft.Blocks.cable ) return null;
-
-        return PeripheralUtil.getPeripheral( world, pos, facing );
+        m_node.updatePeripherals( peripherals );
     }
 
     @Override

src/main/java/dan200/computercraft/shared/peripheral/modem/TileWiredModemFull.java

@@ -13,7 +13,6 @@ import dan200.computercraft.api.network.wired.IWiredNode;
 import dan200.computercraft.api.peripheral.IPeripheral;
 import dan200.computercraft.shared.peripheral.common.BlockCable;
 import dan200.computercraft.shared.peripheral.common.TilePeripheralBase;
-import dan200.computercraft.shared.util.IDAssigner;
 import dan200.computercraft.shared.wired.CapabilityWiredElement;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.nbt.NBTTagCompound;
@@ -24,11 +23,9 @@ import net.minecraft.util.math.Vec3d;
 import net.minecraft.util.text.TextComponentTranslation;
 import net.minecraft.world.World;
 import net.minecraftforge.common.capabilities.Capability;
-import net.minecraftforge.common.util.Constants;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import java.io.File;
 import java.util.*;
 
 public class TileWiredModemFull extends TilePeripheralBase
@@ -76,20 +73,12 @@ public class TileWiredModemFull extends TilePeripheralBase
             BlockPos pos = m_entity.getPos();
             return new Vec3d( pos.getX() + 0.5, pos.getY() + 0.5, pos.getZ() + 0.5 );
         }
-
-        @Nonnull
-        @Override
-        public Map<String, IPeripheral> getPeripherals()
-        {
-            return m_entity.getPeripherals();
-        }
     }
 
     private WiredModemPeripheral[] m_modems = new WiredModemPeripheral[6];
 
     private boolean m_peripheralAccessAllowed = false;
-    private int[] m_attachedPeripheralIDs = new int[6];
-    private String[] m_attachedPeripheralTypes = new String[6];
+    private WiredModemLocalPeripheral[] m_peripherals = new WiredModemLocalPeripheral[6];
 
     private boolean m_destroyed = false;
     private boolean m_connectionsFormed = false;
@@ -99,7 +88,7 @@ public class TileWiredModemFull extends TilePeripheralBase
 
     public TileWiredModemFull()
     {
-        Arrays.fill( m_attachedPeripheralIDs, -1 );
+        for( int i = 0; i < m_peripherals.length; i++ ) m_peripherals[i] = new WiredModemLocalPeripheral();
     }
 
     private void remove()
@@ -152,18 +141,29 @@ public class TileWiredModemFull extends TilePeripheralBase
     {
         if( !world.isRemote && m_peripheralAccessAllowed )
         {
-            Map<String, IPeripheral> updated = getPeripherals();
-
-            if( updated.isEmpty() )
+            boolean hasChanged = false;
+            for( EnumFacing facing : EnumFacing.VALUES )
             {
-                // If there are no peripherals then disable access and update the display state.
-                m_peripheralAccessAllowed = false;
-                updateAnim();
+                hasChanged |= m_peripherals[facing.ordinal()].attach( world, getPos(), facing );
             }
 
-            // Always invalidate the node: it's more accurate than checking if the peripherals
-            // have changed
-            m_node.invalidate();
+            if( hasChanged ) updateConnectedPeripherals();
+        }
+    }
+
+    @Override
+    public void onNeighbourTileEntityChange( @Nonnull BlockPos neighbour )
+    {
+        if( !world.isRemote && m_peripheralAccessAllowed )
+        {
+            for( EnumFacing facing : EnumFacing.VALUES )
+            {
+                if( getPos().offset( facing ).equals( neighbour ) )
+                {
+                    WiredModemLocalPeripheral peripheral = m_peripherals[facing.ordinal()];
+                    if( peripheral.attach( world, getPos(), facing ) ) updateConnectedPeripherals();
+                }
+            }
         }
     }
 
@@ -180,9 +180,9 @@ public class TileWiredModemFull extends TilePeripheralBase
         if( !getWorld().isRemote )
         {
             // On server, we interacted if a peripheral was found
-            Set<String> oldPeriphName = getPeripherals().keySet();
+            Set<String> oldPeriphName = getConnectedPeripheralNames();
             togglePeripheralAccess();
-            Set<String> periphName = getPeripherals().keySet();
+            Set<String> periphName = getConnectedPeripheralNames();
 
             if( !Objects.equal( periphName, oldPeriphName ) )
             {
@@ -220,17 +220,7 @@ public class TileWiredModemFull extends TilePeripheralBase
     {
         super.readFromNBT( tag );
         m_peripheralAccessAllowed = tag.getBoolean( "peripheralAccess" );
-        for( int i = 0; i < m_attachedPeripheralIDs.length; i++ )
-        {
-            if( tag.hasKey( "peripheralID_" + i, Constants.NBT.TAG_ANY_NUMERIC ) )
-            {
-                m_attachedPeripheralIDs[i] = tag.getInteger( "peripheralID_" + i );
-            }
-            if( tag.hasKey( "peripheralType_" + i, Constants.NBT.TAG_STRING ) )
-            {
-                m_attachedPeripheralTypes[i] = tag.getString( "peripheralType_" + i );
-            }
-        }
+        for( int i = 0; i < m_peripherals.length; i++ ) m_peripherals[i].readNBT( tag, "_" + i );
     }
 
     @Nonnull
@@ -239,17 +229,7 @@ public class TileWiredModemFull extends TilePeripheralBase
     {
         tag = super.writeToNBT( tag );
         tag.setBoolean( "peripheralAccess", m_peripheralAccessAllowed );
-        for( int i = 0; i < m_attachedPeripheralIDs.length; i++ )
-        {
-            if( m_attachedPeripheralIDs[i] >= 0 )
-            {
-                tag.setInteger( "peripheralID_" + i, m_attachedPeripheralIDs[i] );
-            }
-            if( m_attachedPeripheralTypes[i] != null )
-            {
-                tag.setString( "peripheralType_" + i, m_attachedPeripheralTypes[i] );
-            }
-        }
+        for( int i = 0; i < m_peripherals.length; i++ ) m_peripherals[i].writeNBT( tag, "_" + i );
         return tag;
     }
 
@@ -294,8 +274,16 @@ public class TileWiredModemFull extends TilePeripheralBase
             if( !m_connectionsFormed )
             {
                 m_connectionsFormed = true;
+
                 connectionsChanged();
-                if( m_peripheralAccessAllowed ) m_node.invalidate();
+                if( m_peripheralAccessAllowed )
+                {
+                    for( EnumFacing facing : EnumFacing.VALUES )
+                    {
+                        m_peripherals[facing.ordinal()].attach( world, getPos(), facing );
+                    }
+                    updateConnectedPeripherals();
+                }
             }
         }
 
@@ -326,60 +314,63 @@ public class TileWiredModemFull extends TilePeripheralBase
     {
         if( !m_peripheralAccessAllowed )
         {
-            m_peripheralAccessAllowed = true;
-            if( getPeripherals().isEmpty() )
+            boolean hasAny = false;
+            for( EnumFacing facing : EnumFacing.VALUES )
             {
-                m_peripheralAccessAllowed = false;
-                return;
+                WiredModemLocalPeripheral peripheral = m_peripherals[facing.ordinal()];
+                peripheral.attach( world, getPos(), facing );
+                hasAny |= peripheral.hasPeripheral();
             }
+
+            if( !hasAny ) return;
+
+            m_peripheralAccessAllowed = true;
+            m_node.updatePeripherals( getConnectedPeripherals() );
         }
         else
         {
             m_peripheralAccessAllowed = false;
+
+            for( WiredModemLocalPeripheral peripheral : m_peripherals ) peripheral.detach();
+            m_node.updatePeripherals( Collections.emptyMap() );
         }
 
         updateAnim();
-        m_node.invalidate();
     }
 
-    @Nonnull
-    private Map<String, IPeripheral> getPeripherals()
+    private Set<String> getConnectedPeripheralNames()
+    {
+        if( !m_peripheralAccessAllowed ) return Collections.emptySet();
+
+        Set<String> peripherals = new HashSet<>( 6 );
+        for( WiredModemLocalPeripheral m_peripheral : m_peripherals )
+        {
+            String name = m_peripheral.getConnectedName();
+            if( name != null ) peripherals.add( name );
+        }
+        return peripherals;
+    }
+
+    private Map<String, IPeripheral> getConnectedPeripherals()
     {
         if( !m_peripheralAccessAllowed ) return Collections.emptyMap();
 
         Map<String, IPeripheral> peripherals = new HashMap<>( 6 );
-        for( EnumFacing facing : EnumFacing.VALUES )
-        {
-            BlockPos neighbour = getPos().offset( facing );
-            IPeripheral peripheral = TileCable.getPeripheral( getWorld(), neighbour, facing.getOpposite() );
-            if( peripheral != null && !(peripheral instanceof WiredModemPeripheral) )
-            {
-                String type = peripheral.getType();
-                int id = m_attachedPeripheralIDs[facing.ordinal()];
-                String oldType = m_attachedPeripheralTypes[facing.ordinal()];
-                if( id < 0 || !type.equals( oldType ) )
-                {
-                    m_attachedPeripheralTypes[facing.ordinal()] = type;
-                    id = m_attachedPeripheralIDs[facing.ordinal()] = IDAssigner.getNextIDFromFile( new File(
-                        ComputerCraft.getWorldDir( getWorld() ),
-                        "computer/lastid_" + type + ".txt"
-                    ) );
-                }
-
-                peripherals.put( type + "_" + id, peripheral );
-            }
-        }
-
+        for( WiredModemLocalPeripheral m_peripheral : m_peripherals ) m_peripheral.extendMap( peripherals );
         return peripherals;
     }
 
-    private String getCachedPeripheralName( EnumFacing facing )
+    private void updateConnectedPeripherals()
     {
-        if( !m_peripheralAccessAllowed ) return null;
+        Map<String, IPeripheral> peripherals = getConnectedPeripherals();
+        if( peripherals.isEmpty() )
+        {
+            // If there are no peripherals then disable access and update the display state.
+            m_peripheralAccessAllowed = false;
+            updateAnim();
+        }
 
-        int id = m_attachedPeripheralIDs[facing.ordinal()];
-        String type = m_attachedPeripheralTypes[facing.ordinal()];
-        return id < 0 || type == null ? null : type + "_" + id;
+        m_node.updatePeripherals( peripherals );
     }
 
     // IWiredElementTile
@@ -415,7 +406,7 @@ public class TileWiredModemFull extends TilePeripheralBase
                 @Override
                 protected boolean canSeePeripheral( @Nonnull String peripheralName )
                 {
-                    return !peripheralName.equals( getCachedPeripheralName( side ) );
+                    return !peripheralName.equals( m_peripherals[side.ordinal()].getConnectedName() );
                 }
 
                 @Nonnull

src/main/java/dan200/computercraft/shared/peripheral/modem/WiredModemLocalPeripheral.java

@@ -0,0 +1,139 @@
+package dan200.computercraft.shared.peripheral.modem;
+
+import dan200.computercraft.ComputerCraft;
+import dan200.computercraft.api.peripheral.IPeripheral;
+import dan200.computercraft.shared.util.IDAssigner;
+import dan200.computercraft.shared.util.PeripheralUtil;
+import net.minecraft.block.Block;
+import net.minecraft.nbt.NBTTagCompound;
+import net.minecraft.util.EnumFacing;
+import net.minecraft.util.math.BlockPos;
+import net.minecraft.world.World;
+import net.minecraftforge.common.util.Constants;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.io.File;
+import java.util.Collections;
+import java.util.Map;
+
+/**
+ * Represents a local peripheral exposed on the wired network
+ *
+ * This is responsible for getting the peripheral in world, tracking id and type and determining whether
+ * it has changed.
+ */
+public final class WiredModemLocalPeripheral
+{
+    private int id;
+    private String type;
+
+    private IPeripheral peripheral;
+
+    /**
+     * Attach a new peripheral from the world
+     *
+     * @param world     The world to search in
+     * @param origin    The position to search from
+     * @param direction The direction so search in
+     * @return Whether the peripheral changed.
+     */
+    public boolean attach( @Nonnull World world, @Nonnull BlockPos origin, @Nonnull EnumFacing direction )
+    {
+        IPeripheral oldPeripheral = this.peripheral;
+        IPeripheral peripheral = this.peripheral = getPeripheralFrom( world, origin, direction );
+
+        if( peripheral == null )
+        {
+            return oldPeripheral != null;
+        }
+        else
+        {
+            String type = peripheral.getType();
+            int id = this.id;
+
+            if( id > 0 && this.type == null )
+            {
+                // If we had an ID but no type, then just set the type.
+                this.type = type;
+            }
+            else if( id < 0 || !type.equals( this.type ) )
+            {
+                this.type = type;
+                this.id = IDAssigner.getNextIDFromFile( new File(
+                    ComputerCraft.getWorldDir( world ),
+                    "computer/lastid_" + type + ".txt"
+                ) );
+            }
+
+            return oldPeripheral == null || !oldPeripheral.equals( peripheral );
+        }
+    }
+
+    /**
+     * Detach the current peripheral
+     *
+     * @return Whether the peripheral changed
+     */
+    public boolean detach()
+    {
+        if( peripheral == null ) return false;
+        peripheral = null;
+        return true;
+    }
+
+    @Nullable
+    public String getConnectedName()
+    {
+        return peripheral != null ? type + "_" + id : null;
+    }
+
+    @Nullable
+    public IPeripheral getPeripheral()
+    {
+        return peripheral;
+    }
+
+    public boolean hasPeripheral()
+    {
+        return peripheral != null;
+    }
+
+    public void extendMap( @Nonnull Map<String, IPeripheral> peripherals )
+    {
+        if( peripheral != null ) peripherals.put( type + "_" + id, peripheral );
+    }
+
+    public Map<String, IPeripheral> toMap()
+    {
+        return peripheral == null
+            ? Collections.emptyMap()
+            : Collections.singletonMap( type + "_" + id, peripheral );
+    }
+
+    public void writeNBT( @Nonnull NBTTagCompound tag, @Nonnull String suffix )
+    {
+        if( id >= 0 ) tag.setInteger( "peripheralID" + suffix, id );
+        if( type != null ) tag.setString( "peripheralType" + suffix, type );
+    }
+
+    public void readNBT( @Nonnull NBTTagCompound tag, @Nonnull String suffix )
+    {
+        id = tag.hasKey( "peripheralID" + suffix, Constants.NBT.TAG_ANY_NUMERIC )
+            ? tag.getInteger( "peripheralID" + suffix ) : -1;
+
+        type = tag.hasKey( "peripheralType" + suffix, Constants.NBT.TAG_STRING )
+            ? tag.getString( "peripheralType" + suffix ) : null;
+    }
+
+    private static IPeripheral getPeripheralFrom( World world, BlockPos pos, EnumFacing direction )
+    {
+        BlockPos offset = pos.offset( direction );
+
+        Block block = world.getBlockState( offset ).getBlock();
+        if( block == ComputerCraft.Blocks.wiredModemFull || block == ComputerCraft.Blocks.cable ) return null;
+
+        IPeripheral peripheral = PeripheralUtil.getPeripheral( world, offset, direction.getOpposite() );
+        return peripheral instanceof WiredModemPeripheral ? null : peripheral;
+    }
+}