feat: announce v1 homes waiting to be imported - #59
Merged
Conversation
An admin who upgrades from Set Homes v1 and reads nothing sees an empty homes list on first boot, while plugins/SetHomes/homes.yml sits there full of homes with nothing saying so. Auto-import on enable was the original proposal and was rejected, so this replaces it. onEnable logs a warning block naming the file, how many homes are waiting and the command to run. Anyone holding sh2.import-homes gets the same reminder in chat on join, because plenty of admins never read the console. The condition is our own database being empty, re-read every time, so both stop for good once any home exists and no marker file is needed.
6 tasks
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
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.
Closes #54. Second of the five sub-issues split out of #41, and it sits on top of #53, which owns the top of
onEnable.What changed
An admin who upgrades from Set Homes v1 and reads nothing sees an empty homes list on first boot, while
plugins/SetHomes/homes.ymlsits there full of homes with nothing saying so. The first anyone hears of it is a player asking where their homes went.Now, when v1 homes are present and our own database is empty:
onEnablelogs aWARNINGblock naming the file, how many homes are waiting, and the command to run.sh2.import-homesgets the same reminder in chat when they join, because plenty of admins never read the console.Both stop for good the moment any home exists here. The condition is re-read every time, so there is no marker file and nothing to switch off after the import.
Auto-import on enable was the original proposal on #41 and was rejected there: the config half of the migration cannot be automated anyway, a silent import makes a dropped home impossible to surface, and plugin-managed worlds are not loaded at our
onEnable. This is the replacement for what auto-import was really buying, which is that nobody discovers the problem from player complaints.Notes for review
PendingV1Importlives inimporters/so the knowledge of v1's file layout stays next toSetHomesV1Importerrather than being duplicated.waitingToBeImported()checks the database before touching disk, so a server that has already imported never reads v1's file, and the join path checks the permission before either, so an ordinary player's join costs nothing.v1ImportPendingis documented in the migration section of the README rather than added to the## Configurationtable, which is explicitly the settings most servers change. A migration-only message override would misrepresent that table.support/PluginBootis new, extracted because this is the second test class that has to arrange state beforeonEnable.SetHomesV1ClashTestkeeps its own copy rather than widening this diff into merged code.Verification
MockBukkit: 377 before, 385 after,
Failures: 0, Errors: 0, Skipped: 0.Three tests were watched failing before the code existed: the startup block, the
unknownHomescount, and the join notice. The other five passed on first run, so the guards were broken four ways to prove them, each restored byte-identical:PlayerJoinaJoiningPlayerWithoutTheImportPermissionIsNotToldcountAll() > 0early return removedtheNoticeStopsOnceAHomeExistsHerewaiting == 0early return weakenednothingIsSaid...teststheNoticeRepeatsOnEveryJoinOne test needed a harness fix, diagnosed rather than guessed: MockBukkit drops a
PermissionAttachmentondisconnect(), so a rejoining player silently held no permissions. Production keeps no per-player state, so the test re-grants before the second join.Verified on the local Paper 1.21.4 server against the real v1 data, both directions:
homes.ymlpresent and our database empty:Set Homes found 12 home(s) in plugins/SetHomes/homes.ymlat startup, and the chat line on join, seen again after a reconnect.import-homes sethomes confirmfrom the console: 11 imported, 1 skipped for the missingcreativeworld.12 is the same total the migration rehearsal recorded for that file, reached here by a different code path.