PR feedback + cleanup

Author: yungcomputerchair

src/Bucket.hpp

@@ -3,6 +3,8 @@
 #include <array>
 #include <optional>
 
+#include <assert.h>
+
 template<class T, size_t N>
 class Bucket {
     std::array<T, N> buf;
@@ -13,9 +15,8 @@ class Bucket {
     }
 
     void add(const T& item) {
-        if (sz < N) {
-            buf[sz++] = item;
-        }
+        assert(sz < N);
+        buf[sz++] = item;
     }
 
     std::optional<T> get(size_t idx) const {

src/Chunking.cpp

@@ -13,11 +13,12 @@ using namespace Chunking;
  * The initial chunkPos value before a player is placed into the world.
  */
 const ChunkPos Chunking::INVALID_CHUNK = {};
-constexpr size_t MAX_PC_PER_AROUND = (CN_PACKET_BUFFER_SIZE - sizeof(int32_t)) / sizeof(sPCAppearanceData);
-constexpr size_t MAX_NPC_PER_AROUND = (CN_PACKET_BUFFER_SIZE - sizeof(int32_t)) / sizeof(sNPCAppearanceData);
-constexpr size_t MAX_SHINY_PER_AROUND = (CN_PACKET_BUFFER_SIZE - sizeof(int32_t)) / sizeof(sShinyAppearanceData);
-constexpr size_t MAX_TRANSPORTATION_PER_AROUND = (CN_PACKET_BUFFER_SIZE - sizeof(int32_t)) / sizeof(sTransportationAppearanceData);
-constexpr size_t MAX_IDS_PER_AROUND_DEL = (CN_PACKET_BUFFER_SIZE - sizeof(int32_t)) / sizeof(int32_t);
+constexpr size_t MAX_PC_PER_AROUND = (CN_PACKET_BUFFER_SIZE - 3 * sizeof(int32_t)) / sizeof(sPCAppearanceData);
+constexpr size_t MAX_NPC_PER_AROUND = (CN_PACKET_BUFFER_SIZE - 3 * sizeof(int32_t)) / sizeof(sNPCAppearanceData);
+constexpr size_t MAX_SHINY_PER_AROUND = (CN_PACKET_BUFFER_SIZE - 3 * sizeof(int32_t)) / sizeof(sShinyAppearanceData);
+constexpr size_t MAX_TRANSPORTATION_PER_AROUND = (CN_PACKET_BUFFER_SIZE - 3 * sizeof(int32_t)) / sizeof(sTransportationAppearanceData);
+constexpr size_t MAX_IDS_PER_AROUND_DEL = (CN_PACKET_BUFFER_SIZE - 3 * sizeof(int32_t)) / sizeof(int32_t);
+constexpr size_t MAX_TRANSPORTATION_IDS_PER_AROUND_DEL = MAX_IDS_PER_AROUND_DEL - 1; // 1 less for eTT
 
 std::map<ChunkPos, Chunk*> Chunking::chunks;
 
@@ -83,7 +84,7 @@ void Chunking::untrackEntity(ChunkPos chunkPos, const EntityRef ref) {
 }
 
 template<class T, size_t N>
-static void sendAroundPacket(const EntityRef recipient, std::vector<Bucket<T, N>>& buckets, uint32_t packetId) {
+static void sendAroundPackets(const EntityRef recipient, std::vector<Bucket<T, N>>& buckets, uint32_t packetId) {
     assert(recipient.kind == EntityKind::PLAYER);
 
     uint8_t pktBuf[CN_PACKET_BUFFER_SIZE];
@@ -100,7 +101,7 @@ static void sendAroundPacket(const EntityRef recipient, std::vector<Bucket<T, N>
 }
 
 template<size_t N>
-static void sendAroundDelPacket(const EntityRef recipient, std::vector<Bucket<int32_t, N>>& buckets, bool isTransportation, uint32_t packetId) {
+static void sendAroundDelPackets(const EntityRef recipient, std::vector<Bucket<int32_t, N>>& buckets, uint32_t packetId) {
     assert(recipient.kind == EntityKind::PLAYER);
 
     uint8_t pktBuf[CN_PACKET_BUFFER_SIZE];
@@ -110,7 +111,7 @@ static void sendAroundDelPacket(const EntityRef recipient, std::vector<Bucket<in
         assert(count <= N);
 
         size_t baseSize;
-        if (isTransportation) {
+        if (packetId == P_FE2CL_AROUND_DEL_TRANSPORTATION) {
             sP_FE2CL_AROUND_DEL_TRANSPORTATION* pkt = (sP_FE2CL_AROUND_DEL_TRANSPORTATION*)pktBuf;
             pkt->eTT = 3;
             pkt->iCnt = count;
@@ -131,23 +132,21 @@ static void sendAroundDelPacket(const EntityRef recipient, std::vector<Bucket<in
 template<class T, size_t N>
 static void bufferAppearanceData(std::vector<Bucket<T, N>>& buckets, const T& data) {
     if (buckets.empty())
-        buckets.push_back(Bucket<T, N>());
-    Bucket<T, N>& bucket = buckets[buckets.size() - 1];
-    assert(!bucket.isFull());
+        buckets.push_back({});
+    auto& bucket = buckets[buckets.size() - 1];
     bucket.add(data);
     if (bucket.isFull())
-        buckets.push_back(Bucket<T, N>());
+        buckets.push_back({});
 }
 
 template<size_t N>
 static void bufferIdForDisappearance(std::vector<Bucket<int32_t, N>>& buckets, int32_t id) {
     if (buckets.empty())
-        buckets.push_back(Bucket<int32_t, N>());
-    Bucket<int32_t, N>& bucket = buckets[buckets.size() - 1];
-    assert(!bucket.isFull());
+        buckets.push_back({});
+    auto& bucket = buckets[buckets.size() - 1];
     bucket.add(id);
     if (bucket.isFull())
-        buckets.push_back(Bucket<int32_t, N>());
+        buckets.push_back({});
 }
 
 void Chunking::addEntityToChunks(std::set<Chunk*> chnks, const EntityRef ref) {
@@ -177,8 +176,7 @@ void Chunking::addEntityToChunks(std::set<Chunk*> chnks, const EntityRef ref) {
                 sNPCAppearanceData npcData;
                 sShinyAppearanceData eggData;
                 sTransportationAppearanceData busData;
-                switch(otherRef.kind)
-                {
+                switch(otherRef.kind) {
                 case EntityKind::PLAYER:
                     pcData = dynamic_cast<Player*>(other)->getAppearanceData();
                     bufferAppearanceData(pcAppearances, pcData);
@@ -216,17 +214,16 @@ void Chunking::addEntityToChunks(std::set<Chunk*> chnks, const EntityRef ref) {
         }
     }
 
-    if (ref.kind != EntityKind::PLAYER)
-        return; // nothing to send
-
-    if (!pcAppearances.empty())
-        sendAroundPacket(ref, pcAppearances, P_FE2CL_PC_AROUND);
-    if (!npcAppearances.empty())
-        sendAroundPacket(ref, npcAppearances, P_FE2CL_NPC_AROUND);
-    if (!shinyAppearances.empty())
-        sendAroundPacket(ref, shinyAppearances, P_FE2CL_SHINY_AROUND);
-    if (!transportationAppearances.empty())
-        sendAroundPacket(ref, transportationAppearances, P_FE2CL_TRANSPORTATION_AROUND);
+    if (ref.kind == EntityKind::PLAYER) {
+        if (!pcAppearances.empty())
+            sendAroundPackets(ref, pcAppearances, P_FE2CL_PC_AROUND);
+        if (!npcAppearances.empty())
+            sendAroundPackets(ref, npcAppearances, P_FE2CL_NPC_AROUND);
+        if (!shinyAppearances.empty())
+            sendAroundPackets(ref, shinyAppearances, P_FE2CL_SHINY_AROUND);
+        if (!transportationAppearances.empty())
+            sendAroundPackets(ref, transportationAppearances, P_FE2CL_TRANSPORTATION_AROUND);
+    }
 }
 
 void Chunking::removeEntityFromChunks(std::set<Chunk*> chnks, const EntityRef ref) {
@@ -236,7 +233,7 @@ void Chunking::removeEntityFromChunks(std::set<Chunk*> chnks, const EntityRef re
     std::vector<Bucket<int32_t, MAX_IDS_PER_AROUND_DEL>> pcDisappearances;
     std::vector<Bucket<int32_t, MAX_IDS_PER_AROUND_DEL>> npcDisappearances;
     std::vector<Bucket<int32_t, MAX_IDS_PER_AROUND_DEL>> shinyDisappearances;
-    std::vector<Bucket<int32_t, MAX_IDS_PER_AROUND_DEL - 1>> transportationDisappearances;
+    std::vector<Bucket<int32_t, MAX_TRANSPORTATION_IDS_PER_AROUND_DEL>> transportationDisappearances;
     for (Chunk *chunk : chnks) {
         for (const EntityRef otherRef : chunk->entities) {
             // skip oneself
@@ -253,8 +250,7 @@ void Chunking::removeEntityFromChunks(std::set<Chunk*> chnks, const EntityRef re
             // notify this *player* of the departure of all visible Entities
             if (ref.kind == EntityKind::PLAYER && other->isExtant()) {
                 int32_t id;
-                switch(otherRef.kind)
-                {
+                switch(otherRef.kind) {
                 case EntityKind::PLAYER:
                     id = dynamic_cast<Player*>(other)->iID;
                     bufferIdForDisappearance(pcDisappearances, id);
@@ -286,17 +282,16 @@ void Chunking::removeEntityFromChunks(std::set<Chunk*> chnks, const EntityRef re
         }
     }
 
-    if (ref.kind != EntityKind::PLAYER)
-        return; // nothing to send
-
-    if (!pcDisappearances.empty())
-        sendAroundDelPacket(ref, pcDisappearances, false, P_FE2CL_AROUND_DEL_PC);
-    if (!npcDisappearances.empty())
-        sendAroundDelPacket(ref, npcDisappearances, false, P_FE2CL_AROUND_DEL_NPC);
-    if (!shinyDisappearances.empty())
-        sendAroundDelPacket(ref, shinyDisappearances, false, P_FE2CL_AROUND_DEL_SHINY);
-    if (!transportationDisappearances.empty())
-        sendAroundDelPacket(ref, transportationDisappearances, true, P_FE2CL_AROUND_DEL_TRANSPORTATION);
+    if (ref.kind == EntityKind::PLAYER) {
+        if (!pcDisappearances.empty())
+            sendAroundDelPackets(ref, pcDisappearances, P_FE2CL_AROUND_DEL_PC);
+        if (!npcDisappearances.empty())
+            sendAroundDelPackets(ref, npcDisappearances, P_FE2CL_AROUND_DEL_NPC);
+        if (!shinyDisappearances.empty())
+            sendAroundDelPackets(ref, shinyDisappearances, P_FE2CL_AROUND_DEL_SHINY);
+        if (!transportationDisappearances.empty())
+            sendAroundDelPackets(ref, transportationDisappearances, P_FE2CL_AROUND_DEL_TRANSPORTATION);
+    }
 }
 
 static void emptyChunk(ChunkPos chunkPos) {