yet more network related fixes

assets/opencomputers/lua/init.lua

@@ -90,27 +90,39 @@ function component.type(id)
 end
 
 function component.id(address)
-  for id, component in ipairs(components) do
+  for id, component in pairs(components) do
     if component.address == address then
       return id
     end
   end
 end
 
+function component.ids()
+  local id = nil
+  return function()
+    id = next(components, id)
+    return id
+  end
+end
+
 event.listen("component_added", function(_, address)
   local id = #components + 1
+  -- TODO testing nativeprint("component_added " .. address .. " ~ " .. id)
   components[id] = {address = address, name = driver.componentType(address)}
   event.fire("component_installed", id)
 end)
 
 event.listen("component_removed", function(_, address)
   local id = component.id(address)
+  -- TODO testing nativeprint("component_removed " .. address .. " ~ " .. id)
   components[id] = nil
   event.fire("component_uninstalled", id)
 end)
 
 event.listen("component_changed", function(_, newAddress, oldAddress)
-  components[component.id(oldAddress)].address = newAddress
+  local id = component.id(oldAddress)
+  -- TODO testing nativeprint("component_changed " .. oldAddress .. " -> " .. newAddress .. " ~ " .. id)
+  components[id].address = newAddress
 end)
 
 
@@ -131,8 +143,20 @@ end)
 event.listen("component_uninstalled", function(_, id)
   if idGpu == id then
     term.gpuId(0)
+    for id in component.ids() do
+      if component.type(id) == "gpu" then
+        term.gpuId(id)
+        return
+      end
+    end
   elseif idScreen == id then
     term.screenId(0)
+    for id in component.ids() do
+      if component.type(id) == "screen" then
+        term.screenId(id)
+        return
+      end
+    end
   end
 end)
 
@@ -190,7 +214,7 @@ function term.write(value, wrap)
   local gpu = term.gpu()
   if not gpu or value:len() == 0 then return end
   local w, h = gpu.getResolution()
-  if w < 1 or h < 1 then return end
+  if not w or not h or w < 1 or h < 1 then return end
   local function checkCursor()
     if cursorX > w then
       cursorX = 1

assets/opencomputers/lua/kernel.lua

@@ -46,6 +46,9 @@ local sandbox = {
 
   write = function() end,
 
+  -- TODO for debbuging only
+  --nativeprint = print,
+
   checkArg = checkArg,
   component = component,
   driver = driver,
@@ -221,7 +224,7 @@ end
 return pcall(function()
   -- Replace init script code with loaded, sandboxed and threaded script.
   local init = (function()
-    local result, reason = load(init(), "init", "t", sandbox)
+    local result, reason = load(init(), "=init", "t", sandbox)
     if not result then error(reason, 0) end
     return coroutine.create(result)
   end)()

li/cil/oc/api/INetworkNode.scala

@@ -98,7 +98,7 @@ trait INetworkNode {
    *
    * @param nbt the tag to read from.
    */
-  def save(nbt: NBTTagCompound) = address = nbt.getInteger("address")
+  def load(nbt: NBTTagCompound) = address = nbt.getInteger("address")
 
   /**
    * Stores the node's address in the specified NBT tag, to keep addresses the
@@ -108,7 +108,7 @@ trait INetworkNode {
    *
    * @param nbt the tag to write to.
    */
-  def load(nbt: NBTTagCompound) = nbt.setInteger("address", address)
+  def save(nbt: NBTTagCompound) = nbt.setInteger("address", address)
 
   protected def onConnect() {}
 

li/cil/oc/client/computer/Computer.scala

@@ -18,7 +18,7 @@ class Computer(owner: Any) extends IComputer {
 
   override def signal(name: String, args: Any*) = ???
 
-  override def readFromNBT(nbt: NBTTagCompound) {}
+  override def load(nbt: NBTTagCompound) {}
 
-  override def writeToNBT(nbt: NBTTagCompound) {}
+  override def save(nbt: NBTTagCompound) {}
 }

li/cil/oc/common/components/IScreenEnvironment.scala

@@ -1,6 +1,7 @@
 package li.cil.oc.common.components
 
 import li.cil.oc.api.{INetworkMessage, INetworkNode}
+import net.minecraft.nbt.NBTTagCompound
 
 /**
  * Environment for screen components.
@@ -35,6 +36,20 @@ trait IScreenEnvironment extends INetworkNode {
     }
   }
 
+  override def load(nbt: NBTTagCompound) = {
+    super.load(nbt)
+    screen.load(nbt.getCompoundTag("screen"))
+  }
+
+  override def save(nbt: NBTTagCompound) = {
+    super.save(nbt)
+
+    val screenNbt = new NBTTagCompound
+    screen.save(screenNbt)
+    nbt.setCompoundTag("screen", screenNbt)
+
+  }
+
   def onScreenResolutionChange(w: Int, h: Int)
 
   def onScreenSet(col: Int, row: Int, s: String)

li/cil/oc/common/components/Screen.scala

@@ -38,11 +38,11 @@ class Screen(val owner: IScreenEnvironment) {
     if (buffer.copy(col, row, w, h, tx, ty))
       owner.onScreenCopy(col, row, w, h, tx, ty)
 
-  def readFromNBT(nbt: NBTTagCompound) = {
+  def load(nbt: NBTTagCompound) = {
     buffer.readFromNBT(nbt.getCompoundTag("buffer"))
   }
 
-  def writeToNBT(nbt: NBTTagCompound) = {
+  def save(nbt: NBTTagCompound) = {
     val nbtBuffer = new NBTTagCompound
     buffer.writeToNBT(nbtBuffer)
     nbt.setCompoundTag("buffer", nbtBuffer)

li/cil/oc/common/computer/IComputer.scala

@@ -30,7 +30,7 @@ trait IComputer {
 
   // ----------------------------------------------------------------------- //
 
-  def readFromNBT(nbt: NBTTagCompound)
+  def load(nbt: NBTTagCompound)
 
-  def writeToNBT(nbt: NBTTagCompound)
+  def save(nbt: NBTTagCompound)
 }

li/cil/oc/common/tileentity/TileEntityComputer.scala

@@ -62,20 +62,20 @@ class TileEntityComputer(isClient: Boolean) extends TileEntityRotatable with ICo
 
   override def readFromNBT(nbt: NBTTagCompound) = {
     super.readFromNBT(nbt)
-    computer.readFromNBT(nbt.getCompoundTag("computer"))
     load(nbt.getCompoundTag("data"))
+    computer.load(nbt.getCompoundTag("computer"))
   }
 
   override def writeToNBT(nbt: NBTTagCompound) = {
     super.writeToNBT(nbt)
 
-    val computerNbt = new NBTTagCompound
-    computer.writeToNBT(computerNbt)
-    nbt.setCompoundTag("computer", computerNbt)
-
     val dataNbt = new NBTTagCompound
     save(dataNbt)
     nbt.setCompoundTag("data", dataNbt)
+
+    val computerNbt = new NBTTagCompound
+    computer.save(computerNbt)
+    nbt.setCompoundTag("computer", computerNbt)
   }
 
   override def updateEntity() = {
@@ -85,6 +85,11 @@ class TileEntityComputer(isClient: Boolean) extends TileEntityRotatable with ICo
     }
     if (isRunning != computer.isRunning) {
       isRunning = computer.isRunning
+      if (network != null)
+        if (isRunning)
+          network.sendToAll(this, "computer.start")
+        else
+          network.sendToAll(this, "computer.stop")
       ServerPacketSender.sendComputerState(this, isRunning)
     }
   }
@@ -100,7 +105,7 @@ class TileEntityComputer(isClient: Boolean) extends TileEntityRotatable with ICo
   // ----------------------------------------------------------------------- //
 
   override def isUseableByPlayer(player: EntityPlayer) =
-    world.getBlockTileEntity(xCoord, yCoord, zCoord) == this &&
+    worldObj.getBlockTileEntity(xCoord, yCoord, zCoord) == this &&
       player.getDistanceSq(xCoord + 0.5, yCoord + 0.5, zCoord + 0.5) < 64
 
   override def world = worldObj

li/cil/oc/common/tileentity/TileEntityKeyboard.scala

@@ -2,6 +2,7 @@ package li.cil.oc.common.tileentity
 
 import cpw.mods.fml.common.network.Player
 import li.cil.oc.api.{INetworkNode, INetworkMessage}
+import net.minecraft.entity.player.EntityPlayer
 import net.minecraft.nbt.NBTTagCompound
 
 class TileEntityKeyboard extends TileEntityRotatable with INetworkNode {
@@ -10,35 +11,36 @@ class TileEntityKeyboard extends TileEntityRotatable with INetworkNode {
   override def receive(message: INetworkMessage) = {
     super.receive(message)
     message.data match {
-      case Array(p: Player, char: Char, code: Int) if message.name == "keyboard.keyDown" => {
+      case Array(p: Player, char: Char, code: Int) if message.name == "keyboard.keyDown" => if (isUseableByPlayer(p)) {
-        // TODO check if player is close enough and only consume message if so
         network.sendToAll(this, "signal", "key_down", char, code)
         message.cancel() // One keyboard is enough.
-        None
       }
-      case Array(p: Player, char: Char, code: Int) if message.name == "keyboard.keyUp" => {
+      case Array(p: Player, char: Char, code: Int) if message.name == "keyboard.keyUp" => if (isUseableByPlayer(p)) {
-        // TODO check if player is close enough and only consume message if so
         network.sendToAll(this, "signal", "key_up", char, code)
         message.cancel() // One keyboard is enough.
-        None
       }
-      case Array(p: Player, value: String) if message.name == "keyboard.clipboard" => {
+      case Array(p: Player, value: String) if message.name == "keyboard.clipboard" => if (isUseableByPlayer(p)) {
-        // TODO check if player is close enough and only consume message if so
         network.sendToAll(this, "signal", "clipboard", value)
         message.cancel()
-        None
       }
-      case _ => None
+      case _ => // Ignore.
     }
+    None
   }
 
   override def readFromNBT(nbt: NBTTagCompound) {
     super.readFromNBT(nbt)
-    load(nbt)
+    load(nbt.getCompoundTag("data"))
   }
 
   override def writeToNBT(nbt: NBTTagCompound) {
     super.writeToNBT(nbt)
-    save(nbt)
+
+    val dataNbt = new NBTTagCompound
+    save(dataNbt)
+    nbt.setCompoundTag("data", dataNbt)
   }
+
+  def isUseableByPlayer(p: Player) = worldObj.getBlockTileEntity(xCoord, yCoord, zCoord) == this &&
+    p.asInstanceOf[EntityPlayer].getDistanceSq(xCoord + 0.5, yCoord + 0.5, zCoord + 0.5) < 16
 }

li/cil/oc/common/tileentity/TileEntityScreen.scala

@@ -11,17 +11,12 @@ class TileEntityScreen extends TileEntityRotatable with IScreenEnvironment {
 
   override def readFromNBT(nbt: NBTTagCompound) = {
     super.readFromNBT(nbt)
-    screen.readFromNBT(nbt.getCompoundTag("screen"))
     load(nbt.getCompoundTag("data"))
   }
 
   override def writeToNBT(nbt: NBTTagCompound) = {
     super.writeToNBT(nbt)
 
-    val screenNbt = new NBTTagCompound
-    screen.writeToNBT(screenNbt)
-    nbt.setCompoundTag("screen", screenNbt)
-
     val dataNbt = new NBTTagCompound
     save(dataNbt)
     nbt.setCompoundTag("data", dataNbt)

li/cil/oc/server/components/GraphicsCard.scala

@@ -4,7 +4,7 @@ import li.cil.oc.api.{INetworkNode, INetworkMessage}
 import net.minecraft.nbt.NBTTagCompound
 
 class GraphicsCard(val nbt: NBTTagCompound) extends INetworkNode {
-  if (nbt.hasKey("address")) address = nbt.getInteger("address")
+  address = nbt.getInteger("address")
 
   val supportedResolutions = List(List(40, 24), List(80, 24))
 

li/cil/oc/server/computer/Computer.scala

@@ -3,11 +3,14 @@ package li.cil.oc.server.computer
 import com.naef.jnlua._
 import java.util.concurrent._
 import java.util.concurrent.atomic.AtomicInteger
+import java.util.logging.Level
 import li.cil.oc.common.computer.IComputer
 import li.cil.oc.{OpenComputers, Config}
 import net.minecraft.nbt._
 import scala.Array.canBuildFrom
+import scala.Some
 import scala.collection.JavaConversions._
+import scala.collection.JavaConverters._
 import scala.io.Source
 
 /**
@@ -129,7 +132,7 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
   // ----------------------------------------------------------------------- //
 
   override def start() = stateMonitor.synchronized(
-    state == State.Stopped && init() && (try {
+    state == State.Stopped && init() && {
       state = State.Suspended
 
       // Mark state change in owner, to send it to clients.
@@ -143,17 +146,9 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
       // Inject component added signals for all nodes in the network.
       owner.network.nodes.foreach(node => signal("component_added", node.address))
 
-      // Initialize any installed components.
-      owner.network.sendToAll(owner, "computer.start")
-
       future = Some(Executor.pool.submit(this))
       true
-    }
+    })
-    catch {
-      // The above code may throw if some component was removed by abnormal
-      // means (e.g. mod providing the block was removed/disabled).
-      case _: Throwable => close(); false
-    }))
 
   override def stop() = saveMonitor.synchronized(stateMonitor.synchronized {
     if (state != State.Stopped) {
@@ -224,7 +219,7 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
 
   // ----------------------------------------------------------------------- //
 
-  override def readFromNBT(nbt: NBTTagCompound): Unit =
+  override def load(nbt: NBTTagCompound): Unit =
     saveMonitor.synchronized(this.synchronized {
       // Clear out what we currently have, if anything.
       stateMonitor.synchronized {
@@ -275,7 +270,7 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
                 case tag: NBTTagDouble => tag.data
                 case tag: NBTTagString => tag.data
               }.toArray)
-          }))
+          }).asJava)
 
           timeStarted = nbt.getDouble("timeStarted")
 
@@ -286,19 +281,17 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
           // Start running our worker thread.
           assert(!future.isDefined)
           future = Some(Executor.pool.submit(this))
-        }
+
-        catch {
+        } catch {
           case t: Throwable => {
-            t.printStackTrace()
+            OpenComputers.log.log(Level.WARNING, "Could not restore computer.", t)
-            // TODO display error in-game on monitor or something
-            //signal("crash", "memory corruption")
             close()
           }
         }
       }
     })
 
-  override def writeToNBT(nbt: NBTTagCompound): Unit =
+  override def save(nbt: NBTTagCompound): Unit =
     saveMonitor.synchronized(this.synchronized {
       stateMonitor.synchronized {
         assert(state != State.Running) // Lock on 'this' should guarantee this.
@@ -331,8 +324,6 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
         for (s <- signals.iterator) {
           val signal = new NBTTagCompound
           signal.setString("name", s.name)
-          // TODO Test with NBTTagList, but supposedly it only allows entries
-          //      with the same type, so I went with this for now...
           val args = new NBTTagCompound
           args.setInteger("length", s.args.length)
           s.args.zipWithIndex.foreach {
@@ -533,7 +524,7 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
       // so that they yield a closure doing the actual call so that that
       // message call can be performed in a synchronized fashion.
       lua.load(classOf[Computer].getResourceAsStream(
-        "/assets/opencomputers/lua/boot.lua"), "boot", "t")
+        "/assets/opencomputers/lua/boot.lua"), "=boot", "t")
       lua.call(0, 0)
 
       // Install all driver callbacks into the state. This is done once in
@@ -558,7 +549,7 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
       // functionality in Lua. Why? Because like this it's automatically
       // persisted for us without having to write more additional NBT stuff.
       lua.load(classOf[Computer].getResourceAsStream(
-        "/assets/opencomputers/lua/kernel.lua"), "kernel", "t")
+        "/assets/opencomputers/lua/kernel.lua"), "=kernel", "t")
       lua.newThread() // Leave it as the first value on the stack.
 
       // Run the garbage collector to get rid of stuff left behind after the
@@ -597,10 +588,6 @@ class Computer(val owner: IComputerEnvironment) extends IComputer with Runnable
 
       // Mark state change in owner, to send it to clients.
       owner.markAsChanged()
-
-      if (owner.network != null) {
-        owner.network.sendToAll(owner, "computer.stop")
-      }
     })
 
   // This is a really high level lock that we only use for saving and loading.

li/cil/oc/server/computer/Network.scala

@@ -27,7 +27,7 @@ import scala.collection.mutable.ArrayBuffer
 class Network private(private val nodeMap: mutable.Map[Int, ArrayBuffer[Network.Node]]) extends INetwork {
   def this(node: INetworkNode) = {
     this(mutable.Map({
-      node.address = 1
+      if (node.address < 1) node.address = 1
       node.address -> ArrayBuffer(new Network.Node(node))
     }))
     Network.send(new Network.ConnectMessage(node), List(node))
@@ -70,7 +70,7 @@ class Network private(private val nodeMap: mutable.Map[Int, ArrayBuffer[Network.
       nodes.foreach(node => sendQueue += ((new Network.ConnectMessage(node), List(addedNode))))
       val newNode = new Network.Node(addedNode)
       if (nodeMap.contains(addedNode.address) || addedNode.address < 1)
-        addedNode.address = findId() // Assign address first since it may be ignored.
+        addedNode.address = findId()
       nodeMap.getOrElseUpdate(addedNode.address, new ArrayBuffer[Network.Node]) += newNode
       addedNode.network = this
       (newNode, sendQueue)
@@ -235,31 +235,12 @@ object Network {
       case _ => None
     }
 
-  private def send(message: Network.Message, nodes: Iterable[INetworkNode]) = {
-    //println("send(" + message.name + "(" + message.data.mkString(", ") + "): " + message.source.address + " -> [" + nodes.map(_.address).mkString(", ") + "])")
-    val iterator = nodes.iterator
-    var result = None: Option[Array[Any]]
-    while (!message.isCanceled && iterator.hasNext) {
-      try {
-        iterator.next().receive(message) match {
-          case None => // Ignore.
-          case r => result = r
-        }
-      } catch {
-        case e: Throwable => OpenComputers.log.log(Level.WARNING, "Error in message handler", e)
-      }
-    }
-    result
-  }
-
   private class Node(val data: INetworkNode) {
     val edges = ArrayBuffer.empty[Edge]
 
     def remove() = {
-      val edgesCopy = edges.toBuffer
       edges.foreach(edge => edge.other(this).edges -= edge)
-      edges.clear()
+      searchGraphs(edges.map(_.other(this)))
-      edgesCopy.map(_.remove().filter(_.values.head.head != this)).flatten
     }
   }
 
@@ -274,40 +255,36 @@ object Network {
     def remove() = {
       left.edges -= this
       right.edges -= this
-      // Build neighbor graphs to see if our removal resulted in a split.
+      searchGraphs(List(left, right))
-      val subGraphs = List(
-        (mutable.Map(left.data.address -> ArrayBuffer(left)), mutable.Queue(left.edges.map(_.other(left)): _*)),
-        (mutable.Map(right.data.address -> ArrayBuffer(right)), mutable.Queue(right.edges.map(_.other(right)): _*)))
-      // Breadth-first search to make early merges more likely.
-      while (!subGraphs.forall {
-        case (_, queue) => queue.isEmpty
-      }) for (subGraph <- subGraphs.filter {
-        case (_, queue) => !queue.isEmpty
-      }) {
-        val (nodes, queue) = subGraph
-        val node = queue.dequeue()
-        // See if the node is already in some other graph, in which case we
-        // merge this graph into the other graph.
-        if (!subGraphs.filter(_ != subGraph).exists {
-          case (otherNodes, otherQueue) => otherNodes.get(node.data.address) match {
-            case Some(list) if list.contains(node) => {
-              otherNodes ++= nodes
-              otherQueue ++= queue
-              nodes.clear()
-              queue.clear()
-              true
-            }
-            case _ => false
-          }
-        }) {
-          nodes.getOrElseUpdate(node.data.address, new ArrayBuffer[Network.Node]) += node
-          queue ++= node.edges.map(_.other(node)).filter(n => !nodes.get(n.data.address).exists(_.contains(n)))
-        }
-      }
-      subGraphs map (_._1) filter (!_.isEmpty)
     }
   }
 
+  private def searchGraphs(seeds: Seq[Network.Node]) = {
+    val seen = mutable.Set.empty[Network.Node]
+    seeds.map(seed => {
+      if (seen.contains(seed)) {
+        // If our seed node is contained in another sub graph we have nothing
+        // to do, since we're a sub graph of that sub graph.
+        mutable.Map.empty[Int, mutable.ArrayBuffer[Network.Node]]
+      }
+      else {
+        // Not yet processed, start growing a network from here. We're
+        // guaranteed to not find previously processed nodes, since edges
+        // are bidirectional, and we'd be in the other branch otherwise.
+        seen += seed
+        val subGraph = mutable.Map(seed.data.address -> mutable.ArrayBuffer(seed))
+        val queue = mutable.Queue(seed.edges.map(_.other(seed)): _*)
+        while (queue.nonEmpty) {
+          val node = queue.dequeue()
+          seen += node
+          subGraph.getOrElseUpdate(node.data.address, new ArrayBuffer[Network.Node]) += node
+          queue ++= node.edges.map(_.other(node)).filter(n => !seen.contains(n) && !queue.contains(n))
+        }
+        subGraph
+      }
+    }) filter (_.nonEmpty)
+  }
+
   private class Message(@BeanProperty val source: INetworkNode,
                         @BeanProperty val name: String,
                         @BeanProperty val data: Array[Any] = Array()) extends INetworkMessage {
@@ -322,4 +299,20 @@ object Network {
 
   private class ReconnectMessage(source: INetworkNode, oldAddress: Int) extends Message(source, "network.reconnect", Array(oldAddress.asInstanceOf[Any]))
 
+  private def send(message: Network.Message, nodes: Iterable[INetworkNode]) = {
+    //println("send(" + message.name + "(" + message.data.mkString(", ") + "): " + message.source.address + ":" + message.source.name + " -> [" + nodes.map(node => node.address + ":" + node.name).mkString(", ") + "])")
+    val iterator = nodes.iterator
+    var result = None: Option[Array[Any]]
+    while (!message.isCanceled && iterator.hasNext) {
+      try {
+        iterator.next().receive(message) match {
+          case None => // Ignore.
+          case r => result = r
+        }
+      } catch {
+        case e: Throwable => OpenComputers.log.log(Level.WARNING, "Error in message handler", e)
+      }
+    }
+    result
+  }
 }