Skip to content

fix: stop a broken station's site choice from starting a build - #34

Merged
XxFran10xX merged 3 commits into
mainfrom
fix/orphan-station-placement
Nov 11, 2026
Merged

XxFran10xX merged 3 commits into
mainfrom
fix/orphan-station-placement

Conversation

@XxFran10xX

@XxFran10xX XxFran10xX commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

What went wrong

A player on Main picked a Windrider at an engineering table, then broke the table to cancel. Afterwards a "Constructing Windrider" text floated where the table had stood, no Windrider came, and the 16 oak logs, 16 white wool and 8 iron ingots were gone.

Breaking an idle station removed it, but the player's 30-second site choice stayed active. The next left click took the materials and started the build on the removed station. Only registered stations tick, so the build never progressed, nothing was refunded, and its timer text stayed until the chunk unloaded.

Changes

  • Removal: removing a station (block or furniture break) now cancels any site choice made at it. Those players are told "Building site choice cancelled: the station was removed."
  • Registration check: a build-site click, and a blueprint clicked in a menu left open, now start only if that exact station is still registered. A new station placed on the same block does not count.

An earlier commit also made "left-click the station to cancel" a feature. The Dev test showed that survival clicks on an ItemsAdder custom-block station never reach PlayerInteractEvent, because ItemsAdder runs the mining. That cancel only worked in creative, so 19adfee removes it and the prompt is back to its old wording.

Testing

  • Five new ManagerTest cases. A reviewer applied the first ones to the old StationManager, and they fail there. All 66 tests pass. LifecycleTest is skipped locally because it sets POSIX permissions, which Windows refuses; CI runs it. StationManager stays at 100% coverage.
  • Three independent review rounds, all VERDICT: APPROVE. Round 3 reviewed the removal.
  • Dev bot test (~/vfb-orphan-test/test.js, two real-character bots, Windrider at an engineering table):
    • 2.2.2 (baseline): the second bot breaks the station during the first bot's site choice, and the first bot's next left click starts the build anyway. The materials are taken and the "Constructing Windrider" text stays. This is the reported bug, reproduced.
    • This branch: all 20 checks pass.
      • A, Velvet's case: the station is broken during the choice. No build starts, the materials stay with the player, no text is left, and the player sees the cancel message.
      • A2, menu left open: the station is broken while the blueprint menu is open. Clicking the blueprint starts no site choice.
      • C, normal build: it starts and shows its text. Breaking the station then cancels it, drops the materials and removes the text.

Follow-ups (existing behaviour, not changed here)

  • A station block destroyed without a player (explosion, piston, WorldEdit) stays registered.
  • onBlockBreak runs at NORMAL priority. A plugin that cancels the break later still gets the station unregistered.
  • Furniture stations taller than one block register at the clicked hitbox block. No live station uses furniture; all are ItemsAdder blocks.

🤖 Generated with Claude Code

XxFran10xX and others added 2 commits November 11, 2026 12:52
Breaking an idle station while choosing a building site left the
placement active. The next left click then took the materials and
started the build on the removed station: it never ticked, never
refunded, and left a "Constructing" text floating where it stood.

- Removing a station drops the site choices made for it.
- The build-site click and the blueprint menu check that the station
  is still registered.
- Left-clicking the station itself now cancels the site choice.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed removal save

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Explain Change →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 36 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d0d0a3a4-22a7-4724-8534-f489361cf309
Commits

Reviewing files that changed from the base of the PR and between 95f3ff5 and 19adfee.

Files selected for processing: 2

  • src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java
  • src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

  • Bug Fixes
    • Prevented blueprint placement from proceeding when its station has been removed or replaced.
    • Pending placements linked to a removed station are now cancelled, with affected players notified.
    • Clicking the station itself cancels site selection; clicking another block can still confirm the build.
    • Improved handling of station-removal failures so restored stations retain their pending site choices.

Station placement now checks whether a station remains registered before starting or confirming a placement. Removing a station clears pending choices tied to that station instance. Tests cover stale placements, cancellation, choice isolation, and failed removal saves.

Changes: Station placement cancellation

Layer / File(s) Summary
Placement selection and site validation
src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java, src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java
Blueprint selection and site selection reject removed stations. Left-clicking the selected station cancels the choice. Tests also cover stale placements, including when a station is recreated at the same location, and confirm that another block can complete construction.
Pending choices during station removal
src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java, src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java
Station removal cancels pending choices tied to the removed station instance and notifies the affected players. Tests cover isolation between stations and retention of a site choice when a failed save restores the station.

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

Change: Bug fix

Possibly related PRs

  • TF-Minecraft/VFBuilders#32: Changes station-removal persistence and construction cancellation in StationManager, which is connected to this PR’s station-removal handling.

Suggested labels: size:L

Suggested reviewers: ryanbarlow97

Merge Risk: Minimal · previous review 95f3f

This change stops a removed station's site choice from starting a build that consumes materials. No merge-blocking risk is evident from the supplied review.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 89ebc7d1-dd5c-409f-bb28-24a456964d9e
Commits

Reviewing files that changed from the base of the PR and between 145d450 and 95f3ff5.

Files selected for processing: 2

  • src/main/java/net/tfminecraft/vfbuilders/managers/StationManager.java
  • src/test/java/net/tfminecraft/vfbuilders/ManagerTest.java

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

@github-actions github-actions Bot added the size:M 30-99 effective changed lines (tests excluded in mixed PRs). label Nov 11, 2026
@XxFran10xX

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Nov 11, 2026
ItemsAdder runs the mining of its custom-block stations, so a survival
player's left click on the station never fires PlayerInteractEvent. The
prompt promised a cancel that only worked in creative. Breaking the
station still cancels the choice with a message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@XxFran10xX
XxFran10xX marked this pull request as ready for review November 11, 2026 12:25
@github-actions github-actions Bot added size:S 10-29 effective changed lines (tests excluded in mixed PRs). and removed size:M 30-99 effective changed lines (tests excluded in mixed PRs). labels Nov 11, 2026
@XxFran10xX

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@XxFran10xX

Copy link
Copy Markdown
Contributor Author

CodeRabbit was rate limited on 19adfee, so this was reviewed independently (Fable 5.2, CodeRabbit-style): VERDICT: APPROVE, no critical or major findings. Minor follow-up: in inventoryClick the materials check runs before the registration check, so a player without materials clicking a blueprint on a removed station's menu sees "You lack the materials" instead of "The station was removed". Nothing is lost.

@XxFran10xX
XxFran10xX merged commit 0a5c1bd into main Nov 11, 2026
3 of 4 checks passed
@XxFran10xX
XxFran10xX deleted the fix/orphan-station-placement branch November 11, 2026 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 effective changed lines (tests excluded in mixed PRs).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant