fix: send one radii pair for primitive shape cylinders

CMakeLists.txt

@@ -39,4 +39,7 @@ if(FALCON_PROTOCOL_BUILD_TESTS)
     add_executable(FalconProtocolDataStoreDecodeTest tests/DataStoreDecode.cpp)
     target_link_libraries(FalconProtocolDataStoreDecodeTest PRIVATE FalconProtocol)
     add_test(NAME data-store-decode COMMAND FalconProtocolDataStoreDecodeTest)
+    add_executable(FalconProtocolPrimitiveShapesTest tests/PrimitiveShapes.cpp)
+    target_link_libraries(FalconProtocolPrimitiveShapesTest PRIVATE FalconProtocol)
+    add_test(NAME primitive-shapes COMMAND FalconProtocolPrimitiveShapesTest)
 endif()

include/Protocol/Types/DebugShapeData.h

@@ -76,14 +76,13 @@ class DebugShapeData {
     bool mShowBackface = false;
     bool mShowTextBackface = false;
 
-    Vector2f mRadiusX;
-    Vector2f mRadiusZ;
     float mHeight = 0.0f;
 
     float mWidth = 0.0f;
     bool mHasDepth = false;
     float mDepth = 0.0f;
 
+    // cylinder and cone: x the bottom radius, y the top one
     Vector2f mRadii2f;
     Vector3f mRadii3f;
 };

src/Protocol/Packets/PrimitiveShapesPacket.cpp

@@ -68,9 +68,9 @@ namespace {
         if (shape.mHasDimension)
             stream.putVarInt(shape.mDimension);
 
-    stream.putOptionalPresent(shape.mHasAttachedToActorId);
-    if (shape.mHasAttachedToActorId)
-        stream.putVarLong((int64_t) shape.mAttachedToActorId);
+        stream.putOptionalPresent(shape.mHasAttachedToActorId);
+        if (shape.mHasAttachedToActorId)
+            stream.putVarLong((int64_t) shape.mAttachedToActorId);
 
         stream.putUnsignedVarInt((uint32_t) toPayloadType(shape.mHasType, shape.mType));
 
@@ -116,8 +116,7 @@ namespace {
                 stream.putBool(shape.mShowTextBackface);
                 break;
             case DebugShapeType::Cylinder:
-                stream.putVector2f(shape.mRadiusX);
-                stream.putVector2f(shape.mRadiusZ);
+                stream.putVector2f(shape.mRadii2f);
                 stream.putLFloat(shape.mHeight);
                 stream.putByte((unsigned char) shape.mSegments);
                 break;
@@ -176,9 +175,9 @@ namespace {
         if (shape.mHasDimension)
             shape.mDimension = stream.getVarInt();
 
-    shape.mHasAttachedToActorId = stream.getOptionalPresent();
-    if (shape.mHasAttachedToActorId)
-        shape.mAttachedToActorId = (uint64_t) stream.getVarLong();
+        shape.mHasAttachedToActorId = stream.getOptionalPresent();
+        if (shape.mHasAttachedToActorId)
+            shape.mAttachedToActorId = (uint64_t) stream.getVarLong();
 
         stream.getUnsignedVarInt();
 
@@ -224,8 +223,7 @@ namespace {
                 shape.mShowTextBackface = stream.getBool();
                 break;
             case DebugShapeType::Cylinder:
-                shape.mRadiusX = stream.getVector2f();
-                shape.mRadiusZ = stream.getVector2f();
+                shape.mRadii2f = stream.getVector2f();
                 shape.mHeight = stream.getLFloat();
                 shape.mSegments = stream.getByte();
                 break;
@@ -256,7 +254,7 @@ PrimitiveShapesPacket::PrimitiveShapesPacket() = default;
 
 void PrimitiveShapesPacket::write(BinaryStream &stream, const PacketCodecContext &context) const {
     stream.putArrayLength((uint32_t) mShapes.size());
-    for (const DebugShapeData &shape : mShapes) {
+    for (const DebugShapeData &shape: mShapes) {
         writeShape(stream, shape);
     }
 }

tests/PrimitiveShapes.cpp

@@ -0,0 +1,46 @@
+#include "Protocol/Packets/PrimitiveShapesPacket.h"
+
+#include <cstring>
+#include <stdexcept>
+
+static void check(bool value, const char *message) {
+    if (!value) {
+        throw std::runtime_error(message);
+    }
+}
+
+int main() {
+    BlockDefinitionRegistry blocks;
+    ItemDefinitionRegistry items;
+    PacketCodecContext context(blocks, items);
+
+    PrimitiveShapesPacket packet;
+    DebugShapeData cylinder;
+    cylinder.mId = 7;
+    cylinder.mHasType = true;
+    cylinder.mType = DebugShapeType::Cylinder;
+    cylinder.mRadii2f = Vector2f(1.5f, 0.5f);
+    cylinder.mHeight = 2.0f;
+    cylinder.mSegments = 20;
+    DebugShapeData removed;
+    removed.mId = 8;
+    packet.mShapes = {cylinder, removed};
+
+    BinaryStream encoded;
+    packet.write(encoded, context);
+    const std::string &bytes = encoded.getBuffer();
+    // count, id, type, eight absent optionals, variant 6, radii, height, segments; then id 8 and nine absent fields
+    check(bytes.size() == 1 + 1 + 2 + 8 + 1 + 8 + 4 + 1 + 1 + 9 + 1, "cylinder carries one radii pair");
+    float bottom = 0.0f;
+    std::memcpy(&bottom, bytes.data() + 13, 4);
+    check(bytes[12] == 6 && bottom == 1.5f, "cylinder radii misplaced");
+
+    ReadOnlyBinaryStream reader(bytes);
+    PrimitiveShapesPacket decoded;
+    decoded.read(reader, context);
+    check(decoded.mShapes.size() == 2, "shape count decoded incorrectly");
+    check(decoded.mShapes[0].mRadii2f.y == 0.5f && decoded.mShapes[0].mHeight == 2.0f
+                  && decoded.mShapes[0].mSegments == 20,
+          "cylinder decoded incorrectly");
+    check(decoded.mShapes[1].mId == 8 && !decoded.mShapes[1].mHasType, "a removal lost its id");
+}