Fix: Bot stack overflow after teleporting to an unwalkable map

[therpr]

Problem

A bot can crash the whole game server with a stack overflow when it warps to a map whose spawn gate places it on a
blocked (non-walkable) tile.

Root cause

Bots are connection-less (BotPlayer : OfflinePlayer). When a warp lands a player on a blocked tile,
ClientReadyAfterMapChangeAsync recovers by calling WarpToSafezoneAsync:

ClientReadyAfterMapChangeAsync
→ WarpToSafezoneAsync
→ WarpToAsync
→ IMapChangePlugIn.MapChangeAsync
→ (offline bot) OfflineMapChangePlugIn.MapChangeAsync
→ ClientReadyAfterMapChangeAsync // synchronous, inline — no network round-trip

For a real player the map-change plugin sends a packet and returns; the next ClientReadyAfterMapChangeAsync arrives
later on a fresh stack from the client's F3 12 ack — no recursion.

For a bot, OfflineMapChangePlugIn.MapChangeAsync calls ClientReadyAfterMapChangeAsync inline. If the destination's
safezone spawn gate is itself blocked, the recovery re-enters on the same growing stack and recurses until it
overflows — crashing the process.

A reactive "remember broken maps, skip them next tick" fix cannot work here: the overflow kills the process on the
first bad warp, before the bot's next navigator tick ever runs.

Fix (proactive, bot-side only)

A bot is never offered a map it cannot stand in, so the recovery recursion can never start.

BotNavigator.HasWalkableSpawnGate(map) parses the destination map's terrain and checks that its safezone spawn gate
(the gate WarpToSafezoneAsync recovers to) contains at least one walkable tile. The two candidate pickers —
TryPickEasierMap and TryPickBetterMapCore — now drop any map that fails this check, alongside the existing legal-warp
and affordability filters:

if (!candidate.TryGetRequirementError(this._player, out _)
&& this.TryGetLegalWarp(candidate, out var candidateWarp)
&& this.CanAffordWarp(candidateWarp)
&& this.HasWalkableSpawnGate(candidate)) // new

Terrain is static configuration, so the verdict is parsed once per map and cached in a static
ConcurrentDictionary<int, bool> shared across all bots — no per-tick terrain parsing, no per-bot duplication.

[sven-n]

Review

What it does: Adds BotNavigator.HasWalkableSpawnGate(map), a statically-cached terrain check, and uses it as an extra filter in TryPickEasierMap and TryPickBetterMapCore so a bot never selects a map whose safezone spawn gate has no walkable tile — avoiding the ClientReadyAfterMapChangeAsync → WarpToSafezoneAsync → OfflineMapChangePlugIn.MapChangeAsync → ClientReadyAfterMapChangeAsync recursion.

The root-cause analysis in the description is accurate — the cycle is verifiable in Player.cs:1185-1187, Player.cs:1106 and Offline/OfflineMapChangePlugIn.cs:29.

Correctness

• The check inspects the wrong map (main issue). The recovery path is WarpToSafezoneAsync → GetSpawnGateOfCurrentMapAsync, which resolves CurrentMap.Definition.SafezoneMap ?? CurrentMap.Definition (Player.cs:2309). Many maps (dungeons, event maps) have a different SafezoneMap. The PR checks map.GetSafezoneGate(...) on the destination map itself, so it can both reject a perfectly safe map and — more importantly — fail to reject the actually-crashing case where the destination's SafezoneMap target is the broken one. It should mirror the runtime resolution:

  var safezoneMap = map.SafezoneMap ?? map;
  var terrain = new GameMapTerrain(safezoneMap);
  var spawnGate = safezoneMap.GetSafezoneGate(terrain);

  and the cache key must then be that resolved map, not map.Number.

• The crash is still reachable through unfiltered paths. Candidate selection is only one way a bot changes maps. The escape/home-town gate path in this same file (BotNavigator.cs:1492, GetSafezoneGate()), death respawn (Player.cs:2064), walking into a portal/exit gate, mini-games and GM/console warps all bypass this filter and still recurse to overflow — a server-killing crash, not a bot glitch. I'd recommend also adding the cheap defensive guard at the recursion site, e.g. in ClientReadyAfterMapChangeAsync skip the WarpToSafezoneAsync recovery when we are already at the resolved safezone gate (or gate it behind a re-entrancy/depth flag on the player). That turns a process crash into a stuck bot on every path, and the navigator filter then remains a nice quality improvement on top.

• Cache keying / lifetime. ConcurrentDictionary<int, bool> keyed on map.Number is static for the process lifetime. OpenMU can host several game servers and the admin panel can reload/edit configuration; two GameConfigurations with the same map Number but different TerrainData will collide, and edited terrain is never re-evaluated. Keying on map.GetId() fixes collisions; a note in the remarks that config edits need a restart would cover staleness.

Quality / style

• GameMapTerrain allocates two bool[256,256], a byte[256,256] and a spawn-point array per construction. Cached, so amortized fine — and when the map is already loaded, GameContext.GetMapAsync(...) exposes Terrain and SafeZoneSpawnGate directly (GameMap.cs:52). Not worth forcing a load for an unloaded map, so the current approach is defensible; worth a comment.
• for (var x = (int)spawnGate.X1; ...) is safe — gate coordinates are bytes, so WalkMap[256, …] can't be hit.
• The ponytail: prefix in the <remarks> block appears nowhere else in the codebase — looks like a stray internal marker. Please drop it.
• Doc comments are otherwise clear and explain the why well.

Tests

No tests added. BotNavigator is internal but reachable from MUnique.OpenMU.Tests; a focused test that builds a GameMapDefinition with a spawn gate over blocked terrain and asserts the map is not selected would be valuable, especially for the resolved-safezone-map logic above.

Security / performance

• No security surface. No per-tick terrain parsing; the filter is O(gate area) once per map.
• GetOrAdd's factory may run concurrently for the same key — pure and idempotent here, so only a redundant parse.

Summary

Sound diagnosis and a cheap, well-documented mitigation, but two items should be addressed before merge: (1) resolve SafezoneMap before checking the gate — as written the check can miss the exact crash it targets; (2) add a guard at the recursion site so non-navigator warp paths can't still take down the server. Cache key and the ponytail: comment are minor cleanups.

---

Generated by Claude Code