Skip to content

Fix #5 — player_stats buffer truncation + tank/witch FK race on map resolve#6

Open
draxios wants to merge 1 commit into
mainfrom
fix/issue-5-sql-truncation-and-fk-race
Open

Fix #5 — player_stats buffer truncation + tank/witch FK race on map resolve#6
draxios wants to merge 1 commit into
mainfrom
fix/issue-5-sql-truncation-and-fk-race

Conversation

@draxios

@draxios draxios commented May 30, 2026

Copy link
Copy Markdown
Collaborator

Closes #5.

Two production bugs surfaced on the Campaign server from yesterday's diagnose. Both are SourcePawn-side, not schema. Tactical fixes that stop the bleeding now; longer-term refactor (parameterized queries for the wide upserts) is noted but out of scope here.

Bug A — query buffer truncation in Bizzy_Session_Flush

session.sp builds a wide INSERT INTO player_stats … ON DUPLICATE KEY UPDATE … statement (35+ columns each side) into a stack char sql[2048]. After migration 010 grew the schema, the rendered statement passed 2048 bytes once you account for %d substitutions. FormatEx silently truncated, and the truncation point varied with the actual integer-digit widths — which is exactly the pattern in the log:

Unknown column 'damage_to_ta'  ←  damage_to_tank cut after 12 chars
Unknown column 'damage_to_tan' ←  same column cut after 13 chars
Unknown column 'VALUE'         ←  VALUES keyword cut after 5 chars
Unknown column 'VALU'          ←  VALUES keyword cut after 4 chars
Unknown column 'V'             ←  VALUES keyword cut after 1 char
syntax error … near ''         ←  cut even further into NULL

The whole txn.AddQuery(sql) for player_stats was being rejected by MySQL, dropping the entire transaction — meaning the session's accumulated counters never landed. Players were losing real stats on every map flush.

Fix: bump sql[2048]sql[4096] in the one offending function. Comment explains the bug, links #5, and notes that the right long-term answer is parameterized queries so we don't string-build a 3KB statement on the stack at all.

Bug B — tank_records.fk_tr_map FK violation

g_CurrentMapId is the resolved maps.id for the active map. It's populated through a two-step async chain kicked off by Bizzy_OnMapStart:

OnMapStart(SM hook)
  → GetCurrentMap(g_CurrentMap)
  → Bizzy_OnMapStart()
    → Bizzy_Identity_ResolveMap()
      → INSERT IGNORE INTO maps (code, game_id, first_seen) VALUES (…)
        → callback: SELECT id FROM maps WHERE code=… AND game_id=…
          → callback: g_CurrentMapId = rs.FetchInt(0)

That works the first time OnMapStart fires. But it misses two real cases:

  1. Plugin hot-reload mid-mapOnMapStart already fired before the reload; on the fresh plugin instance it won't fire again until the next map. g_CurrentMap is empty and g_CurrentMapId is 0.
  2. DB connects after OnMapStart — server boot races. OnMapStart fired with g_DB == null, so ResolveMap no-op'd. By the time the DB connects, we've missed our cue.

Both states leave g_CurrentMapId = 0. The next tank or witch spawn inserts map_id=0, which doesn't exist in maps, and the FK constraint fk_tr_map rejects the row.

Fix: three coordinated changes:

File Change
identity.sp OnServerLookup After g_ServerId is set, call Bizzy_Identity_ResolveMap(). Handles the DB-late case and the hot-reload case in one place — by the time DB-ready fires, we know we have a stable connection and we trigger the resolve ourselves rather than waiting for the next OnMapStart.
identity.sp Bizzy_Identity_ResolveMap Refresh g_CurrentMap from the engine via GetCurrentMap() at the top of the function. Doesn't matter who called us or whether OnMapStart populated the global yet — we read fresh each time. Idempotent, cheap.
tank_witch.sp tank-spawn + witch-spawn Add g_CurrentMapId == 0 to the early-return guards. Defensive — if the above two fixes ever miss an edge case, the failure mode becomes "lose one boss record" instead of "FK error in errors_*.log".

What's NOT in this PR

  • Parameterized queries for the wide upserts. Right answer long-term but bigger refactor (Statement objects, parameter binding, possible Transaction rework). Noted in the buffer-bump comment as the follow-up.
  • A general truncation-detection helper in database.sp. Considered, decided not worth the scope here — bumping the single offender's buffer is sufficient.
  • Anything in web/ or schema/. Schema is fine; the FK constraint is doing its job (catching the race).

Test plan

  • Code review the SourcePawn changes.
  • Build locally or in CI: release.yml should compile bizzymod_stats.smx cleanly with no new warnings.
  • (Optional) On a test server, hot-reload the plugin mid-map and spawn a tank — confirm no FK error in errors_*.log and confirm the tank row exists in tank_records.
  • Merge.
  • Tag v0.6.1 on main. Rename the ## [0.6.1] — UNRELEASED heading in CHANGELOG to today's date when tagging (or do it as part of the tag prep commit — either flow works).
  • Downstream: bump BIZZYMOD_STATS_TAG in bogware/bizzymod-campaign/.github/workflows/deploy.yml from v0.6.0v0.6.1. (Tracked in bogware/bizzymod-campaign#4 wire-up.)

🤖 Generated with Claude Code

Bug A — query buffer truncation in player_stats upsert
-------------------------------------------------------
Bizzy_Session_Flush builds a wide INSERT INTO player_stats … ON DUPLICATE
KEY UPDATE statement into a stack `char sql[2048]`. After the schema
grew through migration 010 the rendered statement exceeded that buffer
and FormatEx silently truncated, depending on the per-int substitution
widths. MySQL then rejected the malformed statement, surfacing as:

  Unknown column 'damage_to_ta' / 'damage_to_tan'
  Unknown column 'VALUE' / 'VALU' / 'V'
  You have an error in your SQL syntax … near '' at line 1

…and the entire flush transaction was dropped, losing the session's
accumulated counters.

Bump the buffer to 4096 with a clear comment + #5 reference. Long-term
fix (parameterized queries — no string-build) is noted there as the
follow-up; this is the safe tactical fix to stop bleeding data.

Bug B — tank_records.fk_tr_map FK violation on boss spawn
---------------------------------------------------------
`g_CurrentMapId` is the resolved `maps.id` for the active map and is
populated through a two-step async chain (INSERT IGNORE INTO maps →
SELECT id → set g_CurrentMapId) kicked off from `OnMapStart`. On a
plugin hot-reload mid-map or a DB connection that completes after
`OnMapStart` has already fired, `g_CurrentMapId` stays at 0 and any
boss insert (tank_records / witch_records) FK-violates against maps.id.

Three-part fix:
  - `OnServerLookup` (the DB-ready callback) now calls
    `Bizzy_Identity_ResolveMap()` after setting `g_ServerId`. Covers
    the late-DB-connect / hot-reload-mid-map case.
  - `Bizzy_Identity_ResolveMap` now refreshes `g_CurrentMap` from the
    engine via `GetCurrentMap()` on every call, so it doesn't no-op when
    invoked from a code path other than `OnMapStart`.
  - Tank- and witch-spawn inserts in tank_witch.sp now bail early when
    `g_CurrentMapId == 0`. Defensive — converts any remaining edge case
    from a noisy FK error to a silently dropped boss record.

Files touched
-------------
  plugin/scripting/bizzymod_stats/session.sp     - sql buffer 2048→4096
  plugin/scripting/bizzymod_stats/identity.sp    - resolve on DB-ready;
                                                   refresh on every call
  plugin/scripting/bizzymod_stats/tank_witch.sp  - g_CurrentMapId guards
  CHANGELOG.md                                   - [0.6.1] entry

Refs #5
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.

Two plugin-side bugs in production: query buffer truncation + tank_records FK race

1 participant