Refactor MultiPacketCollector#Bundle (#467)

Bundle was never called in any place other than BundleOption.
This refactors BundleOption into Bundle and removes BundleOption.
Besides being more efficient, it no longer has the possibility of
swallowing unrelated exceptions.

common/src/main/scala/net/psforever/packet/control/MultiPacketCollector.scala

@@ -74,32 +74,16 @@ class MultiPacketCollector() {
   /**
     * Retrieve the internal collection of packets.
     * Reset the internal list of packets by clearing it.
-    * @return a loaded `MultiPacketBundle` object
-    */
-  def Bundle : MultiPacketBundle =  {
-    try {
-      val out = MultiPacketBundle(bundle)
-      bundle = List.empty
-      out
-    }
-    catch {
-      case _ : Exception => //catch and rethrow the exception
-        throw new RuntimeException("no packets")
-    }
-  }
-
-  /**
-    * A safer `Bundle` that consumes any` Exceptions` that might be thrown in the process of producing output.
-    * @see `Bundle`
     * @return a loaded `MultiPacketBundle` object, or `None`
     */
-  def BundleOption : Option[MultiPacketBundle] = {
-    try {
-      Some(Bundle)
-    }
-    catch {
-      case _ : Exception =>
+  def Bundle : Option[MultiPacketBundle] = {
+    bundle match {
+      case Nil =>
         None
+      case list =>
+        val out = MultiPacketBundle(list)
+        bundle = List.empty
+        Some(out)
     }
   }
 }

common/src/test/scala/control/MultiPacketCollectorTest.scala

@@ -78,7 +78,7 @@ class MultiPacketCollectorTest extends Specification {
     "can retrieve a bundle packets" in {
       val obj = MultiPacketCollector(List(packet1, packet2))
       obj.Bundle match {
-        case MultiPacketBundle(list) =>
+        case Some(MultiPacketBundle(list)) =>
           list.size mustEqual 2
           list.head mustEqual packet1
           list(1) mustEqual packet2
@@ -87,21 +87,16 @@ class MultiPacketCollectorTest extends Specification {
       }
     }
 
-    "can not retrieve a bundle of non-existent packets" in {
-      val obj = new MultiPacketCollector()
-      obj.Bundle must throwA[RuntimeException]
-    }
-
-    "can safely retrieve a bundle of potential packets" in {
+    "can retrieve a bundle of potential packets" in {
       val obj1 = new MultiPacketCollector()
-      obj1.BundleOption match {
+      obj1.Bundle match {
         case Some(_) =>
           ko
         case _ => ;
       }
 
       val obj2 = MultiPacketCollector(List(packet1, packet2))
-      obj2.BundleOption match {
+      obj2.Bundle match {
         case None =>
           ko
         case Some(MultiPacketBundle(list)) =>
@@ -114,15 +109,27 @@ class MultiPacketCollectorTest extends Specification {
     "clear packets after being asked to bundle" in {
       val list = List(packet1, packet2)
       val obj = MultiPacketCollector(list)
-      obj.Bundle mustEqual MultiPacketBundle(list)
-      obj.Bundle must throwA[RuntimeException]
+
+      obj.Bundle match {
+        case Some(MultiPacketBundle(bundle)) =>
+          bundle mustEqual list
+        case _ =>
+          ko
+      }
+
+      obj.Bundle match {
+        case Some(MultiPacketBundle(_)) =>
+          ko
+        case _ =>
+          ok
+      }
     }
 
     "add a packet" in {
       val obj = new MultiPacketCollector()
       obj.Add(packet1)
       obj.Bundle match {
-        case MultiPacketBundle(list) =>
+        case Some(MultiPacketBundle(list)) =>
           list.size mustEqual 1
           list.head mustEqual packet1
         case _ =>
@@ -134,7 +141,7 @@ class MultiPacketCollectorTest extends Specification {
       val obj = new MultiPacketCollector()
       obj.Add(List(packet1, packet2))
       obj.Bundle match {
-        case MultiPacketBundle(list) =>
+        case Some(MultiPacketBundle(list)) =>
           list.size mustEqual 2
           list.head mustEqual packet1
           list(1) mustEqual packet2
@@ -147,38 +154,46 @@ class MultiPacketCollectorTest extends Specification {
 
       val obj1 = new MultiPacketCollector()
       obj1.Add(List(packet1, packet2))
-      val bundle1 = obj1.Bundle
-
-      val obj2 = MultiPacketCollector(bundle1)
-      obj2.Add(packet3)
-      obj2.Bundle match {
-        case MultiPacketBundle(list) =>
-          list.size mustEqual 3
-          list.head mustEqual packet1
-          list(1) mustEqual packet2
-          list(2) mustEqual packet3
+      obj1.Bundle match {
+        case Some(MultiPacketBundle(bundle1)) =>
+          val obj2 = MultiPacketCollector(bundle1)
+          obj2.Add(packet3)
+          obj2.Bundle match {
+            case Some(MultiPacketBundle(list)) =>
+              list.size mustEqual 3
+              list.head mustEqual packet1
+              list(1) mustEqual packet2
+              list(2) mustEqual packet3
+            case _ =>
+              ko
+          }
         case _ =>
           ko
       }
+
     }
 
     "concatenate bundles (2)" in {
       val obj1 = new MultiPacketCollector()
       obj1.Add(List(packet1, packet2))
-      val bundle1 = obj1.Bundle
-
-      val obj2 = new MultiPacketCollector()
-      obj2.Add(packet3)
-      obj2.Add(bundle1)
-      obj2.Bundle match {
-        case MultiPacketBundle(list) =>
-          list.size mustEqual 3
-          list.head mustEqual packet3
-          list(1) mustEqual packet1
-          list(2) mustEqual packet2
+      obj1.Bundle match {
+        case Some(MultiPacketBundle(bundle1)) =>
+          val obj2 = new MultiPacketCollector()
+          obj2.Add(packet3)
+          obj2.Add(bundle1)
+          obj2.Bundle match {
+            case Some(MultiPacketBundle(list))=>
+              list.size mustEqual 3
+              list.head mustEqual packet3
+              list(1) mustEqual packet1
+              list(2) mustEqual packet2
+            case _ =>
+              ko
+          }
         case _ =>
           ko
       }
+
     }
   }
 }

pslogin/src/main/scala/WorldSessionActor.scala

@@ -3637,7 +3637,7 @@ class WorldSessionActor extends Actor
 
   /**
     * Instruct the client to treat this player as the avatar.
-    * Initialize all client-specific data that is dependent on some player beign decalred the "avatar".
+    * Initialize all client-specific data that is dependent on some player being declared the "avatar".
     * @param tplayer the target player
     */
   def HandleSetCurrentAvatar(tplayer : Player) : Unit = {
@@ -11445,7 +11445,7 @@ class WorldSessionActor extends Actor
   def StopBundlingPackets() : Unit = {
     log.trace("WORLD SEND: PACKET BUNDLING SUSPENDED")
     packetBundlingFunc = NoBundlingAction
-    packetBundlingCollector.BundleOption match {
+    packetBundlingCollector.Bundle match {
       case Some(bundle) =>
         sendResponse(bundle)
       case None => ;
@@ -11501,7 +11501,7 @@ class WorldSessionActor extends Actor
     */
   def sendResponse(cont : KeepAliveMessage) : Unit = {
     sendResponse(PacketCoding.CreateGamePacket(0, cont))
-    packetBundlingCollector.BundleOption match {
+    packetBundlingCollector.Bundle match {
       case Some(bundle) =>
         log.trace("WORLD SEND: INTERMITTENT PACKET BUNDLE")
         sendResponse(bundle)