Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the packet handling system by introducing a centralized sendPacket method and consolidating packet routing logic. The changes eliminate code duplication in packet construction and standardize the packet sending process across the codebase.
Key changes:
- Introduced
Packet::sendPacket()method to standardize packet construction and queuing - Consolidated packet routing logic into a single file with comprehensive function declarations in
packetRouter.hpp - Renamed packet handler functions to follow consistent naming conventions (e.g.,
*Packetsuffix)
Reviewed Changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/packet.cpp | Added sendPacket() method to handle packet ID, data assembly, and queuing |
| src/lib/buffer.cpp | Reorganized buffer methods and added prependVarInt() for packet length prefixing |
| src/networking/networkPacketRouter.cpp | Consolidated all packet routing logic with inline state handlers |
| include/network/packetRouter.hpp | Added comprehensive function declarations for all packet handlers |
| include/network/packet.hpp | Added sendPacket() method declaration |
| include/network/buffer.hpp | Updated method declarations to match reorganized implementation |
| src/networking/packet/clientbound/*.cpp | Refactored to use new sendPacket() method |
| src/networking/packet/serverbound/*.cpp | Renamed handlers for consistency and simplified implementations |
| src/data/*.cpp | Updated to use writeString() instead of deprecated writeIdentifier() |
| CMakeLists.txt | New CMake configuration file for project build setup |
Comments suppressed due to low confidence (1)
src/networking/packet/templateClientBoundPacket.cpp:1
- Corrected spelling of 'tesst' to 'test' in PR title.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // g_logger->logNetwork(INFO, "Handling incoming data for player", "Worker"); | ||
| packetRouter(packet, getServer()); | ||
| if (packet->getReturnPacket() == PACKET_SEND) { |
There was a problem hiding this comment.
After calling packetRouter, if PACKET_SEND is returned, the packet is set to nullptr but never pushed to the outgoing queue. The removed line _outgoingPackets.push(packet) at line 23 in the original code was handling this. This will cause packets marked for sending to be lost instead of being transmitted.
| if (packet->getReturnPacket() == PACKET_SEND) { | |
| if (packet->getReturnPacket() == PACKET_SEND) { | |
| _outgoingPackets.push(packet); |
| // g_logger->logNetwork(INFO, "Sending packet to player", "Network Manager"); | ||
| std::cout << "Sending packet 0x" << std::hex << p->getId() << " (" << std::dec << p->getData().getData().size() << " bytes)" | ||
| << std::endl; | ||
| // g_logger->logNetwork(INFO, "to player", "Network Manager"); |
There was a problem hiding this comment.
Incomplete comment fragment 'to player' should either be completed or removed. The original comment was more descriptive.
| // g_logger->logNetwork(INFO, "to player", "Network Manager"); | |
| // g_logger->logNetwork(INFO, "Sending packet to player", "Network Manager"); |
|
|
||
|
|
||
| // ============================================================ | ||
| // ========== MÉTHODES DE LECTURE ========== |
There was a problem hiding this comment.
Comments are in French ('MÉTHODES DE LECTURE', 'MÉTHODES D'ÉCRITURE', 'UTILITAIRES') while the codebase uses English. Comments should be consistent with the project's language standards.
| // ========== MÉTHODES DE LECTURE ========== | |
| // ========== READ METHODS ========== |
| if (entry.has_data && entry.data.has_value()) { | ||
| buffer.writeBool(true); // Données présentes | ||
| buffer.writeNBT("{}"); // Données NBT (vide pour l'instant) | ||
| buffer.writeBytes("{}"); // Données NBT (vide pour l'instant) |
There was a problem hiding this comment.
Comment is in French ('Données NBT (vide pour l'instant)') and should be translated to English to maintain consistency with the rest of the codebase.
| buffer.writeBytes("{}"); // Données NBT (vide pour l'instant) | |
| buffer.writeBytes("{}"); // NBT data (empty for now) |
2 participants
No description provided.