diff --git a/code/framework/src/integrations/server/instance.cpp b/code/framework/src/integrations/server/instance.cpp index 29dcba377..cfec1e0f3 100644 --- a/code/framework/src/integrations/server/instance.cpp +++ b/code/framework/src/integrations/server/instance.cpp @@ -684,17 +684,19 @@ namespace Framework::Integrations::Server { }); // Voice frames are not RPCs: RakVoice writes a raw message id, so they surface on the - // unknown-packet path (the relay host deliberately declines to consume them itself). + // fallback handler (the relay host deliberately declines to consume them itself). net->SetUnknownPacketHandler([this, net](MafiaNet::Packet *packet) { // GetPacketDataOffset() is the offset the peer resolved for this very packet, so an // ID_TIMESTAMP prefix is already skipped. const int offset = net->GetPacketDataOffset(); if (offset < 0 || static_cast(offset) >= packet->length) { - return; + return false; } if (packet->data[offset] == ID_RAKVOICE_RELAY_DATA) { _voiceServer.OnVoiceFrame(packet); + return true; } + return false; }); Logging::GetLogger(FRAMEWORK_INNER_SERVER)->debug("Networking messages registered"); diff --git a/code/framework/src/networking/network_peer.cpp b/code/framework/src/networking/network_peer.cpp index ecf19dd75..6f8baa1ae 100644 --- a/code/framework/src/networking/network_peer.cpp +++ b/code/framework/src/networking/network_peer.cpp @@ -100,10 +100,10 @@ namespace Framework::Networking { if (IsReplicationPacket(packetID)) { continue; } - Logging::GetLogger(FRAMEWORK_INNER_NETWORKING)->trace("Received unknown packet {}", packetID); - if (_onUnknownPacketCallback) { - _onUnknownPacketCallback(_packet); + if (_onUnknownPacketCallback && _onUnknownPacketCallback(_packet)) { + continue; } + Logging::GetLogger(FRAMEWORK_INNER_NETWORKING)->trace("Received unknown packet {}", packetID); } } _replicationManager->EndNetworkUpdate(); diff --git a/code/framework/src/networking/network_peer.h b/code/framework/src/networking/network_peer.h index d39c10761..f6a307523 100644 --- a/code/framework/src/networking/network_peer.h +++ b/code/framework/src/networking/network_peer.h @@ -45,7 +45,7 @@ namespace Framework::Networking { MafiaNet::RakPeerInterface *_peer = nullptr; MafiaNet::Packet *_packet = nullptr; int _packetDataOffset = 0; // Offset to skip timestamp prefix if present - PacketCallback _onUnknownPacketCallback; + fu2::function _onUnknownPacketCallback; mutable MafiaNet::DirectoryDeltaTransfer _assetStreamer; // RPC4 dispatches remote-procedure calls by identifier to C handlers. NetworkIDManager hands @@ -193,7 +193,9 @@ namespace Framework::Networking { return ""; } - void SetUnknownPacketHandler(PacketCallback callback) { + // Handles packets not claimed by the peer or replication plugin. Return true when the + // packet id is recognized; only packets declined by this handler are logged as unknown. + void SetUnknownPacketHandler(fu2::function callback) { _onUnknownPacketCallback = std::move(callback); } diff --git a/code/tests/modules/network_packets_ut.h b/code/tests/modules/network_packets_ut.h index e93117266..1a46840a0 100644 --- a/code/tests/modules/network_packets_ut.h +++ b/code/tests/modules/network_packets_ut.h @@ -9,6 +9,7 @@ #pragma once #include "networking/network_peer.h" +#include "networking/network_server.h" #include "networking/rpc/chat_message.h" #include "networking/rpc/client_identity.h" #include "networking/rpc/server_resources.h" @@ -65,6 +66,70 @@ MODULE(network_packets, { EQUALS(NetworkPeer::IsReplicationPacket(ID_CONNECTION_LOST), false); }); + IT("dispatches voice frames without unknown-packet spam and logs unclaimed packets", { + Framework::Networking::NetworkServer server; + struct Shutdown { + Framework::Networking::NetworkServer &server; + ~Shutdown() { + server.Shutdown(); + } + } shutdown {server}; + // An ephemeral loopback socket lets Receive() drain injected packets without a client. + MafiaNet::SocketDescriptor socket(0, "127.0.0.1"); + EQUALS(server.GetPeer()->Startup(1, &socket, 1), MafiaNet::RAKNET_STARTED); + + // The test runner pauses logging; capture its synchronous null logger without opening files. + auto logger = Framework::Logging::GetLogger(FRAMEWORK_INNER_NETWORKING, false); + auto logs = std::make_shared(8); + logs->set_pattern("%v"); + struct RestoreLogger { + std::shared_ptr logger; + spdlog::level::level_enum level; + ~RestoreLogger() { + logger->sinks().pop_back(); + logger->set_level(level); + } + } restoreLogger {logger, logger->level()}; + logger->sinks().push_back(logs); + logger->set_level(spdlog::level::trace); + + auto enqueue = [&](uint8_t id) { + auto *packet = server.GetPeer()->AllocatePacket(1); + packet->data[0] = id; + server.GetPeer()->PushBackPacket(packet, false); + }; + int voiceFrames = 0; + int declinedPackets = 0; + server.SetUnknownPacketHandler([&](MafiaNet::Packet *packet) { + if (packet->data[server.GetPacketDataOffset()] == ID_RAKVOICE_RELAY_DATA) { + ++voiceFrames; + return true; + } + ++declinedPackets; + return false; + }); + + for (int i = 0; i < 100; ++i) { + enqueue(ID_RAKVOICE_RELAY_DATA); + } + server.Update(); + EQUALS(voiceFrames, 100); + EQUALS(logs->last_raw().empty(), true); + + enqueue(ID_USER_PACKET_ENUM); + server.Update(); + EQUALS(declinedPackets, 1); + EQUALS(logs->last_raw().size(), static_cast(1)); + EQUALS(logs->last_formatted()[0].find("Received unknown packet " + std::to_string(ID_USER_PACKET_ENUM)), static_cast(0)); + + // A known id still gets a diagnostic when no application handler claims it. + server.SetUnknownPacketHandler({}); + enqueue(ID_RAKVOICE_RELAY_DATA); + server.Update(); + EQUALS(logs->last_raw().size(), static_cast(2)); + EQUALS(logs->last_formatted()[1].find("Received unknown packet " + std::to_string(ID_RAKVOICE_RELAY_DATA)), static_cast(0)); + }); + IT("round-trips a ChatMessage payload", { RPC::ChatMessage out {}; out.text = "Expecto Patronum!";