Skip to content

perf(serverwater): O(actors*global_waters) → O(actors*per_area_waters) in per-tick underwater check#270

Merged
CoreyRDean merged 2 commits into
developfrom
refactor/serverwater-per-area-chain
May 26, 2026
Merged

perf(serverwater): O(actors*global_waters) → O(actors*per_area_waters) in per-tick underwater check#270
CoreyRDean merged 2 commits into
developfrom
refactor/serverwater-per-area-chain

Conversation

@CoreyRDean
Copy link
Copy Markdown
Collaborator

Summary (non-technical)

The server checks every actor against every water region in every loaded zone, every server tick — even when only one zone's water can possibly intersect that actor. This PR indexes water regions by their owning zone and walks only the relevant subset. On a 5-zone server with 10 water regions each and 300 actors, that's ~3,000 inner-loop iterations per tick instead of ~15,000. Behaviour is byte-identical; only the iteration is faster.

Technical summary

UpdateActorInstances() (GameServer.bb:665) is the per-tick actor update loop. Inside its body, the underwater-damage check at line 737 previously walked the global For SW.ServerWater = Each ServerWater collection and filtered each candidate via If SW\Area = AInstance\Area. Cost: O(actors × total_water_in_all_loaded_areas) per tick.

Indexing change:

Type Field added
Area Field FirstWater.ServerWater — head of the per-Area chain
ServerWater Field NextWater.ServerWater — chain link

Populated at ServerLoadArea time via head-insert into A\FirstWater (chain order is irrelevant — the hit-test Exits on the first match). The global For Each ServerWater collection still owns every record (allocation / Delete go through Each); the new chain is an O(1)-lookup index into the subset belonging to each Area.

The per-tick hot path in GameServer.bb now reads:

Local SW.ServerWater = AInstance\Area\FirstWater
While SW <> Null
    ; ... hit test (byte-identical to old body) ...
    SW = SW\NextWater
Wend

New cost: O(actors × waters_in_this_actor's_area) per tick.

Chain-lifecycle invariants (documented inline)

  • Head-insert at allocation (ServerLoadArea line 247-248): order doesn't matter; the underwater check Exits on the first match.
  • Dangling on area unload: ServerUnloadArea Deletes both A (which kills the chain head) and every ServerWater (via the existing after-cursor walk that filters by W\Area = A) in a single call. No caller can observe a stale NextWater pointer.
  • Save path unaffected: SaveArea still walks the global For Each ServerWater with the existing If W\Area = A filter. Not a perf-critical path; left alone.

Acceptance criteria

  • Area type gains FirstWater.ServerWater field with inline documentation
  • ServerWater type gains NextWater.ServerWater field with inline documentation
  • ServerLoadArea head-inserts each new water into A\FirstWater
  • GameServer.bb underwater check walks the per-Area chain instead of For Each ServerWater
  • Hit-test body byte-identical to the original (same conditions, same Underwater = Handle(SW) recording, same early Exit)
  • Full compile.bat clean across Server + Client + GUE + Project Manager + 7 Tools (exit 0, unfiltered)
  • test.bat 19/19 green
  • No behaviour change for any actor's water-damage outcome

Trade-offs / deferred

  • SaveArea not converted to the per-Area chain (still walks For Each + filter). Not a per-tick hot path; the conversion would add code without measurable benefit and risks breaking the save-format-stable ordering. Acknowledged in the inline comment.
  • ServerUnloadArea not converted to walk via the per-Area chain (still uses the after-cursor walk). Same reasoning — runs once per area unload, not per tick. The dangling-NextWater-on-Area-Delete invariant is documented inline so a future contributor doesn't try to "fix" it.
  • No benchmark added. Microbenchmarking the per-tick savings would require a packet-handler test harness that doesn't exist (see iteration 2.0.0a1 #9's deferred follow-ups). The structural argument (Each ServerWater + filter → per-Area chain walk) is sound by inspection; the per-iteration body is byte-identical so any difference is purely loop count.

Risk

  • Low. Behaviour-observable change is zero (verified: hit-test conditions, Underwater recording, DistUnder# math, early Exit semantics, the surrounding Underwater = 0 reset path — all preserved).
  • The new chain pointer (W\NextWater) is only set in one place (head-insert at allocation) and read in one place (the GameServer.bb walk). No mutation path; no risk of corruption.
  • Exit inside While...Wend works the same way in Blitz3D as inside For...Next (breaks the innermost loop) — verified by successful compile + test.

🤖 Generated with Claude Code

CoreyRDean and others added 2 commits May 26, 2026 00:09
Adds `Field FirstWater.ServerWater` on Area and `Field NextWater.ServerWater`
on ServerWater, populated at ServerLoadArea time via head-insert into
A\FirstWater. The global `For Each ServerWater` collection still owns
every record (allocation / Delete go through Each); the new chain is an
O(1)-lookup index into the subset belonging to each Area.

Both the load-time insert and the dangling-on-area-unload semantics are
documented inline so a future reader doesn't second-guess the chain
maintenance:

- ServerLoadArea links each newly-allocated water into A\FirstWater
  (head-insert; chain order doesn't affect semantics).
- ServerUnloadArea Deletes both the chain heads (via Delete A) and
  every linked ServerWater (via the existing after-cursor walk at
  ServerUnloadArea) in a single call, so no caller can observe a stale
  NextWater pointer.

No behavioural change. The companion GameServer.bb hot-path swap
(per-tick underwater check) is in the next commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
UpdateActorInstances runs once per tick per ActorInstance. The
underwater-damage check at this site previously walked the global
`For SW.ServerWater = Each ServerWater` collection and filtered by
`If SW\Area = AInstance\Area`. The cost was O(actors * total_water_in_
all_loaded_areas) per tick. On a server running 5 zones with 10 water
regions each (50 total) and 300 actors (mix of players + NPCs), that
was 15,000 inner-loop iterations per tick just for the water check.

Walk AInstance\Area\FirstWater instead (built in the companion
ServerAreas.bb commit). New cost: O(actors * waters_in_this_actor's_
area), typically 5-10 per area, so ~3,000 iterations in the example
above. The per-iteration body is byte-identical to the original;
behaviour is unchanged (same hit-test, same `Underwater = Handle(SW)`
recording, same early Exit on first water-region match).

The `Exit` keyword still works the same inside `While...Wend` as it
did inside `For...Next` -- breaks out of the innermost loop.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@CoreyRDean CoreyRDean requested a review from a team as a code owner May 26, 2026 05:10
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 93850c089a

ℹ️ 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".

Comment on lines +271 to +272
W\NextWater = A\FirstWater
A\FirstWater = W
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve water iteration order within each area

This head-insert reverses each area's water order relative to load/file order, but UpdateActorInstances stops on the first matching water (Exit in the hit-test loop), so overlapping water volumes can now select a different ServerWater than before. In maps where overlaps have different Damage, DamageType, or surface Y, actors will take different damage/breath behavior after this change even though the old global scan was deterministic by creation order.

Useful? React with 👍 / 👎.

@CoreyRDean CoreyRDean merged commit 73ae8a5 into develop May 26, 2026
1 check passed
@CoreyRDean CoreyRDean deleted the refactor/serverwater-per-area-chain branch May 26, 2026 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant