Skip to content

Add hooks for confirming and refunding construction charges - #24

Merged
Drefvelin merged 1 commit into
mainfrom
feat/construction-fee-hooks
Sep 27, 2026
Merged

Drefvelin merged 1 commit into
mainfrom
feat/construction-fee-hooks

Conversation

@Drefvelin

@Drefvelin Drefvelin commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

SimpleFactions is adding vehicle registration fees charged when a build starts, with a confirm step. This adds the hooks it needs.

  • BeginVehicleConstructionEvent#setKeepPlacement(true): when a listener cancels the event with this set, the player's placement stays active, so another left-click within the 30-second window fires the event again. Without the flag, behaviour is unchanged.
  • VehicleConstructionCancelEvent: fired from ActiveStation.cancelConstruction() after the materials are dropped (station removed, or its blueprint vanished on reload), so charges taken at the start can be refunded.
  • CategoryLoader.map is now a LinkedHashMap (the field type stays HashMap for binary compatibility), so the station menu and other plugins list categories in config order.

Consumer: TF-Minecraft/SimpleFactions vehicle fees PR (pins this release).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Construction cancellations now trigger an event with details about the constructor, blueprint, and station.
    • Event listeners can choose whether a placement remains active when construction is cancelled.
  • Improvements
    • Categories now retain their configured order when displayed.

BeginVehicleConstructionEvent gains keepPlacement, so a listener can cancel the
first left-click and wait for a confirming second one without the player having
to reopen the station menu. A new VehicleConstructionCancelEvent fires when a
started construction is abandoned, so charges taken at the start can be refunded
alongside the dropped materials. Categories now keep their config order.

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

coderabbitai Bot commented Sep 27, 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: 0fd4450c-1a17-49e8-9302-edf20053e13c

📥 Commits

Reviewing files that changed from the base of the PR and between d38ca30 and 8a3c54a.

📒 Files selected for processing (5)
  • src/main/java/net/tfminecraft/vfbuilders/core/ActiveStation.java
  • src/main/java/net/tfminecraft/vfbuilders/events/BeginVehicleConstructionEvent.java
  • src/main/java/net/tfminecraft/vfbuilders/events/VehicleConstructionCancelEvent.java
  • src/main/java/net/tfminecraft/vfbuilders/loaders/CategoryLoader.java
  • src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java

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


📝 Walkthrough

Walkthrough

Construction events now support retaining placement after cancellation and report abandoned construction details. The category map now preserves insertion order during iteration.

Changes

Construction lifecycle

Layer / File(s) Summary
Construction event contracts
src/main/java/net/tfminecraft/vfbuilders/events/BeginVehicleConstructionEvent.java, src/main/java/net/tfminecraft/vfbuilders/events/VehicleConstructionCancelEvent.java
BeginVehicleConstructionEvent adds a placement-retention flag. VehicleConstructionCancelEvent exposes the constructor UUID, blueprint, and station.
Construction event handling
src/main/java/net/tfminecraft/vfbuilders/core/ActiveStation.java, src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java
Station cancellation fires VehicleConstructionCancelEvent when a blueprint is present. Cancelled begin events retain active placement when the flag is set.

Category ordering

Layer / File(s) Summary
Ordered category map
src/main/java/net/tfminecraft/vfbuilders/loaders/CategoryLoader.java
The category map is initialized with a LinkedHashMap instead of a HashMap.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 8a3c5

The construction hooks and category ordering appear ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 8a3c5

The new refund hook covers ordinary cancellation, but it does not provide a reliable signal for every recovery path. A failed or repeated cancellation could also leave materials and an external charge out of sync. No direct fee exploit is established because the charging plugin is outside this review.

Retained concerns

  • Medium · security · inferred: The new refund signal is emitted after materials are dropped but before construction state is cleared. Re-entry or interruption during event delivery can leave the material return and a consumer’s refund inconsistent; the event supplies no durable transaction identifier for reconciliation.
  • Medium · security · inferred: A saved construction whose blueprint definition is missing at startup is loaded without an active blueprint, so it cannot emit the new cancellation event. This leaves a potential refund gap for charges associated with such constructions, although blueprint disappearance on startup predates this PR.
Security review details

Security Blast Radius

  • inferred — The exposed path is a player’s construction attempt and any external fee tied to it, rather than a demonstrated server-wide privilege or infrastructure change. Player clicks can reach begin-event listeners; station removal and definition reload can reach cancellation listeners.

Security Findings and Attack Paths

  • inferred — A listener re-entering cancellation before the first dispatch returns can encounter an uncleared blueprint and repeat both the material drop and cancellation signal. Ordinary sequential calls cannot do this after cleanup; no untrusted-player route to listener re-entry is established.

Trust Boundaries and Controls

  • observed — A retained retry remains keyed to the clicking player’s UUID, checks construction distance and required inputs, and expires through timeout or player-quit cleanup. Retention requires a listener to cancel the event and opt in; the new flag defaults to false.

Resilience and Maintainability Implications

  • inferred — Constructor UUID supports attribution for ordinary new constructions, but neither it nor the newly exposed station object establishes a durable, unique fee transaction across cancellation and restart.

Hardening Proposals

  • proposed — Define a durable construction or charge identifier and idempotent refund handling with the consuming plugin; specify recovery for missing blueprints at startup and for missed event delivery before relying on the hook for financial compensation.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: it adds hooks that support confirming and refunding vehicle construction charges.
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.
  • 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 watched the stations hum
As blueprints paused and events came
It kept a place for one more try
Then sorted categories in a line
And hopped away beneath the sky.

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

@Drefvelin
Drefvelin merged commit 85ac9dd into main Sep 27, 2026
2 checks passed
@Drefvelin
Drefvelin deleted the feat/construction-fee-hooks branch September 27, 2026 12:56
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.

2 participants