[GameServer][Bug] Projectile hit checks ignore the previous position (tunneling) and spam Warning-level logs per player per tick — CampZone + DungeonZone #645

New Issue

2026-07-17T07:09:34Z

spikerj commented

2026-07-17 07:09:34 +00:00

Found via SonarQube S1172 triage (2026-07-17) — CheckProjectileHitPlayers(projectile, oldX, oldY, oldZ, ...) in BOTH CampZone (:459) and DungeonZone (:1270) receives the projectile's previous position and never uses it: the hit test is a point-vs-radius check on the CURRENT position only. A projectile moving more than ~3 units per tick (the threshold is 9 dist²) can pass straight through a player between ticks — classic tunneling. The unused parameters are the fossil of the intended swept check.

Fix shape: segment-sphere intersection from (oldX,oldY,oldZ) to the current position against each candidate's hit radius (EntityCollisionSystem already has SIMD radius machinery worth reusing). Needs gameplay testing — changing hit detection changes game feel, so not a drive-by fix.

Second issue, same methods: every check logs multiple LogWarning lines PER PLAYER PER TICK ("[CAMP] CheckProjectileHitPlayers: Checking player...", distance prints, etc.) — leftover debug spam at Warning level on the hottest path in the zone, flooding Seq whenever combat happens. These should drop to LogTrace or be removed outright; that part IS safe to do immediately and can ride any GameServer PR.

Both zones share the copy-pasted implementation — consider extracting one swept-hit helper while fixing.

**Found via SonarQube S1172 triage (2026-07-17)** — `CheckProjectileHitPlayers(projectile, oldX, oldY, oldZ, ...)` in BOTH CampZone (:459) and DungeonZone (:1270) receives the projectile's previous position and never uses it: the hit test is a point-vs-radius check on the CURRENT position only. A projectile moving more than ~3 units per tick (the threshold is 9 dist²) can pass straight through a player between ticks — classic tunneling. The unused parameters are the fossil of the intended swept check. **Fix shape:** segment-sphere intersection from (oldX,oldY,oldZ) to the current position against each candidate's hit radius (EntityCollisionSystem already has SIMD radius machinery worth reusing). Needs gameplay testing — changing hit detection changes game feel, so not a drive-by fix. **Second issue, same methods:** every check logs multiple `LogWarning` lines PER PLAYER PER TICK ("[CAMP] CheckProjectileHitPlayers: Checking player...", distance prints, etc.) — leftover debug spam at Warning level on the hottest path in the zone, flooding Seq whenever combat happens. These should drop to LogTrace or be removed outright; that part IS safe to do immediately and can ride any GameServer PR. Both zones share the copy-pasted implementation — consider extracting one swept-hit helper while fixing.

spikerj commented

2026-07-17 07:13:47 +00:00

Related combat-system finding from the same S1172 pass: ApplyDamageToShip (:1200) and ApplyDamageToSpacecraft (:1292) receive 'sourceEntityId' (xml-doc: 'Entity that caused the damage') and never use it — while kill attribution IS a live mechanic (HandlePlayerDeath(player, killerId) → death event carries KillerId at :3121). Ship/spacecraft destruction therefore cannot credit the killer through this path. Fold into the combat-system fix alongside the tunneling work; the sonar findings for these params stay open as the tracker.

spikerj referenced this issue

2026-07-17 07:25:53 +00:00

[Backend] Remove 12 genuinely dead private parameters (SonarQube S1172) — the residue after the family's triage #647

spikerj referenced this issue

2026-07-17 07:38:19 +00:00

[Backend] S1172 unused-parameter family (74, incl. 28 unused CancellationTokens): rubric batch after the #632 baseline lands #641

spikerj commented

2026-07-18 18:17:20 +00:00

Log half fixed — backend PR #416 (mergeable): the 10-per-player-per-tick LogWarning chatter in both zones' CheckProjectileHitPlayers dropped to Trace, and the two LogError-on-normal-hit sites dropped to Debug. Message templates unchanged, so hit-detection diagnostics remain available by raising the level.

Still open — the tunneling half: the hit test still ignores oldX/oldY/oldZ (swept segment-sphere check needed). Leaving this ticket open for that, per its own gameplay-testing gate; the unused parameters remain in place as the seam for the swept implementation.

**Log half fixed** — backend PR [#416](https://git.spikersoft.com/spikerj/spikersoft-backend/pulls/416) (mergeable): the 10-per-player-per-tick `LogWarning` chatter in both zones' `CheckProjectileHitPlayers` dropped to `Trace`, and the two `LogError`-on-normal-hit sites dropped to `Debug`. Message templates unchanged, so hit-detection diagnostics remain available by raising the level. **Still open — the tunneling half:** the hit test still ignores `oldX/oldY/oldZ` (swept segment-sphere check needed). Leaving this ticket open for that, per its own gameplay-testing gate; the unused parameters remain in place as the seam for the swept implementation.

spikerj commented

2026-07-22 02:43:38 +00:00

Ticket-hygiene sweep note: PR #416 merged only the LOG half (hit-check spam Warning/Error → Trace/Debug, per its own title). The actual swept-collision fix (checking against the previous position) has no merged evidence — leaving open.

spikerj commented

2026-07-29 15:35:03 +00:00

Audited against origin/master — exactly one of the two halves shipped. Staying open for the tunneling half. Notes updated with precise evidence.

Log half: DONE. spikersoft-backend PR #416 (acf4a994, merge 0eb35aee) — the commit subject itself scopes it: "projectile hit-check log spam Warning/Error → Trace/Debug (#645, log half)". Confirmed: grep -c "LogWarning\|LogError" across CampZone.CheckProjectileHitPlayers (CampZone.cs:459-600) now returns 0; every diagnostic in both zones is LogTrace. No more per-player-per-tick Warning flood.

Tunneling half: NOT DONE, and it currently looks done, which is the dangerous part.

Both zones already take the previous position as parameters —

SpikerSoft.GameServer/Zones/DungeonZone.cs:1267 — CheckProjectileHitPlayers(ProjectileEntity projectile, float oldX, float oldY, float oldZ, bool skipOwner = false)
SpikerSoft.GameServer/Zones/CampZone.cs:459 — identical signature, called with real values from :260

— but oldX/oldY/oldZ are never read in either body. Grepping each method's full span returns exactly one hit apiece: the signature line itself. The hit test still uses only projectile.X/Y/Z (the post-move point), so a fast projectile still steps clean over a player between frames.

Worse, DungeonZone.cs:1257-1259 carries the doc comment:

/// Uses swept collision from old position to new position

That comment is false today. Anyone auditing this file — or grepping for "swept" as I did — will conclude the fix is in. Whoever picks this up should treat the parameters and the comment as a scaffold that was never filled in, not as an existing implementation to tweak.

Remaining work: implement the actual segment-vs-body-part test (old→new) in both DungeonZone and CampZone, and either make the DungeonZone doc comment true or delete it until it is.

Audited against `origin/master` — **exactly one of the two halves shipped.** Staying open for the tunneling half. Notes updated with precise evidence. **Log half: DONE.** spikersoft-backend PR #416 (`acf4a994`, merge `0eb35aee`) — the commit subject itself scopes it: *"projectile hit-check log spam Warning/Error → Trace/Debug (#645, **log half**)"*. Confirmed: `grep -c "LogWarning\|LogError"` across `CampZone.CheckProjectileHitPlayers` (`CampZone.cs:459-600`) now returns **0**; every diagnostic in both zones is `LogTrace`. No more per-player-per-tick Warning flood. **Tunneling half: NOT DONE, and it currently *looks* done, which is the dangerous part.** Both zones already take the previous position as parameters — - `SpikerSoft.GameServer/Zones/DungeonZone.cs:1267` — `CheckProjectileHitPlayers(ProjectileEntity projectile, float oldX, float oldY, float oldZ, bool skipOwner = false)` - `SpikerSoft.GameServer/Zones/CampZone.cs:459` — identical signature, called with real values from `:260` — but **`oldX`/`oldY`/`oldZ` are never read in either body.** Grepping each method's full span returns exactly one hit apiece: the signature line itself. The hit test still uses only `projectile.X/Y/Z` (the post-move point), so a fast projectile still steps clean over a player between frames. Worse, `DungeonZone.cs:1257-1259` carries the doc comment: ``` /// Uses swept collision from old position to new position ``` That comment is false today. Anyone auditing this file — or grepping for "swept" as I did — will conclude the fix is in. Whoever picks this up should treat the parameters and the comment as a scaffold that was never filled in, not as an existing implementation to tweak. **Remaining work:** implement the actual segment-vs-body-part test (old→new) in both `DungeonZone` and `CampZone`, and either make the `DungeonZone` doc comment true or delete it until it is.

spikerj referenced this issue

2026-07-29 16:56:19 +00:00

[Feature][Calendar] Create-for-child from the parent calendar — owner selector in the event modal (follow-up to #788) #790

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#645