Skip to content

Native MafiaNet replication (ReplicaManager3 + RPC4 + DeltaSerializer)#201

Open
Segfaultd wants to merge 70 commits into
developfrom
feature/mafianet-native-replication
Open

Native MafiaNet replication (ReplicaManager3 + RPC4 + DeltaSerializer)#201
Segfaultd wants to merge 70 commits into
developfrom
feature/mafianet-native-replication

Conversation

@Segfaultd

@Segfaultd Segfaultd commented Jun 2, 2026

Copy link
Copy Markdown
Member

Important

MAJOR netcode break — client and server must be updated together.

TL;DR

Replaces the hand-rolled flecs + BitStream streaming layer with native MafiaNet plugins (ReplicaManager3, RPC4, VariableDeltaSerializer) and removes flecs from the framework core entirely.

Scope 66 files, +1,800 / −2,425 (net simplification)
Risk MAJOR — netcode + sync flow; client & server must ship together
Downstream MafiaHub/MafiaMP migration/mafianet-replicas

What changed

🔁 Replication core — networking/replication/

The replicated world is now the MafiaNet plugin itself, not a flecs layer on top of it.

  • NetworkEntity : VirtualWorldReplica3 — owns its state as plain members. Per-tick updates ride VariableDeltaSerializer (a variable is sent only when it changes); construction sends a full snapshot.
  • ReplicationManager : ReplicaManager3 + NetworkIDManager + GridSectorizer are the world — it creates/destroys entities, resolves them by NetworkID, tracks each connection's viewer entity, and drives interest management.
  • Authority is keyed on ownerGUID via QuerySerialization: the server serializes to everyone except the owner, the owning client serializes upstream, and deserialize accepts state only from the current owner.

🗑️ Legacy world engine removed

Removed Replaced by
World::Engine facade ReplicationManager (used directly)
world/ server & client
game_sync messages, world/game_rpc native replication / RPC4
CoreModules::GetWorldEngine() CoreModules::GetReplication()

flecs is gone from the framework core.

📡 RPC over RPC4

  • Handlers registered as slots, dispatched via Signal() and routed through RPC4's per-slot context — no file-static handler pointers.
  • Typed payload helpers on NetworkPeer: RegisterRPC / BroadcastRPC / SendRPC.

🎮 Server-authoritative overrides

  • NetworkEntity::ForceState / WriteForcedState / OnStateForced — push state onto an owning client so the server gets the last word.
  • SetOwner — grant ownership directly to a client (serialize to an owner is withheld, so the grant can't ride normal replication).

✨ Other improvements

  • VirtualWorld dimension scoping — entities derive from VirtualWorldReplica3, so dimension filtering happens natively before the topology decision.
  • JS-safe NetworkIDs — server hands out small sequential ids within JavaScript's 2⁵³ exact-integer range, so scripts can hold entity ids as plain numbers.
  • Chat hoisted to the framework — transport (networking/rpc/chat_message.h, native std::string) and the scriptable Chat API (scripting/builtins/chat.h), reusable across mods and targeting players through the base Entity handle.
  • Scripting — reusable header-only Builtins::Entity base + property.h registration helpers; accessors use SetAccessorProperty; integral setters accept any JS number.
  • Reconnect leak fixedOnClosedConnection destroys a dropped peer's server-created viewer, so avatars no longer leak across reconnects.
  • Owned entities are never culled; interest queries optimized; dropped a committed MafiaHubServices.lib build artifact.

Status & risk

Builds

  • ✅ Framework libraries + MafiaMP server build green.
  • ✅ Client builds in a full VS env (DirectXTK shader toolchain).

Validated in-game

  • ✅ Vehicle enter · engine + config sync · player teleport.

Not yet validated

  • ⚠️ 2-client driving sync (the ownership grant).
  • ⚠️ Broader soak testing.

Known open

  • 🐛 Forced-state can race replica construction for freshly-created entities — benign for spawn, since position rides construction.

Summary by CodeRabbit

  • New Features

    • Added chat messaging system for player-to-player and broadcast communication
    • Added support for Discord user identity integration
    • Enhanced networking replication system for improved entity synchronization
  • Chores

    • Updated MafiaNet networking library to version 0.8.0
    • Improved network disconnection handling with detailed reason tracking

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Major refactor replacing Flecs world and legacy messages with a Replica3-based replication system and RPC4 payload dispatch. Client/server flows now use RPC payload structs for identity/resources/chat, integrate ReadyEvent, and move scripting/resources to be world-independent with replication-driven entity ownership.

Changes

Replication and runtime refactor

Layer / File(s) Summary
Replication contracts and build
.gitignore, code/framework/CMakeLists.txt, src/core_modules.h, src/networking/rpc/*, src/networking/messages/messages.h, src/networking/connection.h
Adds replication sources to build, swaps CoreModules to ReplicationManager, introduces RPC payload structs and disconnection contracts, and updates message IDs.
Replicated entities and factories
src/networking/replication/network_entity.*, entity_factory.*
Defines replicated entity state/serialization/authority and a factory for type registration/creation.
Replication orchestration
replication_manager.*, replication_connection.*
Implements manager init/tick/grid/viewers, ownership/state RPCs, and per-connection interest and allocation.
Transport and RPC4 migration
network_peer.*, network_client.*, network_server.*
Moves to RPC4 slots, integrates ReadyEvent and TwoWayAuth, adds ready callbacks and disconnect/auth reason handling.
Client runtime flow
integrations/client/instance.*
Removes world-engine, bridges EmitLuaEvent to JS, handles ServerResources/ready, attaches replication, and adds chat send/receive.
Server runtime flow
integrations/server/instance.*
Removes world-engine/modules, authenticates clients, sends resources, pushes replication connections, and parses chat/commands.
Scripting and resources decoupling
integrations/*/scripting/module.*, scripting/resource/*, tests
Drops Flecs root-entity ownership, adds entity tracking and replication-driven cleanup, adjusts constructors and tests.
V8 bindings
scripting/builtins/*
Adds property helpers, Entity V8 wrapper over NetworkEntity, and Chat builtin API.
World and legacy removals
world/*, logging/formatters.h, legacy game_sync/message headers
Deletes world engine/modules and related legacy networking/RPC artifacts.
Discord wrapper
external/discord/wrapper.*
Adds GetUserId accessor.
Vendor update
vendors/mafianet/*
Upgrades to v0.8.0 and adds optional disconnect reason payload support.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server
  participant RPC4
  participant ReadyEvent
  participant ReplMgr as ReplicationManager
  Client->>Server: Connect + TwoWayAuth
  Server-->>Client: Auth OK + ServerResources(readyEventId,tickRate)
  Client->>ReadyEvent: Add(guid), Set(readyEventId)
  ReadyEvent-->>Client: ALL_SET(eventId)
  Client->>ReplMgr: Init(peer, idMgr, rpc, isServer=false)
  Server->>ReplMgr: PushReplicationConnection(guid), Init(isServer=true)
  RPC4-->>Client: EmitLuaEvent/ChatMessage
  Client-->>RPC4: ClientIdentity/ChatMessage
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • zpl-zak

Poem

A rabbit taps the net with gentle paws,
Replicas hop where Flecs once set the laws;
Packets burrow through RPC4 lanes,
Chat bubbles rise like clover after rains;
Scripts now dance, unbound and fleet—
Ready bells ring, and ticks repeat. 🐇✨

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/mafianet-native-replication

@Segfaultd Segfaultd added the experimental Experimental; not for production label Jun 2, 2026
@Segfaultd Segfaultd force-pushed the feature/mafianet-native-replication branch 3 times, most recently from 623f0b2 to c0d2dee Compare June 2, 2026 18:53
Segfaultd added 19 commits June 3, 2026 11:45
Replace the flecs+BitStream entity-sync layer with native MafiaNet
replication under networking/replication/:

- NetworkEntity : Replica3 owns its state directly (no ECS) and
  serializes per-tick updates through VariableDeltaSerializer (the
  documented ReplicaManager3 delta path) instead of a whole-object
  memcmp. Construction sends a full snapshot; Serialize() returns
  RM3SR_SERIALIZED_UNIQUELY and only changed variables go on the wire.
- ReplicationManager (a ReplicaManager3) owns the entity set, the
  per-connection viewer map and a configurable GridSectorizer interest
  index. DestroyEntity only clears a viewer mapping for actual viewer
  entities (owned non-viewer entities share the owner GUID). The grid
  inserts a small box per entity (GridSectorizer asserts on zero area)
  and is sized for a 20km map by default, configurable via ConfigureGrid.
- ReplicationConnection drives QUERY_CONNECTION_FOR_REPLICA_LIST relevance.
- EntityFactory maps a CRC32 type id to a constructor for both peers.

Authority (C1) is enforced by a custom QuerySerialization keyed on
ownerGUID plus a stale-owner deserialize gate.

Deletes the game_sync/* streamer messages.
@
Remove the IRPC/IGameRPC class hierarchy and the per-type RPC4 slot
trampoline (Rpc4Slot<T>) that bridged capturing handlers onto RPC4.

RPCs are now plain payload structs (a stable kIdentifier + a symmetric
Serialize) dispatched by RPC4 to ordinary C function handlers — the only
shape RPC4 accepts. Helpers live in networking/rpc/rpc.h
(Register/Read/Broadcast/SendTo). There is no separate "game RPC": an
entity-scoped call simply carries a NetworkID field the handler resolves
through the ReplicationManager. Identifiers are explicit namespaced
literals, not typeid (stable across compilers/binaries).

NetworkPeer keeps the RPC4/NetworkIDManager/StatisticsHistory/Replication
subsystems and exposes GetRPC(); the typed RegisterRPC/SendRPC/SendGameRPC
surface is gone. NetworkServer gains BroadcastRPCExcept for the
broadcast-to-all-but-one case (RPC4::Signal has no exclusion param).

Drops the dead INTERNAL_RPC dispatcher id and the GAME_SYNC_* message ids
(entity sync is native now); client_connection_finalized no longer carries
the server entity id.
@
The world engine no longer runs a flecs streaming world. Engine/Server/
ClientEngine are thin facades over ReplicationManager: entities are
NetworkEntity*, with CreateEntity(typeId)/RemoveEntity/SetOwner/GetOwner/
GetEntityByNetworkID. The flecs world is retained only for the scripting
resource tree.

Removes the streaming systems (StreamEntities/AssignEntityOwnership/
TickRateRegulator/...), the Streamable/Streamer/Transform/ServerID
components and modules_impl.cpp, the SetTransform/SetFrame game RPCs, and
the dead client entity-destroy callback (override NetworkEntity instead).

Send macros are now native and global: FW_BROADCAST_RPC / FW_SEND_RPC_TO
(engine.h) and FW_SERVER_BROADCAST_RPC_EXCEPT (server.h); the entity-scoped
*_GAME_RPC macros are gone. Player/StreamingFactory just stamp ownership
(and viewer, server-side); nickname/hwid belong on the game entity now.
@
Server: the player avatar is created by the game in its onPlayerConnect
handler (which now receives just the GUID) and registered as the viewer;
disconnect clears the viewer mapping and lets ReplicaManager3 tear down
the player entities. ClientConnectionFinalized no longer carries an
entity id.

Client: the local avatar arrives via replication (recognised by
ownerGUID == myGUID); the connection-finalized callback only carries the
tick rate. EmitLuaEvent is now a native RPC payload handled by a plain
RPC4 function that reaches the scripting engine via CoreModules.

CMake: build the networking/replication sources; drop modules_impl.cpp.
@
Drop the FW_*_RPC send macros in favour of typed wrapper methods:
NetworkPeer::RegisterRPC<T>/BroadcastRPC<T>/SendRPC<T> and
NetworkServer::BroadcastRPCExcept<T>. rpc.h keeps only the Read<T> decode
helper for handlers.

Reword comments that narrated the migration into plain descriptions of
the current code.
@
ReplicaManager3 ran at its default 30ms autoserialize interval while the
server advertised ServerConfig::tickInterval (60Hz default) to clients.
Apply the configured rate via SetAutoSerializeInterval: the server uses
cfg.tickInterval, the client uses the rate it receives at OnConnect. Drop
the now write-only _cfg member.
@
Make NetworkServer::SignalExcept public so games can relay a raw RPC4
bitstream to all systems except the originator (the server-authoritative
event relay), and drop the unused typed BroadcastRPCExcept<T> wrapper.
Add scripting/builtins/property.h with RegisterProperty /
RegisterReadonlyProperty (scalars + strings) and RegisterObjectProperty /
RegisterReadonlyObjectProperty (v8pp-wrapped values like Color/Vector3).
The getter/setter are template parameters so V8's accessor callbacks stay
non-capturing, replacing the per-class boilerplate (and macros) that builtins
were each duplicating.
The owner is authoritative over a replicated entity's transform and the
server withholds serialize updates to it, so the server cannot relocate an
owned entity (teleport) through normal replication. Add a built-in mechanism:

- NetworkEntity::ForceTransform() (server) pushes the entity's current
  position/rotation to its owner via a built-in Framework::ForceTransform
  RPC4 call; no-op for unowned entities.
- NetworkEntity::OnTransformForced() (client) is invoked after the framework
  applies the received transform, so games apply it to their engine (teleport,
  world preload, ...).

ReplicationManager::Init takes the RPC4 and registers the handler; it exposes
ForceTransform(entity).
Generalize the transform-only ForceTransform into entity-defined
forced state: WriteForcedState/ReadForcedState/OnStateForced and
ForceState, so the server can override any owned entity (the server
always has the last word) while the owner stays authoritative for its
own streaming.

Assign small sequential NetworkIDs on the server instead of RakNet's
random 64-bit ones, which exceed JavaScript's 2^53 exact-integer range
and corrupt entity ids when scripts read them.

Register RPC handlers as RPC4 slots so Signal() actually dispatches
them; RegisterFunction handlers are only reached by Call().
Add header-only Framework::Scripting::Builtins::Entity, a reusable
replicated-entity scripting handle (NetworkID resolved via the world
engine, with a server-authoritative position/rotation routed through
ForceState) that mods can derive from.

Register read/write properties with SetAccessorProperty instead of
SetNativeDataProperty: a native-data-property setter installed on a
prototype is never invoked when a script assigns to the property on an
instance, so the write was silently dropped.
Add NetworkEntity::SetOwner / ReplicationManager::SetOwner: set the
owner and Signal a built-in grant RPC straight to the new owner. The
server withholds serialize to an entity's owner, so the grant cannot
ride normal replication; other peers and a revoked previous owner
still learn it through serialize, and the Deserialize authority gate
rejects stale-owner uploads during the handover. ServerEngine::SetOwner
routes through it. Lets a client become authoritative for an entity it
gains after construction (e.g. a vehicle it drives).
Describe what the scripting/networking helpers do instead of justifying
them against the previous or alternative API.
Provide a reusable chat feature shared by all mods: a ChatMessage RPC
payload, a global JS Chat builtin (sendToAll / sendToPlayer over the
base Entity handle), server-side receive/parse/dispatch exposed via
chat callbacks, and client-side send plus a received-message callback.

Mods keep only their own UI and a thin bridge from the chat callbacks
to their scripting events.
@
MafiaNet v0.6.0 adds a per-registration void* context to RegisterSlot,
so handlers no longer need a file-static instance pointer to find their
owner. RegisterRPC<T> now takes a decoded-payload callable, keeps it
alive for the peer's lifetime, and dispatches it via a single trampoline
that recovers the handler from the slot context. The replication and
chat handlers capture their instance directly; the g_manager,
g_chatInstance and g_chatClientInstance globals are gone.
SignalExcept iterated the connection list to skip the sender; RPC4 Signal
already treats the system identifier as the peer to exclude when
broadcasting, so one call does it. Also drops a const_cast on the
already-const GetReplicaCount in ForEachEntity (the one on the non-const
GetReplicaAtIndex stays until MafiaNet const-qualifies it).
MafiaNet v0.6.1 const-qualifies ReplicaManager3::GetReplicaAtIndex, so
ForEachEntity can iterate replicas without casting away const.
MafiaNet's BitStream has first-class std::string Write/Read overloads
using the same length-prefixed wire format as RakString, so the manual
write/read branch collapses to a single symmetric Serialize call. Wire
format is unchanged.
@Segfaultd Segfaultd force-pushed the feature/mafianet-native-replication branch from 3a59a5d to fb22bfd Compare June 3, 2026 09:48
Segfaultd added 4 commits June 3, 2026 12:00
Derive NetworkEntity from VirtualWorldReplica3 instead of Replica3 so
the dimension is owned by the base (Get/SetVirtualWorld) rather than a
redundant field. The two topology query overrides become the base's
QueryConstructionWithinWorld / QuerySerializationWithinWorld hooks.

Streaming now filters with VirtualWorldsCanSee, gaining the
VIRTUAL_WORLD_GLOBAL "visible everywhere" sentinel the old equality
check could not express. QueryReplicaList syncs the connection's world
to its avatar so the construction filter and the base serialize-path
filter agree. virtualWorld is dropped from the construction snapshot as
server-only streaming metadata.
Entity replication is fully native (ReplicaManager3 / NetworkEntity) and
the scripting Entity builtin wraps NetworkEntity by id, so the flecs world
only backed the resource tree -- which duplicated ResourceManager's own
std::map registry. Drop flecs entirely from the framework:

- Engine: remove the flecs::world, GetWorld(), and the progress() tick;
  it is now purely the replication facade.
- Resource/ResourceManager: drop _rootEntity, OwnedResource, GetRootEntity,
  DestroyChildEntities, child_of and the flecs::world* ctor params. The
  existing std::map registry is the tree.
- Delete the empty Base/Mod modules, the dead logging/formatters.h, the
  module import calls and server InitModules(); remove the dead
  _weatherManager member and stale flecs includes.
- Drop flecs_static from the framework link.
- Move the resource tests off the flecs::world fixture.

Breaking change: Engine no longer exposes an ECS world. MafiaMP is
unaffected; other mods migrate later.
Post-flecs the World::Engine / ServerEngine / ClientEngine hierarchy
owned nothing -- it was a pure forwarder to the ReplicationManager, which
is owned by the NetworkPeer. Remove the layer entirely and promote the
ReplicationManager to the top-level networked-world object.

- CoreModules: replace Get/SetWorldEngine (World::Engine*) with
  Get/SetReplication (ReplicationManager*), set from the peer on server
  init / client connect and cleared on shutdown / disconnect.
- Delete world/engine, world/server, world/client and world/errors.
- Entity CRUD, ownership and the auto-serialize tick rate now go straight
  to the ReplicationManager; peer-lifecycle wiring moved to the
  integration Instances.
- Scripting modules no longer take a world-engine pointer.

Breaking change. MafiaMP is updated in lockstep; other mods migrate later.
The 5.7 MB prebuilt static lib was committed by mistake in f8e0882.
Remove it and gitignore the services/lib Debug/Release output dirs so it
cannot be re-added.
Segfaultd added 5 commits June 3, 2026 19:08
The connection handshake is fully native now (TwoWayAuthentication,
RPC4, ReadyEvent, ReplicaManager3), and MafiaMP uses only that
paradigm. ClientInitPlayer was the last framework-owned IMessage and
was already inert, so remove the whole legacy layer:

- Delete client_initialise_player.h and GAME_INIT_PLAYER
- Strip IMessage and the GameMessages enum from messages.h, keeping
  DisconnectionReason and the connect/disconnect callback typedefs
- Remove NetworkPeer::Send(IMessage&) and RegisterMessage overloads,
  the _registeredMessageCallbacks map, and its now-unused includes
- Simplify the receive loop to fall straight to the unknown handler
Nothing message-related is left in the file after the IMessage
removal — only DisconnectionReason and the connect/disconnect
callback aliases. Move them to networking/connection.h under
Framework::Networking (dropping the now-pointless ::Messages
sub-namespace) and delete the empty messages/ directory.
Brings in the native disconnect-reason payload on CloseConnection /
ID_DISCONNECTION_NOTIFICATION (MafiaNet #24, tag v0.8.0), synced from
develop. No local patches on the vendored tree.
Use MafiaNet v0.8.0's CloseConnection reasonData instead of a separate
Kick RPC raced against the disconnect. The server serializes the reason
(+ custom string) into the ID_DISCONNECTION_NOTIFICATION body; the
client decodes it from the notification packet and surfaces it through
the disconnect callback.

- Add DisconnectPayload and extend DisconnectPacketCallback with the
  custom reason string in connection.h
- KickPlayer now CloseConnection(..., &reasonData); drop SendRPC
- Client decodes the optional payload in the ID_DISCONNECTION_NOTIFICATION
  handler, falling back to GRACEFUL_SHUTDOWN for a bodyless disconnect
- Fold the reason->message mapping into the disconnect callback and
  delete the now-unused rpc/kick.h
- network_peer.h: include MessageIdentifiers.h directly (was lost as a
  transitive include in the messages.h -> connection.h rename)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
vendors/mafianet/Source/src/RakPeer.cpp (1)

1676-1681: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the unknown SystemIndex sentinel instead of coercing it to peer 0.

Line 1680 turns an unresolved slot into 0, but the rest of RakPeer.cpp uses (SystemIndex)-1 as the invalid sentinel. On the churn path this synthesizes an ID_CONNECTION_LOST packet that can look like it belongs to peer 0, so code that keys off packet->systemAddress.systemIndex / packet->guid.systemIndex can tear down the wrong connection.

Suggested fix
-		packet->systemAddress.systemIndex = static_cast<SystemIndex>(remoteSystemListIndex == -1 ? 0 : remoteSystemListIndex);
+		packet->systemAddress.systemIndex =
+			remoteSystemListIndex == -1 ? static_cast<SystemIndex>(-1)
+			                            : static_cast<SystemIndex>(remoteSystemListIndex);
 		packet->guid.systemIndex=packet->systemAddress.systemIndex;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vendors/mafianet/Source/src/RakPeer.cpp` around lines 1676 - 1681, The code
coerces an unresolved remoteSystemListIndex into 0 when building the
ID_CONNECTION_LOST Packet, which hides the intended sentinel and can
misattribute disconnects; change the assignment for
packet->systemAddress.systemIndex and packet->guid.systemIndex to preserve the
invalid sentinel (i.e. do not map -1 to 0) so the unresolved slot remains
(SystemIndex)-1 — locate the AllocPacket/Packet creation where
packet->systemAddress.systemIndex is set (currently using remoteSystemListIndex
== -1 ? 0 : remoteSystemListIndex) and assign the remoteSystemListIndex value
directly (using static_cast<SystemIndex> if needed) so the sentinel is preserved
for both packet->systemAddress.systemIndex and packet->guid.systemIndex.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@vendors/mafianet/Source/src/RakPeer.cpp`:
- Around line 6149-6162: The disconnect reason bytes copied in the
ID_DISCONNECTION_NOTIFICATION path are unbounded; clamp the copied length to a
defined small constant (e.g., MAX_DISCONNECT_REASON_LENGTH) before allocating
and copying into remoteSystem->disconnectReasonData in the
ClearDisconnectReason/receive block, set remoteSystem->disconnectReasonLength to
the clipped size, and ensure the same MAX_DISCONNECT_REASON_LENGTH is enforced
in NotifyAndFlagForShutdown when sending/attaching a reason so both sides use
the same protocol limit; update any checks in RunUpdateCycle handling the
synthesized packet to rely on remoteSystem->disconnectReasonLength (already set)
rather than trusting incoming byteSize.

---

Outside diff comments:
In `@vendors/mafianet/Source/src/RakPeer.cpp`:
- Around line 1676-1681: The code coerces an unresolved remoteSystemListIndex
into 0 when building the ID_CONNECTION_LOST Packet, which hides the intended
sentinel and can misattribute disconnects; change the assignment for
packet->systemAddress.systemIndex and packet->guid.systemIndex to preserve the
invalid sentinel (i.e. do not map -1 to 0) so the unresolved slot remains
(SystemIndex)-1 — locate the AllocPacket/Packet creation where
packet->systemAddress.systemIndex is set (currently using remoteSystemListIndex
== -1 ? 0 : remoteSystemListIndex) and assign the remoteSystemListIndex value
directly (using static_cast<SystemIndex> if needed) so the sentinel is preserved
for both packet->systemAddress.systemIndex and packet->guid.systemIndex.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 21009929-831f-4c10-9a73-7f9ea818b3e1

📥 Commits

Reviewing files that changed from the base of the PR and between aa4be13 and 51d7b05.

📒 Files selected for processing (19)
  • code/framework/CMakeLists.txt
  • code/framework/src/integrations/client/instance.cpp
  • code/framework/src/integrations/server/instance.cpp
  • code/framework/src/networking/connection.h
  • code/framework/src/networking/messages/client_initialise_player.h
  • code/framework/src/networking/messages/messages.h
  • code/framework/src/networking/network_client.cpp
  • code/framework/src/networking/network_client.h
  • code/framework/src/networking/network_peer.cpp
  • code/framework/src/networking/network_peer.h
  • code/framework/src/networking/network_server.cpp
  • code/framework/src/networking/network_server.h
  • vendors/mafianet/CMakeLists.txt
  • vendors/mafianet/README.md
  • vendors/mafianet/Source/include/mafianet/peer.h
  • vendors/mafianet/Source/include/mafianet/peerinterface.h
  • vendors/mafianet/Source/include/mafianet/version.h
  • vendors/mafianet/Source/src/RakPeer.cpp
  • vendors/mafianet/VERSION.txt
💤 Files with no reviewable changes (2)
  • code/framework/src/networking/messages/client_initialise_player.h
  • code/framework/src/networking/messages/messages.h
✅ Files skipped from review due to trivial changes (4)
  • vendors/mafianet/CMakeLists.txt
  • vendors/mafianet/VERSION.txt
  • vendors/mafianet/Source/include/mafianet/version.h
  • vendors/mafianet/README.md
🚧 Files skipped from review as they are similar to previous changes (6)
  • code/framework/CMakeLists.txt
  • code/framework/src/networking/network_peer.cpp
  • code/framework/src/networking/network_server.h
  • code/framework/src/integrations/client/instance.cpp
  • code/framework/src/networking/network_server.cpp
  • code/framework/src/integrations/server/instance.cpp

Comment thread vendors/mafianet/Source/src/RakPeer.cpp
Segfaultd added 24 commits June 3, 2026 22:15
Ship Framework::Scripting::Builtins::Player : Entity so connection-level
operations live on a player handle instead of every entity. It exposes
kick(reason), which resolves the owning connection and disconnects it
with a reason via the native CloseConnection path.

- NetworkPeer gains a virtual KickPlayer (no-op base, NetworkServer
  override) so the header-only Player binding can kick through a
  NetworkPeer* without a cast, server or client
- Trim the verbose comments added with the disconnect-reason work
Register the built-in ForceState/SetOwner RPC4 slots on the client
only. They are server->owner pushes, so a server that also registered
them would dispatch a peer's forged Signal and let a client teleport
any entity or reassign ownership it does not hold. Unregister them on
manager teardown, since _rpc outlives the manager in the peer's member
order and a late signal would otherwise dispatch into a freed manager.

Replace the per-connection O(entities) QueryReplicaList scan with the
connection's working set: construct candidates come from the in-range
grid set, the entities it owns, the always-visible set and its avatar;
destruction walks GetConstructedReplicas. The owned/always-visible
indices are rebuilt each Tick from the live entity set so they stay
correct against direct ownerGUID/alwaysVisible writes, and DestroyEntity
scrubs them so an intra-tick delete cannot dangle.

On a locally-initiated client Disconnect, pass a null packet rather than
a stale _packet to the disconnect callback.
The per-resource hash was always written as 0 and never read on the client; file integrity and deltas are handled natively by DirectoryDeltaTransfer. Remove it from the wire format and both resource structs.
alwaysVisible/visible/isViewer/range were loose public fields on NetworkEntity, present (and dead) on the client too. Group them under a documented server-only Streaming sub-struct so the surface reads honestly and the per-entity wire state isn't conflated with interest policy.
Add a single typed Manager() accessor so the ReplicaManager3* downcast lives in one place instead of being repeated at six call sites. Fold the duplicated server-authoritative owner-adoption logic (construction + delta paths) into AdoptIncomingOwner. Make typeId private (factory-friended) so the wire identity is no longer game-settable.
The spatial index, its lazy init, the owned-by-guid / always-visible sets, the per-tick rebuild and the radius query were spread through the manager. Move them into a dedicated InterestGrid so the relevance data and the queries over it are one cohesive unit; the manager just drives it. Rename Tick() to RebuildInterest() to say what it does next to ReplicaManager3::Update().
Detect CRC32 collisions at Register() and log them instead of silently shadowing the earlier type. Add a typed Register<T>(name) overload that removes the per-site constructor lambda, and store the registered name in Entry for the collision diagnostic.
App::OnContextCreated was defined but never declared, and the App
class never overrode GetRenderProcessHandler despite inheriting
CefRenderProcessHandler (C2509). Declare both, and compile
renderer_app.cpp into FrameworkClient so CallEventHandler::Execute
resolves at link time.
Replace the mirrored Write/Read pairs on NetworkEntity (construction snapshot, forced state, construction extension hook) with single Serialize(bs, write) functions over BitStream::Serialize, matching the RPC layer. Per-tick fields now flow through a FieldSerializer adapter so the same Field(member) call serializes and deserializes, removing the silent field-order desync footgun for subclasses.
Self-registration helper for NetworkEntity subclasses, mirroring MafiaNet::RPC4GlobalRegistration. One line ties a subclass to its wire name/type id, removing the forgot-to-register and name-mismatch footguns.
Move the visible() relevance predicate and candidate gathering out of ReplicationConnection into InterestGrid::CollectVisible, so the server's relevance rule lives in one place. QueryReplicaList now just diffs the relevant set against already-constructed replicas. Drops the EntitiesOwnedBy/AlwaysVisibleEntities/QueryRadius pass-throughs on ReplicationManager in favour of a single CollectInterest facade.
ReplicationManager::Init now takes its owning NetworkPeer and routes the ForceState/SetOwner pushes through the peer's RPC helpers instead of poking RPC4 directly. SetOwner uses a typed SetOwnerRPC payload; ForceState uses a new RegisterRawRPC/SendRawRPC pair since its tail is polymorphic (resolve the entity by id, then let it deserialize). Handlers now live on the peer for its lifetime, removing the manual UnregisterSlot bookkeeping.
The type both registers entity types and constructs them; "registry" is the more honest noun. Renames the class and its files, and documents that CreateEntity returns a non-owning pointer (ReplicaManager3 owns and deletes the entity).
Register<T>(name) already covers one-line registration without a macro.
The Replica3/VirtualWorldReplica3 plumbing overrides are framework-owned;
games extend NetworkEntity through the virtual hooks (SerializeFields,
OnSerializeConstruction, OnConstructed, SerializeForcedState, OnStateForced),
never by reimplementing the wire/authority methods. Mark those overrides
final so the compiler enforces it, and return the *WithinWorld hooks to
protected to match VirtualWorldReplica3 rather than widening their access.
NetworkID is a bare typedef uint64_t, so peer guids and entity ids were the same type to the compiler. Wrap a peer guid in enum class PeerGuid (trivially copyable -> identical on the wire) and use it across the owner/viewer/disconnect surface, with ToPeerGuid/ToGuid at the MafiaNet boundary.
…ative-replication

# Conflicts:
#	vendors/mafianet/CMakeLists.txt
#	vendors/mafianet/README.md
#	vendors/mafianet/Source/include/mafianet/version.h
#	vendors/mafianet/VERSION.txt
Drop the homemade PeerGuid enum/helpers in favour of MafiaNet::PeerGuid (vendored v0.9.0) and carry it through the owner/viewer/disconnect surface, removing the uint64_t casts at the boundaries.
The per-tick VariableDeltaSerializer path only carries changes against a shared baseline, so a late-constructing connection never received a field that was not actively changing (a parked car's colour, an idle player's health). Run SerializeFields once verbatim in the construction snapshot via a plain FieldSerializer backend to seed the full current state.
On disconnect, return any non-viewer entity the peer owned (e.g. a vehicle it was driving) to the server; otherwise the authority gate freezes it against an owner that no longer exists.
The exact-hash build-token handshake replaced the semver-range version gate; nothing calls VersionSatisfies.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experimental Experimental; not for production

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants