minecraft/brass-and-sigil
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

pack: v0.36.0 — bnstoolkit 0.4.1 (review fixes + hub UI + NPC integration) #74

Merged
Matt merged 1 commits from feature/v0.36.0-bnstoolkit-0.4.1 into main 2026-06-07 09:28:21 +00:00
Conversation 0 Commits 1 Files Changed 3
+52 -2
Matt commented 2026-06-07 09:28:20 +00:00
Owner
Copy Link
Copy Source

Summary

Closing out all 4 review findings in one PR.

Critical fix

Bounty board + plot office were reading NBT keys that don't exist. They showed empty state regardless of reality. Now read the actual KubeJS keys (bnsBountyPool, bnsBountyActive, bnsBountyCompleted per-player; server-scope bnsPlots via KubeJS MinecraftServer mixin reflectively).

Hub UI + key rebind

  • B opens a new hub screen with buttons routing to each sub-screen
  • K/J/N/H/M direct shortcuts default to UNBOUND (rebindable for power users in vanilla controls menu)
  • M used to conflict with vanilla Social Interactions + FTB Chunks fullscreen map (hard conflict) — fully resolved
  • Audit verified B is free across vanilla 1.21.1 + all 97 pack mods

NPC dialogue + scripting integration

  • New /bns open <screen> command with tab-complete (hub|tier|bounty|plot|shop|questlog)
  • Easy NPC dialogue COMMAND actions can now open any bnstoolkit screen — verified executable path via Easy NPC's CommandExecutor.executePlayerCommand
  • FTB Quests reward commands, command blocks, and /trigger objectives all route through the same verb
  • config/easy_npc/security.cfg ships with executeAsUserCommandAllowList.ALL=bns so dialogue-triggered /bns actually runs

Network hardening

  • Per-player token-bucket rate limit (10/s sustain, 20 burst)
  • All STRING_UTF8 codec fields capped (kind=8, args=80, which=16)
  • Verb whitelist replaced with structured parse → regex-validate per verb → rebuild command (no prefix-startsWith surprises)
  • Tier-upgrade per-player in-flight guard (500ms) closes the concurrent-packet double-spend race
  • Bridge log lines sanitised so a hostile client can't inject control chars into server logs

Review-driven nits

  • Per-player tick counter for HUD push (was a shared static — N-player skew)
  • VillagerTradeRebalance: try/catch around inner; null cost-A passthrough; round-to-nearest denom split (was floor → ~10% silent price haircut); silenced spammy null-entity pre-fetch warnings from recipe-viewer mods
  • SpurHud Component cache (was allocating 2 per frame)
  • ClientBnsState.completedIds → HashSet (O(1) per-frame lookups)
  • BountyBoardScreen dead branch removed
  • Wire-format version bumped 0.2 → 0.4 (added OpenScreenPacket)

Verified live

Server restart at 09:26 UTC with bnstoolkit-0.4.1.jar:

  • Done (3.180s) on port 25565
  • [bnstoolkit] registered /bns open <screen> command logged at boot
  • bnstoolkit/net] registered payloads v0.4 logged at boot
  • Zero [bns/trades] inner ItemListing ... threw warnings (down from 100+ in 0.4.0 — the null-entity pre-fetch noise is now silenced)
  • No ModLoadingException

Test plan tomorrow

  • Press B in game — HubScreen opens with tier+balance header, 5 buttons
  • Each button opens its sub-screen with real data populated
  • Accept a bounty via screen, kill the mob — progress reflects in BountyBoardScreen + QuestLog
  • Buy a plot — PlotPurchaseScreen shows you as owner; another player sees "taken by [you]"
  • Spawn an Easy NPC, configure dialog with COMMAND action /bns open shop, click — bazaar opens
  • Try /bns open hub from console — should fail with "must be run as a player"
  • Press the now-unbound K (default) — nothing happens (correct)
  • Rebind K to OPEN_TIER in vanilla controls menu — direct shortcut works

🤖 Generated with Claude Code

## Summary Closing out all 4 review findings in one PR. ### Critical fix Bounty board + plot office were reading NBT keys that don't exist. They showed empty state regardless of reality. Now read the actual KubeJS keys (`bnsBountyPool`, `bnsBountyActive`, `bnsBountyCompleted` per-player; server-scope `bnsPlots` via KubeJS MinecraftServer mixin reflectively). ### Hub UI + key rebind - **B** opens a new hub screen with buttons routing to each sub-screen - K/J/N/H/M direct shortcuts default to UNBOUND (rebindable for power users in vanilla controls menu) - M used to conflict with vanilla Social Interactions + FTB Chunks fullscreen map (hard conflict) — fully resolved - Audit verified B is free across vanilla 1.21.1 + all 97 pack mods ### NPC dialogue + scripting integration - New `/bns open <screen>` command with tab-complete (`hub|tier|bounty|plot|shop|questlog`) - Easy NPC dialogue COMMAND actions can now open any bnstoolkit screen — verified executable path via Easy NPC's `CommandExecutor.executePlayerCommand` - FTB Quests reward commands, command blocks, and `/trigger` objectives all route through the same verb - `config/easy_npc/security.cfg` ships with `executeAsUserCommandAllowList.ALL=bns` so dialogue-triggered `/bns` actually runs ### Network hardening - Per-player token-bucket rate limit (10/s sustain, 20 burst) - All STRING_UTF8 codec fields capped (`kind=8`, `args=80`, `which=16`) - Verb whitelist replaced with structured parse → regex-validate per verb → rebuild command (no prefix-startsWith surprises) - Tier-upgrade per-player in-flight guard (500ms) closes the concurrent-packet double-spend race - Bridge log lines sanitised so a hostile client can't inject control chars into server logs ### Review-driven nits - Per-player tick counter for HUD push (was a shared static — N-player skew) - VillagerTradeRebalance: try/catch around inner; null cost-A passthrough; round-to-nearest denom split (was floor → ~10% silent price haircut); silenced spammy null-entity pre-fetch warnings from recipe-viewer mods - SpurHud Component cache (was allocating 2 per frame) - `ClientBnsState.completedIds` → `HashSet` (O(1) per-frame lookups) - `BountyBoardScreen` dead branch removed - Wire-format version bumped 0.2 → 0.4 (added OpenScreenPacket) ## Verified live Server restart at 09:26 UTC with `bnstoolkit-0.4.1.jar`: - `Done (3.180s)` on port 25565 - `[bnstoolkit] registered /bns open <screen> command` logged at boot - `bnstoolkit/net] registered payloads v0.4` logged at boot - Zero `[bns/trades] inner ItemListing ... threw` warnings (down from 100+ in 0.4.0 — the null-entity pre-fetch noise is now silenced) - No ModLoadingException ## Test plan tomorrow - [ ] Press B in game — HubScreen opens with tier+balance header, 5 buttons - [ ] Each button opens its sub-screen with real data populated - [ ] Accept a bounty via screen, kill the mob — progress reflects in BountyBoardScreen + QuestLog - [ ] Buy a plot — PlotPurchaseScreen shows you as owner; another player sees "taken by [you]" - [ ] Spawn an Easy NPC, configure dialog with COMMAND action `/bns open shop`, click — bazaar opens - [ ] Try `/bns open hub` from console — should fail with "must be run as a player" - [ ] Press the now-unbound K (default) — nothing happens (correct) - [ ] Rebind K to OPEN_TIER in vanilla controls menu — direct shortcut works 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Matt added 1 commit 2026-06-07 09:28:20 +00:00
pack: v0.36.0 — bnstoolkit 0.4.1 (NBT fix + hub UI + NPC integration + hardening) ad9a261627 
Post-review consolidation PR. All four open issues from the parallel
code/security/hotkey/NPC audits resolved in one ship.

Key user-facing changes:

UI
- Single keybind: B opens a hub screen with buttons to the 5 sub-screens.
- K/J/N/H/M direct shortcuts default to UNBOUND so they no longer
  compete with the ~100 other mods. Rebindable in vanilla controls menu.
- M used to clash with vanilla Social Interactions + FTB Chunks
  fullscreen map; fully resolved.

NPC dialogue + scripting integration
- New /bns open <screen> command. Easy NPC dialogue COMMAND actions
  can now open any bnstoolkit screen on the clicking player.
- FTB Quests reward commands, command blocks, and /trigger objectives
  can all route into bnstoolkit screens via the same command.
- config/easy_npc/security.cfg ships with executeAsUserCommandAllowList
  .ALL=bns so dialogue-triggered /bns commands actually run.

Bug fix (this is the load-bearing item)
- Bounty board + plot office screens were reading NBT keys that don't
  exist in KubeJS, so they always showed "no active bounties" and "all
  plots free" no matter what was actually in the world. Fixed by reading
  the real per-player keys (bnsBountyPool/Active/Completed) and the
  server-scope bnsPlots compound via KubeJS' MinecraftServer mixin
  (reflective, no compileOnly dep on KubeJS).

Hardening (no behavior change, defense-in-depth)
- Per-player token bucket on C2S packets (10/s sustain, 20 burst).
- All STRING_UTF8 codec fields capped at sane sizes (≤80 chars).
- Verb whitelist replaced with structured parse + per-verb regex
  validation + rebuilt command. No prefix-startsWith surprises.
- Tier-upgrade in-flight guard closes the concurrent-packet
  double-spend race window.
- Log lines sanitised to prevent control-char injection.

Plus the nits caught by review:
- Per-player tick counter for HUD push (was a shared static)
- VillagerTradeRebalance try/catch + null-cost-A passthrough +
  round-to-nearest denom split (was floor — ~10% silent price haircut)
- SpurHud component cache (was allocating per frame)
- BountyBoardScreen dead branch removed

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Matt merged commit f2c9cabefe into main 2026-06-07 09:28:21 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Matt
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: minecraft/brass-and-sigil#74
Delete Branch "feature/v0.36.0-bnstoolkit-0.4.1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?