Stop auto mode repairing healthy factories - #5
Closed
grepsedawk wants to merge 28 commits into
Closed
Conversation
Should fix the issue with 2x2 trees not growing correctly or deleting saplings in the process
namelayer had no working tests and a few latent bugs, which made it risky to touch. This change fixes the bugs behind a new safety net and clears the way for further cleanup, without altering the public API that the ~15 dependent plugins rely on. Seal the last facade leak: citadel looked up a player's groups through namelayer's internal DAO. Route it through the public GroupManager.getAllGroupNames so no plugin reaches past the facade. Delete the orphaned group-link command and GUI classes (LinkGroups, UnlinkGroups, LinkingGUI). They were never registered and only added noise. The group-merge code is left untouched pending a decision on whether to revive or remove it. Add a MockBukkit/JUnit/Mockito test harness, modeled on civmodcore, plus characterization tests that pin current behavior as a regression net for the fixes below. Fix three confirmed bugs: a cancelled GroupCreateEvent still inserted the group; getDefaultGroup read a ResultSet without next(); and isPassword threw on a group that has no password. Collapse the per-role N+1 member load in the Group constructor into a single query, and make cache invalidation O(1) instead of a linear scan.
Add a "Time remaining: N <unit>" line to pearl item lore directly beneath the existing health line. The interval count is computed from the configured decay rate, rounded up, and uses the same admin-configured unit string as the existing repair-cost lines, so operators control plurality and translation in one place. The line is hidden when the pearl is inactive (decay is already flagged separately as "suspended due to Inactivity"), when health has reached zero, or when decay is disabled in config. While here, hoist the shared decay-per-interval and unit values so the existing repair-cost loop reuses them instead of recomputing per material.
compareTo read this block's world UID for both operands, so the world comparison was always zero and blocks in different worlds with identical coordinates compared equal. That breaks the Comparable contract and corrupts TreeSet/TreeMap membership. Read the other block's world instead, and add a regression test covering cross-world ordering and antisymmetry.
The convenience insertData overloads delegated to the canonical method with the server and world arguments transposed, so callers using the documented (server, world) order had the two columns stored swapped. No caller in the tree relied on the broken order. Pass the arguments in the correct order, and add a test asserting server and world each bind to their own column.
Migrator prepared the REPLACE INTO migrations statement and bound its parameters but never executed it, so the applied id was never stored and every migration re-ran on each boot. Execute the statement, and close the prepared and created statements with try-with-resources to fix the leaks.
2x2 Tree grow fix
…ing-on-pearls Show days remaining on pearls
Factory QoL and Decompactor fixes
Fix world comparison in BastionBlock.compareTo
…world Fix swapped server/world in insertData overloads
Persist applied migration id in Migrator
CivScoreBoard truncates the displayed line at 40 characters but caches the full string, so a destination longer than ~29 chars left a stale line that never cleared when switching destinations. Cap the rendered value so the line fits and the cached and displayed text match. Also null-guard the setting-change listener to match restoreDestinationDisplay, avoiding a stray update during reload.
Show rail destination on the sidebar scoreboard
GroupManager.getGroup(int) queries the database on the calling thread, and a not-found result was never cached. Citadel reinforcements and JukeAlert snitches keep referencing deleted group ids forever, so every interaction with such a block was a blocking main-thread round-trip. On CivPVP one orphaned id (a Name_Layer_Special row missing from the faction table) drove ~100 queries/second during fights, lining up with every multi-second watchdog stall in the logs. Remember ids that resolved to nothing and return null without querying. The negative cache is cleared wholesale in invalidateCache so a merge or manual repair is picked up without a restart.
A null group is a normal access denial: movement handlers such as Bastion's bastion overlay routinely ask hasAccess about groups whose reinforcement is on an unloaded chunk, so getGroup returns null. The old guard treated that as a caller bug and logged a full stack trace at INFO, spamming the console on every move event. Split the guard so a null perm is still reported as a real caller mistake while a null group denies quietly. Keeping the check inside hasAccess preserves the op and namelayer.admin bypass, which runs before the null check, so privileged callers are unaffected. Add a regression test that captures the logger to prove the null-group path stays silent while the null-perm path still logs.
Refactor namelayer: fix bugs, prune, add tests
Bastion never listened to BlockFertilizeEvent, so bonemeal could grow moss (and anything else fertilizable) inside enemy bastion fields. Citadel's earlier fix only protected reinforced blocks, which left unreinforced field terrain convertible. Filter fertilized blocks against bastions the player lacks place permission on, mirroring the block-place check, and cancel dispenser-fired fertilizing that enters a foreign field like other playerless block events.
Block bonemeal use inside bastion fields
In auto mode the repair recipe ran on every redstone pulse, even at full health, so a factory with a repair kit in its chest burned through kits fixing nothing. Auto mode now only repairs when the factory is fully broken. The rest of the time it runs production, and after a repair it returns to the recipe the player selected instead of the first one in the list. That preference is remembered across restarts. Recipe selection moved into a small pure method covered by unit tests for the broken, healthy, and out-of-materials cases.
Owner
Author
|
Superseded by EdenMinecraft#36; targeting Eden instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A factory in auto mode kept running its repair recipe on every redstone pulse, even at full health. With a repair kit in the chest, it burned through kits to fix nothing. This came up in-game on a compactor running on a monthly clock.
Auto mode now only repairs when the factory is fully broken. The rest of the time it runs production, and after a repair it returns to the recipe the player selected rather than the first one in the recipe list. That preference is persisted, so it survives a restart.
The recipe-selection decision moved into a pure method,
chooseAutoRecipe, with unit tests covering the broken, healthy, and out-of-materials cases.Manual repair behaves as before, and a fully broken factory still auto-repairs back to full.