Skip to content

feat: choose gun ammunition with crouch + right-click - #31

Merged
XxFran10xX merged 2 commits into
mainfrom
feat/ammo-select
Oct 5, 2026
Merged

XxFran10xX merged 2 commits into
mainfrom
feat/ammo-select

Conversation

@XxFran10xX

@XxFran10xX XxFran10xX commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Why

Player suggestion zmfy3-6wiir (D_ta, 7 ✅ / 0 ❌): "Change how Musketeer weapons pick which ammo in your inventory they use."

A reload loads the first caliber in the gun's list that the player carries. On Main every list runs from weakest to strongest (e.g. rifled barrels: ironshot → ironshot_powered → steelshot). Anyone carrying one iron shot could never load steel or powered shot without dropping the iron first.

What changes for players

  • Crouch + right-click with a gun steps to the next caliber you carry. Calibers you don't carry are skipped. The action bar shows Next load: <shot> (N carried) with a lever click. The pick is saved on the gun, so each gun remembers its own.
  • Reloads use only the picked shot. When it runs out, the gun does not quietly load something else (so you don't burn mythril by accident). The action bar says You have no <shot> left. Crouch and right-click to choose another.
  • Guns with no pick behave as before (first carried caliber in config order). The same applies if the pick stops being a valid caliber, e.g. after a part change.
  • Reloading a gun that accepts several calibers shows Loading <shot> on the action bar.
  • Newly crafted or refreshed guns with more than one caliber get a lore line under Caliber: Crouch and right-click to change shot.
  • Loaded bullets stay loaded; the pick applies to the next reload. Crouch-clicking during a reload does nothing.
  • Behaviour change: crouching + right-click no longer fires or reloads. Guns have no crouch abilities on Main (checked the MMOItems gun templates), and accuracy doesn't depend on crouching. The F key was not an option because MMOCore uses SWAP_HANDS to open skill casting.

Arclock caliber fix

resolveCalibers kept only the last entry of a caliber-override list. On Main the Arclock overrides to bronzeshot, bronzeshot_powered, mythrilshot, mythrilshot_powered, so Arclock rifles only accepted mythrilshot_powered. Now the last part that has an override supplies its whole list. Steamlock (ironshot only) is unaffected.

Calibers are stamped on the gun when it is built. Existing Arclocks keep the old single caliber until they are rebuilt: either /gg refresh while holding one, or the automatic refresh after an Arclock part config change.

Implementation notes

  • New PDC key gunsandgadgets:ammo_selected (ammo key). copyRuntimeAmmoPdc carries it through rebuilds/refreshes.
  • Ammo scanning in takeAmmo is split into findAmmo / countStacks so switching can count stacks the same way reloads do.
  • One click can reach handleGunUse as several interact events: one per hand on a block, or an entity click plus a right-click-air. These can land in neighbouring ticks; on Dev, one block click in eight switched twice with a same-tick guard. A switch request within 2 ticks of the last accepted one is now ignored. Clients allow a new click only every 4 ticks, so deliberate clicks are never swallowed.
  • MMOCore's stat bar (❤ … | ★ … | ⛨ …, every 5 ticks on Main) overwrote the message within ~0.25 s. Messages now reserve MythicLib's action bar at NORMAL priority for 40 ticks (2 s) before sending. If a higher-priority bar holds it, the message is skipped.
  • No config or message-file changes; messages follow the in-character wording from fix: reword player messages in character #28.

Dev test (TFMCDev01, mineflayer bot as a Musketeer)

Covered: the lore hint, the first crouch-click picking steel over iron, the action bar text, the HUD pause, block clicks with duplicated off-hand packets, steel loading ahead of iron, crouch-click not firing a loaded gun, refusal once steel runs out (iron untouched), switching back to iron, the no-shot message, an Arclock rifle listing 4 calibers and loading bronze shot. Results are in the PR comments.

Tests

mvn clean verify: 171 tests pass, 100% line/branch/instruction coverage. New tests cover switching (skip + wrap, no fire/reload, repeat events of one click within 2 ticks ignored), no usable ammo, reload with a pick, refusal once the pick runs out (and recovery), stale pick fallback, the single-caliber case, override lists, the lore hint, preserving the pick on rebuild, and the action-bar reservation (and yielding to a busier bar).

🤖 Generated with Claude Code

Reloads took the first caliber in the gun's list that the player
carried, so anyone holding a single iron shot could never load steel or
powered shot. Crouch + right-click now steps to the next caliber the
player carries and saves it on the gun; reloads use only that pick and
say so when it runs out. Guns without a pick keep the old order.

Caliber overrides also kept only the last caliber of the list, so
Arclock rifles accepted mythrilshot_powered alone. The last part with an
override now supplies all of its calibers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 835c3e59-73c0-4622-b6c6-c8f1ce79053c
📥 Commits

Reviewing files that changed from the base of the PR and between 9f88047 and 3798603.

📒 Files selected for processing (2)
  • src/main/java/net/tfminecraft/gunsandgadgets/manager/GunManager.java
  • src/test/java/net/tfminecraft/gunsandgadgets/manager/GunManagerTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.


📝 Summary

Summary by CodeRabbit

  • New Features
    • Players can crouch and right-click to choose which carried calibre a multi-calibre gun uses for its next reload. The choice is saved on the gun.
    • Reloads show the calibre being loaded when a gun accepts multiple calibres. If the selected calibre is unavailable, no ammunition is used and the player is notified.
    • Gun descriptions indicate when calibre selection is available.
  • Bug Fixes
    • Calibre overrides now follow the last part with an override, and saved ammunition selections are preserved when guns are rebuilt. If a saved selection is no longer compatible, reloads fall back to available calibres.

Walkthrough

Guns with multiple calibres can save a selected calibre and use it for the next reload. Players can cycle through supported calibres they carry by sneaking and right-clicking. Calibre resolution, gun rebuilding, action-bar messages, and tests are updated.

Changes

Ammunition selection

Layer / File(s) Summary
Calibre resolution and saved state
src/main/java/net/tfminecraft/gunsandgadgets/manager/inventory/InventoryManager.java, src/test/java/net/tfminecraft/gunsandgadgets/manager/inventory/InventoryManagerTest.java, README.md
The last part with a non-empty override supplies the resolved override list. Rebuilt guns preserve the selected-ammunition value. Multi-calibre lore and the README describe the selection control. Tests cover override resolution and runtime state copying.
Cycle carried calibres
src/main/java/net/tfminecraft/gunsandgadgets/manager/GunManager.java, src/test/java/net/tfminecraft/gunsandgadgets/manager/GunManagerTest.java
A sneaking right-click cycles through supported calibres the player carries and saves the selection on the gun. Action-bar messages report the selected calibre and carried amount. Tests cover cycling, repeated events, and no carried ammunition.
Reload with the selected calibre
src/main/java/net/tfminecraft/gunsandgadgets/manager/GunManager.java, src/test/java/net/tfminecraft/gunsandgadgets/manager/GunManagerTest.java
Reloads use a valid saved selection. If that calibre has no ammunition, the reload does not reserve another calibre’s ammunition. Tests cover selected, unavailable, invalid, and single-calibre reloads.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant Player
  participant GunManager
  participant PlayerInventory
  participant GunItem
  Player->>GunManager: Sneaking right-click
  GunManager->>PlayerInventory: Find carried supported calibres
  PlayerInventory-->>GunManager: Matching ammunition stacks
  GunManager->>GunItem: Save selected calibre
  GunManager-->>Player: Show selected calibre and carried amount
  Player->>GunManager: Reload
  GunManager->>GunItem: Read selected calibre
  GunManager->>PlayerInventory: Take ammunition for that calibre
Loading

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 37986

This change adds saved calibre selection and selection-aware reloads; the available evidence establishes no concrete user-facing failure that should block merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 37986

Ammunition choice remains constrained to the held gun’s compatible ammunition and the player’s inventory. No expanded privileges or cross-player access were identified. A bounded recovery concern remains because new ammunition notifications execute before reload cleanup or task creation completes.

Retained concerns

  • Low · reliability · inferred: New ammunition notifications are on the reload transition’s critical path without failure isolation. If notification generation or delivery throws, the no-ammo branch skips clearing the player’s reload lock; after ammunition reservation, initiation can stop before a completion task is created. Slot-change cancellation can recover reserved ammunition, but automatic recovery from this interruption is not established. This is a conditional failure-containment concern, not a verified security exploit.
Security review details

Security Blast Radius

  • inferred — The traced new behavior is scoped to an interacting player’s held gun and ammunition inventory. It introduces no identified cross-player mutation or privileged operation. The conditional notification failure affects that player’s reload state and reserved ammunition.

Trust Boundaries and Controls

  • observed — Selection reuses the existing event entrypoints and item/class controls rather than adding a separate authority path. Compatibility validation and inventory matching remain between player choice and ammunition consumption. No bypass of those controls was established in the compared paths.

Resilience and Maintainability Implications

  • inferred — State-transition conclusions rely on normal serialized Bukkit event/task execution. Duplicate-click and normal cancellation paths are covered by source and inspected tests, but genuine multithreaded invocation and the pinned notification dependency’s failure behavior were not established.

Hardening Proposals

  • proposed — Keep optional ammunition notifications outside reload cleanup and reservation-completion guarantees, so presentation failure cannot interrupt resource recovery. Validate the null-message contract against the exact deployed dependency versions.
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Oct 5, 2026
Dev testing showed MMOCore's stat bar redrawing over the ammo message
within a quarter second; reserve MythicLib's action bar at NORMAL
priority for two seconds before sending it, and skip the message when a
busier bar holds it. Trim the trailing space from ammo item names.

One crouch-click on a block switched twice about one time in eight: its
main- and off-hand events can land in neighbouring ticks. Ignore switch
requests within two ticks of the last one; clients allow a new click
only every four ticks.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@XxFran10xX

Copy link
Copy Markdown
Contributor Author

Dev test: TFMCDev01, commit 3798603

Ran a mineflayer bot as a Musketeer against the PR jar (gunsandgadgets-DEV-ammoselect.jar), with Dev's live parts.yml and ammunition.yml (ammunition is identical to Main's). All 23 checks pass:

# Check Result
1 gg give flintlock rifle (rifled barrel: iron, powered iron, steel); lore shows Crouch and right-click to change shot PASS
2 Carrying 5 iron + 2 steel, first crouch-click picks steel (powered iron not carried, so skipped); bar Next load: Steel Shot (2 carried); no ammo taken PASS
2 MMOCore stat bar paused after the message (2069 ms until it came back) PASS
3 6 crouch-clicks on a block, each sending main- and off-hand packets: exactly one switch each (1,1,1,1,1,1) PASS
4 Reload: Loading Steel Shot, 1 steel taken, iron untouched, gun loaded with steelshot PASS
5 Crouch-click on a loaded gun switches without firing (bullets stay 1) PASS
6 Fire, reload the last steel, fire; next reload refuses: You have no Steel Shot left. Crouch and right-click to choose another. Iron stays at 5 PASS
7 Crouch-click with only iron left: Next load: Iron Shot (5 carried); reload takes iron PASS
8 No shot carried: You carry no shot this weapon can fire. PASS
9 Arclock rifle lists 4 calibers and loads bronze shot (before this PR it accepted only mythrilshot_powered) PASS

No GunsAndGadgets errors in Dev's latest.log.

The first Dev run found the two problems fixed in 3798603: the stat bar overwrote the message within about 0.25 s, and 1 in 8 block clicks switched twice.

@XxFran10xX
XxFran10xX merged commit 09193c5 into main Oct 5, 2026
2 checks passed
@XxFran10xX
XxFran10xX deleted the feat/ammo-select branch October 5, 2026 19:18
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