Cleanup PlayerStateMessage and name unknown fields (#121)

common/src/main/scala/net/psforever/packet/game/PlayerStateMessage.scala

@@ -8,8 +8,6 @@ import scodec.Codec
 import scodec.codecs._
 import shapeless.{::, HNil}
 
-import scala.collection.mutable
-
 /**
   * The server instructs some clients to render a player (usually not that client's avatar) to move in a certain way.<br>
   * <br>
@@ -68,7 +66,7 @@ import scala.collection.mutable
   * @param is_crouching avatar is crouching
   * @param is_jumping avatar is jumping;
   *                   must remain flagged for jump to maintain animation
-  * @param unk2 na
+  * @param jump_thrust provide a measure of vertical stability when really close to the avatar character
   * @param is_cloaked avatar is cloaked by virtue of an Infiltration Suit
   */
 final case class PlayerStateMessage(guid : PlanetSideGUID,
@@ -80,7 +78,7 @@ final case class PlayerStateMessage(guid : PlanetSideGUID,
                                     unk1 : Int,
                                     is_crouching : Boolean = false,
                                     is_jumping : Boolean = false,
-                                    unk2 : Boolean = false,
+                                    jump_thrust : Boolean = false,
                                     is_cloaked : Boolean = false)
   extends PlanetSideGamePacket {
   type Packet = PlayerStateMessage
@@ -97,7 +95,7 @@ object PlayerStateMessage extends Marshallable[PlayerStateMessage] {
   val booleanCodec : Codec[fourBoolPattern] = (
     ("is_crouching" | bool) ::
       ("is_jumping" | bool) ::
-      ("unk2" | bool) ::
+      ("jump_thrust" | bool) ::
       ("is_cloaked" | bool)
     ).as[fourBoolPattern]
 
@@ -118,7 +116,7 @@ object PlayerStateMessage extends Marshallable[PlayerStateMessage] {
   implicit val codec : Codec[PlayerStateMessage] = (
     ("guid" | PlanetSideGUID.codec) ::
       ("pos" | Vector3.codec_pos) ::
-      optional(bool, "unk1" | Vector3.codec_vel) ::
+      optional(bool, "vel" | Vector3.codec_vel) ::
       ("facingYaw" | uint8L) ::
       ("facingPitch" | uint8L) ::
       ("facingYawUpper" | uint8L) ::
@@ -138,192 +136,3 @@ object PlayerStateMessage extends Marshallable[PlayerStateMessage] {
     }
   )
 }
-
-//TODO the following logic is unimplemented
-/*
-There is a boolean that is currently unhandled(?) that determines if the packet is aware that this code would run.
-If it passes, the first 8-bit value is the number of times the data will be iterated over.
-On each pass, a 4-bit value is extracted from the packet and compared against 15.
-When 15 is read, an 8-bit value is read on that same turn.
-On each subsequent turn, 8-bit values will be read until the number of iterations or until there is an exception.
-Until I find a packet that responds somehow, I have no clue what any of this is supposed to do.
- */
-/**
-  * na
-  * @param size a length to be applied to the next list, but not necessarily the length of that list
-  *             (if I could prove that size == list.size always then I could eliminate superfluous logic from `Extra1`)
-  * @param data a list of data that comes as either an 8-bit value, or as a 4-bit value and, maybe, an 8-bit value
-  */
-final case class Extra1(size : Int,
-                        data : List[Extra2])
-
-/**
-  * na
-  * @param unk1 na;
-  *             the first 8-bit value in one-value form or the first 4-bit value in two-value form;
-  *             in two-value form, when equal to 15, the second value is read
-  * @param unk2 na;
-  *             the potential second 8-bit value in two-value form
-  * @param more the next data in the sequence
-  */
-final case class Extra2(unk1 : Int,
-                        unk2 : Option[Int],
-                        more : Option[Extra2] = None)
-
-object Extra1 {
-  /**
-    * Take a chain of `Extra2` objects produced from decoding and compress it into a `List`.
-    * @param lst the list in which the `Extra2` data will be stored
-    * @param nesting the current link in the chain of `Extra2` objects
-    */
-  private def packExtraList(lst : mutable.ListBuffer[Extra2], nesting : Option[Extra2]) : Unit = {
-    if(nesting.isEmpty) { //escape case
-      return
-    }
-    val elem : Extra2 = nesting.get
-    lst += Extra2(elem.unk1, elem.unk2)
-    packExtraList(lst, elem.more) //tail recursion
-  }
-
-  /**
-    * Take a `List` of `Extra2` objects for encoding and expand it into a chain.
-    * @param iter the iterator for a `List` of `Extra2` data
-    * @return the head of a chain of `Extra2` objects
-    */
-  private def unpackExtraList(iter : Iterator[Extra2]) : Option[Extra2] = {
-    //TODO as I don't think I can use tail recursion, how do I do this iteratively?
-    if(!iter.hasNext)
-      return None
-    val elem : Extra2 = iter.next
-    Some(Extra2(elem.unk1, elem.unk2, unpackExtraList(iter)))
-  }
-
-  implicit val codec : Codec[Extra1] = (
-    ("size" | uint8L) >>:~ { sz =>
-        //external logic: the client checks sz < dword_D33D38 before decoding beyond this point
-        conditional(sz != 0, "data" | Extra2.processData(sz)).hlist
-    }
-    ).xmap[Extra1] (
-    {
-      case a :: None :: HNil =>
-        Extra1(a, List.empty) //it's okay if a != 0
-      case a :: b :: HNil =>
-        val list = mutable.ListBuffer[Extra2]()
-        packExtraList(list, b)
-        Extra1(a, list.toList)
-    },
-    {
-      case Extra1(a, Nil) =>
-        a :: None :: HNil
-      case Extra1(a, b) =>
-        a :: unpackExtraList(b.iterator) :: HNil
-    }
-  )
-}
-
-object Extra2 {
-  /**
-    * An abbreviated constructor for the one-value form.
-    * @param a na
-    * @return an `Extra2` object
-    */
-  def apply(a : Int) : Extra2 = {
-    Extra2(a, None)
-  }
-
-  /**
-    * An abbreviated constructor for the two-value form.
-    * @param a na
-    * @param b na
-    * @return an `Extra2` object
-    */
-  def apply(a : Int, b : Int) : Extra2 = {
-    Extra2(a, Some(b))
-  }
-
-  /**
-    * A `Codec` for reading a single value.
-    */
-  private val oneValueCodec : Codec[Extra2] = ("unk2" | uint8L).hlist.xmap[Extra2] (
-    {
-      case  a :: HNil =>
-        Extra2(a, None, None)
-    },
-    {
-      case Extra2(a, None, _) =>
-         a :: HNil
-    }
-  )
-
-  /**
-    * A `Codec` for reading potentially two values.
-    */
-  private val twoValueCodec : Codec[Extra2] = (
-    ("unk1" | uint4L) >>:~ { unk =>
-      conditional(unk == 15, "unk2" | uint8L).hlist
-    }
-    ).xmap[Extra2] (
-    {
-      case a :: b :: HNil =>
-        Extra2(a, b, None)
-    },
-    {
-      case Extra2(a, b, _) =>
-        a :: b :: HNil
-    }
-  )
-
-  /**
-    * Half of a recursive `Codec` that allows for swapping between different `Codec`s in between `List` elements.<br>
-    * <br>
-    * The function calls itself to process each element in the sequence of data in the same manner until complete.
-    * The `Extra2` object that is recovered from the first choice of `Codec`s is merely an intermediary object.
-    * Due to immutability, the initial object is repackaged to append the chain of `Extra2` in an `Extra2` object.
-    * Eventually, `processData` will parse a 4-bit value of 15 and will pass control over to `processDataSingle`.
-    * @param size the number of iterations of the looping process left to perform, including this one
-    * @return a `Codec` translating a chain of `Extra2` data
-    * @see Extra2.processDataSingle
-    */
-  def processData(size : Int) : Codec[Extra2] = (
-    //TODO: without tail recursion, this might cause a stack overflow
-    twoValueCodec >>:~ { elem =>
-      conditional(size > 0, newcodecs.binary_choice(elem.unk2.isDefined,
-        processDataSingle(size - 1),
-        processData(size - 1))
-      ).hlist
-    }
-    ).xmap[Extra2] (
-    {
-      case a :: b :: HNil =>
-        Extra2(a.unk1, a.unk2, b)
-    },
-    {
-      case Extra2(a, b, c) =>
-        Extra2(a, b) :: c :: HNil
-    }
-  )
-
-  /**
-    * Latter half of a recursive `Codec` that allows for swapping between different `Codec`s in between `List` elements.
-    * This `Codec` no longer performs swapping and merely runs out the data.<br>
-    * <br>
-    * @param size the number of iterations of the looping process left to perform, including this one
-    * @return a `Codec` translating a chain of `Extra2` data
-    * @see Extra2.processData
-    */
-  private def processDataSingle(size : Int) : Codec[Extra2] = (
-    //TODO: without tail recursion, this might cause a stack overflow
-    oneValueCodec >>:~ { elem =>
-      conditional(size > 0, processDataSingle(size - 1)).hlist
-    }
-    ).xmap[Extra2] (
-    {
-      case a :: b :: HNil =>
-        Extra2(a.unk1, a.unk2, b)
-    },
-    {
-      case Extra2(a, b, c) =>
-        Extra2(a, b) :: c :: HNil
-    }
-  )
-}

common/src/test/scala/game/PlayerStateMessageTest.scala

@@ -14,7 +14,7 @@ class PlayerStateMessageTest extends Specification {
 
   "decode (short)" in {
     PacketCoding.DecodePacket(string_short).require match {
-      case PlayerStateMessage(guid, pos, vel, facingYaw, facingPitch, facingUpper, unk1, crouching, jumping, unk2, unk3) =>
+      case PlayerStateMessage(guid, pos, vel, facingYaw, facingPitch, facingUpper, unk1, crouching, jumping, jthrust, cloaked) =>
         guid mustEqual PlanetSideGUID(1696)
         pos.x mustEqual 4003.7422f
         pos.y mustEqual 5981.414f
@@ -26,8 +26,8 @@ class PlayerStateMessageTest extends Specification {
         unk1 mustEqual 83
         crouching mustEqual false
         jumping mustEqual false
-        unk2 mustEqual false
+        jthrust mustEqual false
-        unk3 mustEqual false
+        cloaked mustEqual false
       case _ =>
         ko
     }
@@ -35,7 +35,7 @@ class PlayerStateMessageTest extends Specification {
 
   "decode (mod)" in {
     PacketCoding.DecodePacket(string_mod).require match {
-      case PlayerStateMessage(guid, pos, vel, facingYaw, facingPitch, facingUpper, unk1, crouching, jumping, unk2, unk3) =>
+      case PlayerStateMessage(guid, pos, vel, facingYaw, facingPitch, facingUpper, unk1, crouching, jumping, jthrust, cloaked) =>
         guid mustEqual PlanetSideGUID(1696)
         pos.x mustEqual 4003.7422f
         pos.y mustEqual 5981.414f
@@ -47,8 +47,8 @@ class PlayerStateMessageTest extends Specification {
         unk1 mustEqual 83
         crouching mustEqual false
         jumping mustEqual true
-        unk2 mustEqual false
+        jthrust mustEqual false
-        unk3 mustEqual true
+        cloaked mustEqual true
       case _ =>
         ko
     }
@@ -56,7 +56,7 @@ class PlayerStateMessageTest extends Specification {
 
   "decode (vel)" in {
     PacketCoding.DecodePacket(string_vel).require match {
-      case PlayerStateMessage(guid, pos, vel, facingYaw, facingPitch, facingUpper, unk1, crouching, jumping, unk2, unk3) =>
+      case PlayerStateMessage(guid, pos, vel, facingYaw, facingPitch, facingUpper, unk1, crouching, jumping, jthrust, cloaked) =>
         guid mustEqual PlanetSideGUID(1696)
         pos.x mustEqual 4008.6016f
         pos.y mustEqual 5987.6016f
@@ -71,8 +71,8 @@ class PlayerStateMessageTest extends Specification {
         unk1 mustEqual 165
         crouching mustEqual false
         jumping mustEqual false
-        unk2 mustEqual false
+        jthrust mustEqual false
-        unk3 mustEqual false
+        cloaked mustEqual false
       case _ =>
         ko
     }