Skip to content

test: enforce full GunsAndGadgets coverage and fix runtime regressions - #26

Merged
ryanbarlow97 merged 2 commits into
mainfrom
test/full-coverage
Sep 29, 2026
Merged

ryanbarlow97 merged 2 commits into
mainfrom
test/full-coverage

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

GunsAndGadgets had no executable coverage suite. Add regression tests covering firing/collisions, reload/ammo accounting, crafting/inventories, stats/skins/provenance, persistence/configuration, commands/lifecycle and automatic refresh.

Fix the defects exposed by those tests:

  • Shots could damage an entity behind a nearer wall; registered vehicle armor stands bypassed vehicle damage. Respect nearer block collisions and prioritize vehicle hitboxes, preserving passable glass/leaves behavior.
  • Equal gun IDs stored in distinct String objects interrupted reloads; negative capacities duplicated ammo. Compare ID contents and reject nonpositive ammo requests.
  • Free-crafting mode still consumed ingredients. Invalid/cancelled rebuilds could consume materials or replace an existing gun; full inventories lost crafted outputs. Validate outputs before consumption/replacement and drop inventory leftovers.
  • Missing skins crashed output construction, malformed numeric part stats aborted loading, and the Stats header condition was reversed.

mvn clean verify now enforces 100% production line, branch and instruction coverage without exclusions. CI uploads JaCoCo reports. Tests use real dependency types and mock external server/plugin boundaries. Small refactors remove checks ruled out by callers/API contracts and make private random calculations deterministic in tests.

Build inputs include checksum-pinned NBTAPI/ModelEngine and fastutil for integration tests. CoreProtect is declared as a provided API dependency for VehicleFramework type loading, using the shared verified-release installer; none of these dependencies are bundled in the runtime JAR.

Validation: Java 21 clean offline Maven verify passed with 161 tests, zero failures/errors/skips; 2,335/2,335 lines, 1,196/1,196 branches and 10,879/10,879 instructions. No live-server deployment performed.

Additional regressions fix the remaining damage/accuracy and ammunition issues: rocket self-damage scales linearly and ignores arming-time reduction, piercing is clamped to 0–100% and can be lethal, and accuracy improves continuously from the existing zero-stat spread with a 0.07-degree floor. Cancelled reloads refund exact consumed item snapshots even after skin/ammo configuration changes; overflow drops safely. Reservation identity prevents an older reload task from interfering with a newer one. Unknown ammunition definitions preserve existing loaded state rather than consuming it.

Summary by CodeRabbit

  • Bug Fixes
    • Crafting now preserves materials when an output cannot be created and drops completed items that do not fit in the inventory.
    • Cancelled reloads return reserved ammunition, and unavailable ammunition no longer leaves firing or unloading in an inconsistent state.
    • Accuracy spread calculations and projectile damage behavior have been corrected. Projectiles can also hit registered vehicles.
    • Invalid gun-part stat values are skipped, and the “Stats:” lore header is no longer duplicated.
  • Tests
    • Expanded automated coverage for crafting, reloading, projectiles, configuration, and other gameplay systems.

@coderabbitai

coderabbitai Bot commented Sep 29, 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: 82dbc827-12c2-492c-aeee-54686f794b58

📥 Commits

Reviewing files that changed from the base of the PR and between c15ad38 and 00e8c6e.

📒 Files selected for processing (29)
  • .github/dependencies.sha256
  • .github/scripts/install-local-dependencies.sh
  • .github/scripts/prepare-release.sh
  • .github/workflows/build.yml
  • README.md
  • pom.xml
  • src/main/java/net/tfminecraft/gunsandgadgets/guns/parts/GunPart.java
  • src/main/java/net/tfminecraft/gunsandgadgets/guns/skins/SkinData.java
  • src/main/java/net/tfminecraft/gunsandgadgets/guns/skins/SkinResolver.java
  • src/main/java/net/tfminecraft/gunsandgadgets/guns/stats/StatApplier.java
  • src/main/java/net/tfminecraft/gunsandgadgets/guns/stats/StatCalculator.java
  • src/main/java/net/tfminecraft/gunsandgadgets/manager/CraftingManager.java
  • src/main/java/net/tfminecraft/gunsandgadgets/manager/GunManager.java
  • src/main/java/net/tfminecraft/gunsandgadgets/manager/inventory/InventoryManager.java
  • src/main/java/net/tfminecraft/gunsandgadgets/shooter/ProjectileShooter.java
  • src/main/java/net/tfminecraft/gunsandgadgets/util/ImpactVfx.java
  • src/main/java/net/tfminecraft/gunsandgadgets/utils/GunBrokenMarker.java
  • src/main/java/net/tfminecraft/gunsandgadgets/utils/GunStatRefresher.java
  • src/main/java/net/tfminecraft/gunsandgadgets/utils/TierLore.java
  • src/test/java/net/tfminecraft/gunsandgadgets/ConfigurationTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/GunDomainTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/GunRefreshTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/GunsLifecycleCommandTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/GunsTestSupport.java
  • src/test/java/net/tfminecraft/gunsandgadgets/HelpersTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/manager/CraftingManagerTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/manager/GunManagerTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/manager/inventory/InventoryManagerTest.java
  • src/test/java/net/tfminecraft/gunsandgadgets/shooter/ProjectileShooterTest.java
💤 Files with no reviewable changes (1)
  • src/main/java/net/tfminecraft/gunsandgadgets/utils/TierLore.java

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


📝 Walkthrough

Walkthrough

The pull request adds Maven test and coverage configuration, broad automated tests, and changes to configuration parsing, crafting, gun management, inventory handling, and projectile behavior.

Changes

Gameplay behavior and test coverage

Layer / File(s) Summary
Test dependencies and coverage pipeline
.github/dependencies.sha256, .github/scripts/*, .github/workflows/build.yml, pom.xml, README.md
Maven adds test dependencies and JaCoCo coverage checks. Release scripts download and install pinned plugin JARs with checksums. CI uploads coverage reports, and the README documents test execution and coverage requirements.
MockBukkit setup and lifecycle commands
src/test/java/net/tfminecraft/gunsandgadgets/GunsTestSupport.java, src/test/java/net/tfminecraft/gunsandgadgets/GunsLifecycleCommandTest.java
Shared test support initializes and cleans up MockBukkit state. Lifecycle and command tests cover plugin setup, reload and refresh commands, and tab completion.
Configuration, gun data, and refresh behavior
src/main/java/net/tfminecraft/gunsandgadgets/guns/*, src/main/java/net/tfminecraft/gunsandgadgets/util/*, src/main/java/net/tfminecraft/gunsandgadgets/utils/*, src/test/java/net/tfminecraft/gunsandgadgets/ConfigurationTest.java, src/test/java/net/tfminecraft/gunsandgadgets/GunDomainTest.java, src/test/java/net/tfminecraft/gunsandgadgets/GunRefreshTest.java, src/test/java/net/tfminecraft/gunsandgadgets/HelpersTest.java
Parsing, skin resolution, stat calculations and lore, impact effects, broken-gun markers, and refresh validation change. Tests cover configuration loaders, gun-domain data, utility behavior, and refresh outcomes.
Crafting costs and inventory item handling
src/main/java/net/tfminecraft/gunsandgadgets/manager/CraftingManager.java, src/main/java/net/tfminecraft/gunsandgadgets/manager/inventory/InventoryManager.java, src/test/java/net/tfminecraft/gunsandgadgets/manager/CraftingManagerTest.java, src/test/java/net/tfminecraft/gunsandgadgets/manager/inventory/InventoryManagerTest.java
Crafting rejects unusable outputs before consuming materials, applies configured input requirements, and drops inventory leftovers. Inventory handling adds a missing-skin barrier result and removes several null checks. Tests cover crafting and inventory behavior.
Reload reservations, refunds, and ammunition
src/main/java/net/tfminecraft/gunsandgadgets/manager/GunManager.java, src/test/java/net/tfminecraft/gunsandgadgets/manager/GunManagerTest.java
GunManager records exact ammunition stacks for reload refunds, validates reload tasks, and returns refunded or unloaded items to inventory or drops. Firing validates ammunition before proceeding. Tests cover reloads, firing, crossbows, and item restrictions.
Projectile collision and damage
src/main/java/net/tfminecraft/gunsandgadgets/shooter/ProjectileShooter.java, src/test/java/net/tfminecraft/gunsandgadgets/shooter/ProjectileShooterTest.java
Projectile tasks receive a random source for rocket wobble. Entity tracing includes registered vehicles, vehicle hits apply projectile or rocket damage, and block traversal stops at the nearer collision. Damage calculations and tests cover entity, vehicle, and block interactions.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ProjectileShooter
  participant World
  participant VehicleFramework
  participant HitTarget
  ProjectileShooter->>World: Ray-trace projectile segment
  World-->>ProjectileShooter: Return entity and block hit positions
  ProjectileShooter->>VehicleFramework: Resolve hit entity as a vehicle when enabled
  VehicleFramework-->>ProjectileShooter: Return vehicle or no vehicle
  alt Registered vehicle hit
    ProjectileShooter->>HitTarget: Apply projectile or rocket damage
  else Living entity hit
    ProjectileShooter->>HitTarget: Apply entity damage and hit effects
  end
Loading

Merge Risk: ⚪ Minimal · up to 00e8c

This change adds broad test coverage and fixes to crafting, reloading, and projectile handling. The evidence available shows no established defect that should block merging. Whether a shot can damage the shooter's own vehicle has not been confirmed and may be worth a follow-up check.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 00e8c

The changes mostly strengthen validation and recovery. One new full-inventory fallback places crafted weapons in the shared world, where another player may be able to collect them. Release dependency checks are present, but some deployment-specific controls remain unverified.

Retained concerns

  • Low · security · inferred: When an inventory is full, the new delivery path drops the crafted output into the world without an owner restriction in this handler. A nearby player may be able to collect an item paid for by the crafter; effective pickup protection depends on server behavior not established here.
Security review details

Security Blast Radius

  • inferred — The identified exposure is a crafted item dropped at one player’s location when their inventory cannot accept it. No evidence establishes a service-wide privilege change or a broader asset compromise from this path.

Security Findings and Attack Paths

  • inferred — A nearby player could collect a newly dropped crafted weapon if the server does not apply pickup protection. The full-inventory fallback prevents silent loss, but this handler supplies no ownership restriction for the drop.

Trust Boundaries and Controls

  • observed — The release script requires a token, and checksum verification precedes local installation of downloaded JARs. The checked-in controls do not prove the provenance of the manifest or production secret permissions.

Resilience and Maintainability Implications

  • inferred — Reload refunds are memory-backed; the reviewed shutdown hook flushes revisions but does not demonstrate recovery of an outstanding reservation. Whether an interruption can worsen the pre-existing ammunition-loss behavior remains unresolved.

Hardening Proposals

  • proposed — Consider an owner-restricted drop or another owner-controlled delivery fallback for crafted outputs, subject to the server’s intended loot rules.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 252 functions across 24 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: enforcing full GunsAndGadgets test coverage and fixing runtime regressions.
Full details: Docstring Coverage

Explanation

Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 252 functions across 24 files. (4 skipped: 4 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the coverage rows,
Then hops where JaCoCo’s green light glows.
A cartridge finds its proper place,
A rocket traces through the space.
The tests all spring from burrowed ground,
And little bunny paws applaud the sound.

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

@ryanbarlow97
ryanbarlow97 marked this pull request as ready for review September 29, 2026 19:02
@ryanbarlow97
ryanbarlow97 merged commit 45d1fbe into main Sep 29, 2026
2 checks passed
@ryanbarlow97
ryanbarlow97 deleted the test/full-coverage branch September 29, 2026 19:11
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