Dev to master aug18 - #67
Merged
Merged
Conversation
…esTwo into masterToDevAug14
A master to dev down-merge carries files that ship, so the gate demands a changeset for them. Adding one would be wrong: the next promotion of dev consumes it and bumps the version a second time for work already released. Skip the requirement when a pull request into dev carries the downmerge label. The label is ignored on any other base, so a release still cannot reach master without a changeset. The gate now also runs on labeled and unlabeled, so applying it re-runs the check.
…-bypass ci: let a labelled down-merge skip the changeset gate
… nodes anUnknownNodeIsIgnored and anUnparseableValueLeavesTheDefaultAlone previously asserted only conditions that held before apply() ran, so they passed even against a no-op body. Both now also capture the logger and assert the specific warning apply() emits.
…leak Adds coverage for the ensureColumn early-return path, which every real server restart takes after the first upgrade, and closes the Statement that pragma table_info opens in ensureColumn.
Refresh now claims a name exclusively by clearing it off any other UUID's rows before pointing this player's rows at it. uuidForName returns null instead of guessing when a name still resolves to more than one distinct UUID.
HomesDao.getAll inlined the same lowercase-compare rule as ServerUtil.isWorldBlacklisted, risking silent divergence between the two enforcement paths. Add a two-argument overload that takes an already-fetched blacklist, have the single-argument form delegate to it, and have HomesDao.getAll call the overload instead of inlining.
Consolidate add-to-blacklist, remove-from-blacklist, and
get-blacklisted-dimensions into a single blacklist command with the old
names kept as aliases. The three permission nodes stay declared and are
enforced per subcommand inside the executor, but the command-level
plugin.yml permission has to OR across all three (Bukkit's own
Command.testPermission supports this via a semicolon-separated list) so a
holder of any single node can still reach the executor - a single node
there would silently block the other two actions before onCommand ever
runs.
The executor infers the subcommand from the label Bukkit actually invoked
it with, not just args[0], so the old bare form ("/add-to-blacklist
world_nether", with no subcommand) keeps working exactly as it did before.
Extract HomeActionsGui's inline move-home mutation into a public applyMove/MoveOutcome pair, mirroring the applyRename/RenameOutcome pattern already used for the anvil rename prompt. The GUI keeps its own navigation and messaging; the new /move-home command (alias /uhome) reuses the same config keys and enum defaults so a customised message shows identically from both paths.
The move rewrites world and dimension, but nothing asserted it. Deleting both writes left the whole suite green, so a cross-dimension move could have silently kept the old world.
/go-home already ignored case but /move-home and /delete-home did not, so /go-home Base found a home named base and the other two reported it missing. Creating a home already rejects a name that differs only by case, so at most one home can match.
Three operator commands for working on another player's homes, with the v1 aliases home-of, delhome-of and uhome-of. Each resolves the target through ServerUtil.getPlayerUUID, so an offline player who has saved homes can be addressed by name. Renames UserError.PLAYER_NOT_ONLINE to PLAYER_NOT_FOUND and rewords it, because the name now only fails to resolve when it matches nobody at all. It is overridable as the playerNotFound config key.
/delete-player-home steve base failed where Steve worked, which sat badly next to home names now being matched without regard to case. Minecraft names are unique ignoring case, so at most one player can match.
The comment on GoPlayerHome claimed the admin dao keeps canTeleport set. It does not: only getAll applies the blacklist rule, so the flag stays at its default whichever dao is used, and the admin reached the home by accident rather than by design. Also cover the two commands that had no permission test, the not-found path on all three, and echo the stored owner name rather than what was typed, so a reply cannot say sTeVe's home 'base'.
The isItem check was untested, and losing it stored a non-item material such as WATER as a home icon, which made HomesGui throw when it built the ItemStack and stopped the homes menu opening at all. defaultHomeItem was documented as the icon used when a home is created without one but only the importer read it, so a server that set it saw it apply to imported homes and not to new ones.
A bare /create-home creates a home called default and a bare /go-home teleports to it, restoring the v1 unnamed home. The name matches what SetHomesV1Importer gives an imported v1 unnamed home. Also fixes the unknown-home error, which was sent unformatted and so showed players the literal %s placeholder. It now names the home that was looked for and takes a homeDoesNotExist config override.
The GUI rename applied HomeNameValidator but create-home never did, so a double space, which Bukkit turns into an empty first argument, saved a home with an empty name. No command can address such a home, so no command can delete it either, and on a server that denies sh2.manage-homes it was unreachable in game and kept occupying a max-homes slot. maxHomeNameLength was enforced on rename and ignored on create for the same reason.
…port delay Version 1 had a single homes.config_bypass node covering all three. Splitting it lets an admin grant a delay skip without also granting blacklist bypass. sh2.bypass-blacklist is enforced on create, on move, and on the reads that decide whether a home is teleportable, which is passed to HomesDao by its callers rather than threading a Player into the DAO.
Four commands reported 'That home no longer exists', which is the GUI's message for a home that vanished while the menu was open. Typed at a prompt after a typo it reads as though the plugin deleted the home, and it echoed nothing back. They now use the same message /go-home does, which names the home that was looked for. The two GUI sites keep the original wording, where it is accurate.
README now covers /move-home and the three -of admin commands, the consolidated /blacklist, the bypass nodes and the two permission bundles, the config permissions block, a Set Homes v1 migration table for both commands and permissions, and the behaviour changes an existing v2 server will notice on upgrade. default-config.yml gains playerNotFound, playerHomeDeleted and playerHomeMoved, which the code already read but the file never shipped. Marks invalidHomeItem vestigial and documents the placeholder hazards. Rewords CREATE_HOME_USAGE, which carried the word "default" twice with two different meanings on the one line a player sees when they have no homes.
Editing an already merged pull request's title or body could pull an issue backwards out of Ready for release, since the pull request job had no allowed-current guard on its non-closed path. Add one. Also skip the job entirely for fork-originated pull requests, since GitHub withholds secrets from those runs and the first API call would fail for a reason the contributor cannot fix. Scope GITHUB_TOKEN permissions down to none, since this workflow only ever uses the PAT in GH_TOKEN. And track failures across the issue loop instead of letting only the last invocation decide the step's exit code.
Down-merges from master to dev can repeat a Closes reference for an issue that already shipped; the merged-into-dev path now allows every status except Done, so a merge still beats an earlier state without dragging a shipped issue backwards. An empty PREV_TAG with tags already present in the repository now fails fast instead of silently scanning all of history and closing every issue any pull request ever referenced; a genuine first release with no tags at all is unaffected. A branch deletion after its pull request merges can fire a push event with no branch left to check out; the push job now skips deleted refs.
…automation Automate issue status through a project board
The push job chose its target from the branch name alone, so merging the base branch into a branch that already had an open pull request reported the issue as in progress. The Todo and unset guard could not help: it stops an issue being pulled out of In review, but an issue that never reached In review is not protected by it. The job now asks whether the branch has an open pull request and aims for In review when it does, advancing from In progress as well as Todo and unset. Neither plan names Ready for release or Done, so a push still cannot disturb a shipped issue. The decision lives in push_plan so it is covered by the suite.
… into issue-57-import-never-writes-to-v1
…om/Blockframe-Studios/SetHomesTwo into issue-57-import-never-writes-to-v1
…writes-to-v1 test: assert the v1 import never writes to plugins/SetHomes/
The steps told admins to import first and move the old jar afterwards, which the v1 jar refusal now makes impossible rather than merely unwise. Reordered into six steps with the swap before the import, and folded the old callout into a paragraph that explains why instead of repeating the instruction. Adds the rollback path and its one-way limitation, says a pasted config setting does nothing until the server restarts, and notes that a v1 server without a permissions plugin was never enforcing max-homes in the first place. Also corrects the EssentialsX FAQ, which still said the command clash goes to whichever plugin loads last and told readers to import before removing it.
…ps-readme docs: correct the migration order, and add the rollback path
Point the release at BukkitDev project 312833, the listing v1 servers already watch, so v2 arrives as the update on the page they know rather than a download they have to go and find. The published artifact and the GitHub Release become SetHomes.V<x>.jar and "Set Homes V<x>". The script comment named the two project ids the wrong way round; that is corrected. Also refuse to publish anything below 2.0.0. v1 last shipped 1.3.1 and the pom is at 1.2.2, so until the major bump is made by hand at promotion a push to master would offer those servers a lower version than they are running. The guard turns that into a failed release step instead of a public downgrade, and sits beside the existing game-version sanity check. plugin.yml's name, the data folder and the Maven artifact are unchanged.
…t-homes-listing ci: publish to the Set Homes listing as SetHomes.V<x>.jar
The migration rehearsal surfaced British spellings in text server owners actually read: the import report, two teleport messages, the config file comments and the README. House style is American English, so they now read capitalization, color and canceled. Also adds a missing word to the block printed when a Set Homes v1 jar is found alongside this one. It read "Nothing ever writes to it, but is needed for migrating homes to v2". The generated changelog entries keep the spelling they were published with, since they are a record of past releases rather than living text. Bukkit's own isCancelled spelling is untouched, and comments describing event cancellation keep it to match the API.
The upgrade rehearsal showed an operator teleporting with no countdown on a server whose config sets a delay, with nothing in the release notes to explain it. sh2.admin defaults to op and grants all three bypass nodes as children. This is v1 parity rather than a new rule: v1's homes.config_bypass was a child of homes.*, which also defaulted to op, and it skipped the teleport delay in GoHome and the blacklist and home limit in SetHome and UpdateHome. So a server migrating from v1 sees no change. A server already on 1.2.2 does, because 1.2.2 had no bypass nodes at all, and that is the case this sentence covers.
docs: say that operators hold the bypass permissions by default
…-user-text chore: use American spelling in user-facing text
update pom to decrease generated jar file size
… into devToMasterAug18
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
1 participant
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.
No description provided.