Commit graph

2 commits

Author SHA1 Message Date
Kamron Batman
547c2ea0fa
fix: Fixes tick count wrap-around in movement throttle, and eliminates more allocations in NetState (#2603)
## Summary

Removes the per-tick allocation in the movement throttle, fixes tick-count wrap-around bugs in the throttle and RTT probe state, and trims per-connection allocations and dead fields in `NetState`.

## Movement throttle

- **No more per-tick `List<NetState>` snapshot.** `ProcessAllQueues()` iterates the `HashSet` directly and removes drained or disconnected states in place. `HashSet<T>.Remove` does not invalidate enumerators on .NET Core 3.0+ (verified on 10.0.11); only inserting a *new* member does, and the only `Add` is in the packet handler, which never nests with `Slice()`. The eager `Remove` calls in `RejectAndReset`, `ClearQueue`, and `ProcessMovementQueue` are gone; membership is reconciled once per tick from `_hasQueuedMovements`.
- **Debug logging** is now gated solely by the per-connection `NetState.MovementLogging` flag. The global `movementThrottle.debugLogging` setting is removed.
- **New settings**: `movementThrottle.maxRttBonus`, `movementThrottle.maxChainGap`, and `movementThrottle.speedHackNotificationCooldown` were fields with no config binding.

## Tick-count wrap-around

All comparisons are now in subtraction form and no tick field uses zero as a sentinel:

- `now < _nextMovementTime` in the queue drain loop → `now - _nextMovementTime < 0`.
- `_lastMovementRecordTime > 0`, `_lastSpeedHackNotification`, `_rttProbeTime > 0`, and `_nextRttProbe == 0` sentinels replaced with `_hasMovementRecord`, `_speedHackNotified`, `_rttProbePending`, and a seeded `_nextRttProbe`.
- `_lastQueueDepthCheck` and `_movementWindowStart` are seeded from `Core.TickCount` at construction and on reset instead of zero.

User-visible effects of the old code: on hosts with pass-through counters (GCP) movement history never recorded and speed hack detection was silently off; on every host, staff speed hack notifications were suppressed until `Core.TickCount` exceeded the five-minute cooldown.

## NetState

- `Instances` returns `HashSet<NetState>` again so engine-internal `foreach` uses the struct enumerator instead of boxing through `IReadOnlySet<T>`.
- Removed `_sustainedQueueDepth` (declared and zeroed since #2266, never read), `_lastRtt` (now derived as `LastRtt` from the newest history slot), and `_rttProbeTimestampHiRes` (only fed one debug log line). 20 bytes per connection.
- `HuePickers`, `Menus`, and `Trades` are lazily created instead of allocating three lists per connection, including every login-server connection that dies on shard select. `Trades` is released when it empties. All helpers and the `HuePickerResponse` / `MenuResponse` handlers are null-tolerant; the trade cancel loops keep their `i < Count` guards because `SecureTrade.Cancel()` runs virtual item hooks that can re-enter the same list.

## Testing

- `dotnet build -c Release` clean.
- All MovementThrottle tests pass (27), plus the Trade / Menu / HuePicker / NetState tests (32).
2026-09-01 20:25:20 -07:00
Kamron Batman
04d438239d
feat: Adds robust speed hack detection and movement throttling (#2266)
## Summary

   Server-side movement throttle that prevents speed hacking while accurately identifying cheaters with detection of lagging connections.

   **Key features:**
   - Credit buffer (200ms) absorbs timing jitter from legitimate players
   - Movement queue handles larger bursts, draining at proper game-tick intervals
   - RTT measurement distinguishes network lag from speed hacks
   - Queue depth detection catches ACK-throttled speed hacks (going straight)

   ## How It Works

   **Throttle** (prevention): Movements arriving too early either consume credit or get queued. The queue drains at
   correct intervals, so speed hackers can't move faster regardless of what they send.

   **Detection** (identification): Combines multiple signals to identify cheaters:
   | Signal | What it catches |
   |--------|-----------------|
   | Queue depth ≥4 sustained | ACK-throttled speed hacks (client limits unacked moves to 5) |
   | Movement rate >1.05x | Direction-change speed hacks where timing is visible |
   | Stable RTT + high queue | Eliminates false positives from laggy players |

   **RTT-Aware Logic:**
   - Probes only sent to players actively moving (event-driven, not global loop)
   - Stable low-latency + problems = suspicious
   - Unstable/high-latency + problems = probably just lag, throttle handles it

   ## Configuration

   ```json
   {
     "movementThrottle.maxCredit": 200,
     "movementThrottle.softQueueLimit": 6,
     "movementThrottle.hardQueueLimit": 10,
     "movementThrottle.debugLogging": false
   }
   ```
2026-03-07 11:44:37 -08:00