Salvage from #18: wire and transport fixes, cursor throttling, map pings and host lobby controls

[Rollocraft]

Everything from #18 that is worth keeping, rebuilt to fit this codebase. Stacks on cleanup-codebase (#21), which stacks on resync-trigger-fixes (#20), so the diff here is only the new work.

Original ideas and first implementations by @t1garbiznisbrate-ship-it; every commit that took something carries a Co-Authored-By for them.

⚠️ Not compiled

Same caveat as #20 and #21: no .NET SDK and no CS2 game assemblies were available, so nothing here has been built. What was done instead: every game API used is one this repo already calls somewhere (OverlayRenderSystem.GetBuffer/DrawCircle, CameraController.pivot, ToArchetypeChunkArray, GetEntityTypeHandle), no new Game.* type is introduced on faith; brace balance, using resolution, duplicate type declarations and namespace convention checked across all changed files.

Protocol v51

One new command id (29, map ping). Both players need this build.

1. Wire codec and socket framing

WriteFloat allocated a four-byte array per value through BitConverter.GetBytes and relied on the runtime's endianness matching the manual little-endian integer writes beside it. Floats are the densest thing on this wire — a terrain brush or a road curve is little else — so both the allocation and the unstated assumption sat on the hottest path. Writer and reader now reinterpret through an overlaid struct and lay the bytes out explicitly. Bit-identical on a little-endian host, so the wire format does not move.

The send loop wrote the length prefix and the payload as two calls. Over TLS each call is its own record, so describing four bytes of length cost ~29 bytes of record overhead, and with NoDelay set it was also its own TCP segment — close to a fixed tax per command. Payloads up to 8 KiB are now written once.

Also: 1 MiB kernel socket buffers (best-effort), a write deadline on the TLS handshake to match the read deadline that was already there, and a null-stream guard in ReadExactly — Close() clears that field from another thread, so the read loop could fault between the close and its own cancellation check.

BlobReassembler sizes its buffer to the announced total instead of doubling from the default roughly once per 256 KiB chunk. The total is already rejected above the channel's registered ceiling before construction, so the allocation stays bounded by that ceiling rather than by the sender's claim.

2. Two crash sites

NetUpgradeSyncSystem and DisasterSyncSystem read components off entities taken from a query snapshot. A delete realized between the snapshot and the loop — or the simulation reaping a finished disaster on its own cadence — leaves a handle that no longer resolves, and GetComponentData on it throws out of the entire pass, taking every other live entry with it. Both scans now check Exists plus the components they are about to read, and skip the one entry rather than losing the pass.

3. Cursor sync stops shouting

PlayerCursorSyncSystem transmitted at 10 Hz for the whole session regardless of whether the camera had moved, so a player reading a panel or sitting in a menu — the common case in a co-op session — sent ten packets a second to say nothing had changed. It now uses that cadence only while focus, eye or yaw actually move, and drops to a 1 Hz keepalive otherwise, well inside the five seconds after which a peer stops drawing the marker.

The tree channel's sweep copied every tree in the city into a NativeArray to pick at most MaxRecords of them — a six-figure allocation per sweep on a forested map, and the sweep, not the send, is what that channel costs the host. It now walks the archetype chunks the ECS already holds, with a chunk-granular round-robin cursor so every tree is still eventually visited.

4. Latency that means something

Peer latency was the raw last sample, which swings with whatever the OS was doing when the echo landed. It is now the Jacobson/Karels smoothed estimate with the standard 1/8 and 1/4 gains, and the mean deviation it produces is kept as jitter and reported beside the latency spread. A link reads as fine on the average right up until the variation is what is hurting it.

5. Clients get a player list

The roster JSON was gated on Role == Host; clients rendered nothing, so a two-player session looked like a single-player one from one side of it. Clients now build one from the host peer plus the players they track positions for.

Another client's name genuinely is not known there — no roster travels to clients — so it reads "Player 3" rather than inventing one, and an unmeasured latency is -1 so the panel can show nothing instead of a misleading zero. MultiplayerSession.HostPlayerId is public for the same reason: the game layer has to tell which entry is the host, and #18's version guessed 0 (the host is 1), which mislabelled the host on every client.

6. Map pings — as a command, not as chat text

A transient "look here" beacon with an optional note, drawn as a ring that grows and fades over six seconds in the sender's cursor colour.

#18 encoded this into a chat line (/ping x y z id label) and parsed it back out on receipt. That makes the author of the marker whoever typed the line — anyone could drop a ping signed with another player's name — and every chat line in the session had to be tested for it. As command id 29 the sender comes from the message envelope the session already authenticates, and the coordinates go through WireGuard like every other command's.

A ping mutates nothing, so it is deliberately outside the sync pipeline: no echo guard, no snapshot, no resync on loss. A ping that does not arrive is a ping nobody saw, and that is the whole failure mode. The sender records its own ring at send time rather than waiting for the echo — a host is notified of its own commands and a client is not, so relying on the echo would draw the host's pings and silently swallow every client's.

Not gated on ShowPartnerMarkers: that setting hides the ambient cursor rings, and someone who turned those off still wants to see a partner deliberately pointing at something.

7. Camera navigation and host lobby controls

/goto <player>, /goto ping, /follow, /unfollow. Chat commands hold no camera reference and have no business taking one, so they record an intent on the service and PlayerCursorSyncSystem — which already owns the CameraUpdateSystem reference — carries it out. Follow ends when the player takes their camera back, detected by the pivot drifting from where the follow lerp last put it rather than by polling UnityEngine.Input.GetKey as #18 did: nothing to maintain as keybindings change, and it cannot fire while someone is typing. Name resolution takes an exact id, then an exact name, then a unique prefix — an ambiguous prefix resolves to nothing, because following the wrong partner is worse than being asked to be specific.

/lock and /unlock refuse new joins without ending the session; everyone already connected stays. /banlist and /unban make the existing per-session address bans visible and reversible, which matters mostly for undoing a ban placed on the wrong player. All host-only, and they say so when they are not. The lock clears when hosting ends, alongside the ban list.

/help lists what exists; /clear clears the local log only and says so. An unrecognised slash word is not claimed and goes out as ordinary chat, so a typo is visible rather than silently swallowed.

8. World sync says what it is doing

Pressing sync as host with nobody connected opened an epoch, found no participants and closed it again in silence — indistinguishable from a failure. It now says so. Completion was equally quiet on both sides; each now gets a line when it finishes. SanitizeSpeed gained an upper bound: the resume speed arrives over the wire and the game's own selector tops out at 3, so a peer sending a large finite value would have had the simulation resume at it.

What was deliberately left in #18

Worth recording, because the omissions are the larger half of that PR.

The 22 new sync systems and their commands are not here, and should not be. Beyond the mechanical problems — none of them are registered in Mod.cs (SyncSystemRegistration.RegisterAll has no callers, so none of them ever run), all 15 Broadcast* entry points have no callers, four identify entities by raw ECS index/version which never agree across two Worlds, and none implement ISimulationCommand or pass through WireGuard — almost every feature they add already exists here:

#18 command	Already on dev via
BuildingToggle	the "Out of Service" policy — PolicySyncSystem/Realize.cs:255-264 handles BuildingOption.Inactive and ExtensionFlags.Disabled explicitly. #18's version adds Unity.Entities.Disabled to the building, which would remove it from every query in the game.
ServiceDistrict, ParkFee, TransitLineDetail, TransitFare, ServiceFleet	EntityPolicyCommand + PolicySyncSystem, which scans districts, routes, buildings and owned upgrades and carries each policy's Active and Adjustment (Capture.cs:16-20, :85)
TransitColor	RouteCreateCommand / RouteUpdateCommand already carry RGBA
CityBudget	ServiceBudgetStateChannel (ch 15)
CityLoan	LoanStateChannel (12)
SimulationSpeed	SimulationSpeedChannel (11)
Milestone	MilestoneStateChannel (4)
WeatherControl	WeatherStateChannel (13)
Daylight	GameClockStateChannel (14)
CustomName	EntityNameCommand (26) + CityNameStateChannel (17)
TrafficLight	NetUpgradeSyncSystem already applies the signal upgrade on a node

Landing those would mean two systems writing the same state — a desync source, not a feature.

UtilityTrade stores four booleans nothing reads and corresponds to no vanilla setting. Pollution is a deterministic simulation output. Checksum is a genuine gap and a good idea — a rolling state hash would give #20's ResyncArbiter a divergence signal it does not have today — but #18's is inert, and it is worth its own change.

Also left behind: the hardcoded D:\Games\Cities - Skylines II\...\System.Memory.dll hint path (nothing on that branch uses Span, Memory or unsafe); SessionBackupManager, whose CleanLegacyCokBackups deletes every *.cok file in a directory derived by concatenating "Low" onto LocalApplicationData; a static cached EntityQuery in ConstructionCharger that would outlive the World that owns it; the lock removal in SyncInbox; the chat flood limit loosened from 5/s to a literal 12; and ~620 lines with no call path (VarInt, BufferPool, StunClient, EntityMapTable, CommandDeduplicator.ShouldProcess, the write-only replay buffer).

Worth exercising before merge

• Follow mode's break threshold is 20 m of pivot drift. If CS2's camera applies its own smoothing to pivot, that read-back may differ from what the lerp wrote — the threshold is generous, but this is the one tuned number here.
• The tree channel's cursor is now chunk-granular rather than entity-granular. Coverage is unchanged; the visit order is not.
• Protocol v51 — both players need this build.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa53af62d8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid preallocating the entire announced blob size

When a client receives the first map chunk, the host-controlled TotalBytes may be as large as the registered 256 MiB ceiling, so this constructor immediately reserves 256 MiB even if the chunk contains only a few bytes. A malicious or malfunctioning host can repeatedly replace incomplete transfer IDs and force large allocations much faster than data arrives, potentially terminating the game with an out-of-memory failure; retain incremental growth or otherwise delay/limit allocation based on bytes actually received.

Useful? React with 👍 / 👎.

[t1garbiznisbrate-ship-it]

Fix for Blob Preallocation (Initial Capacity Ceiling)

Capping the initial buffer allocation to 4 MiB (or expectedBytes, whichever is smaller) prevents immediately reserving up to the 256 MiB channel ceiling on the Large Object Heap upon receiving the first chunk.

This directly addresses the concern where incomplete or malicious transfers could force large allocations much faster than data actually arrives:

Announced Size	Initial Allocation
100 KB	100 KB
1 MB	1 MB
4 MB	4 MB
20 MB	4 MB
100 MB	4 MB
256 MB	4 MB

MemoryStream grows dynamically as subsequent chunks arrive, while maintaining the existing channel ceiling checks for total blob validation.

(Note: Add using System; if not already present, or use Math.Min).

Suggested change

	_buffer = expectedBytes > 0 ? new MemoryStream(expectedBytes) : new MemoryStream();
	private const int InitialBufferCapacity = 4 * 1024 * 1024; // 4 MiB
	private readonly MemoryStream _buffer;
	public BlobReassembler(int expectedBytes, long nowMs)
	{
	ExpectedBytes = expectedBytes;
	LastChunkAtMs = nowMs;
	int initialCapacity = expectedBytes > 0
	? Math.Min(expectedBytes, InitialBufferCapacity)
	: 0;
	_buffer = initialCapacity > 0
	? new MemoryStream(initialCapacity)
	: new MemoryStream();
	}

[chatgpt-codex-connector]

Let ordinary camera movement cancel follow mode

Follow cancellation compares only the current frame's pivot displacement against a 20 m threshold. A normal pan usually moves less than 20 m per frame, after which the follow lerp immediately overwrites that movement and resets _followPivot, so the displacement can never accumulate; rotation and zoom do not change the pivot at all. Consequently the advertised “Move your camera to stop” behavior generally requires an abrupt one-frame jump or /unfollow instead of normal camera input.

Useful? React with 👍 / 👎.

[t1garbiznisbrate-ship-it]

This will likely need in-game testing and fine-tuning to find the ideal threshold. Alternatively, we could simplify this by removing the camera-movement break condition entirely and having /unfollow (or clicking another player / issuing a new command) be the explicit way to exit follow mode.

[t1garbiznisbrate-ship-it]

Fix for Follow Mode Camera Cancellation (Time-Based Debounce & Pre-Lerp Reference Check)

FollowBreakDistanceSq = 400f ($20^2\text{ m}$) was far too large to trip during ordinary camera panning. Furthermore, the cancellation check needs to evaluate the pivot displacement at the start of the frame against the previous frame's follow-commanded position before applying the next lerp.

This fix:

1. Lowers the movement threshold: Uses FollowBreakDistanceSq = 0.01f (~10 cm).
2. Uses a 50 ms time-based debounce: Replaces frame-count checks with now - _followBreakStartMs >= FollowBreakDebounceMs, ensuring consistent break latency across varying framerates (30 to 144+ FPS).
3. Restructures reference-point ordering: Evaluates manual displacement against _followPivot before computing the lerp, then updates _followPivot to the newly commanded pivot for the subsequent frame.
4. Lifecycle resets: Resets debounce state on camera jumps, target timeout, null controller, or StopFollowing().

(Note: Add private const float FollowBreakDistanceSq = 0.01f;, private const long FollowBreakDebounceMs = 50;, and private long _followBreakStartMs = -1; alongside _followPivotValid at the top of the class).

One caveat remains: the camera-jump branch doesn't return, so it can continue into follow processing during the same frame. That's not necessarily wrong, and I wouldn't change it unless the intended TakeCameraJump() semantics require the jump to stand alone.

Suggested change

	if (_followPivotValid &&
	math.distancesq((float3)controller.pivot, _followPivot) > FollowBreakDistanceSq)
	private void ApplyCameraIntent(MultiplayerService service, CameraController controller, long now)
	{
	if (controller == null)
	{
	_followPivotValid = false;
	_followBreakStartMs = -1;
	return;
	}
	float3 jump;
	if (service.TakeCameraJump(out jump))
	{
	controller.pivot = jump;
	_followPivot = jump;
	_followPivotValid = true;
	_followBreakStartMs = -1;
	}
	int followId = service.FollowPlayerId;
	if (followId < 0)
	{
	_followPivotValid = false;
	_followBreakStartMs = -1;
	return;
	}
	RemotePlayer target = service.FindRemotePlayer(followId);
	if (target == null || (now - target.LastUpdateMs > FollowStaleTimeoutMs))
	{
	service.StopFollowing();
	_followPivotValid = false;
	_followBreakStartMs = -1;
	return;
	}
	if (_followPivotValid)
	{
	if (math.distancesq((float3)controller.pivot, _followPivot) > FollowBreakDistanceSq)
	{
	if (_followBreakStartMs < 0)
	{
	_followBreakStartMs = now;
	}
	if (now - _followBreakStartMs >= FollowBreakDebounceMs)
	{
	service.StopFollowing();
	service.AppendSystemChat("Stopped following - camera moved.");
	_followPivotValid = false;
	_followBreakStartMs = -1;
	return;
	}
	}
	else
	{
	_followBreakStartMs = -1;
	}
	}
	float3 targetPivot = new float3(target.X, target.Y, target.Z);
	float dt = UnityEngine.Time.deltaTime;
	float t = math.clamp(dt * FollowLerpPerSecond, 0.05f, 0.5f);
	controller.pivot = math.lerp(controller.pivot, targetPivot, t);
	_followPivot = controller.pivot;
	_followPivotValid = true;
	}

[chatgpt-codex-connector]

Clear the remembered ping when a session ends

_hasLastPing is never reset during disconnect, fault, or a subsequent connection, while MultiplayerService survives across sessions. After leaving one city and joining another, /goto ping therefore reports success and moves the camera to coordinates from the previous city instead of saying that no ping exists in the current session; clear this state as part of session teardown or initialization.

Useful? React with 👍 / 👎.

[t1garbiznisbrate-ship-it]

Fix for Session Ping State Persistence

Codex's recommendation is to ensure _hasLastPing is cleared during session lifecycle transitions rather than only lazily when TryGetLastPing() is invoked.

• Required Lifecycle Fix: Clear _hasLastPing = false; in the existing session reset/teardown path (alongside remote player cleanup)!!!!!!!!!!!!!!!!!!
• Defensive Guard: Keep the !GameplaySyncReady check inside TryGetLastPing() as an extra safety net against reading stale coordinates before the world is synchronized.

Suggested change

	private float3 _lastPing;
	private bool _hasLastPing;
	private float3 _lastPing;
	private bool _hasLastPing;
	/// <summary>Where the most recent ping landed, for "/goto ping".</summary>
	public bool TryGetLastPing(out float3 position)
	{
	if (!GameplaySyncReady)
	_hasLastPing = false;
	position = _lastPing;
	return _hasLastPing;
	}

(Note: Add _hasLastPing = false; to the existing session teardown/disconnect handler).