Repository navigation
chore: remove the announcement system - #832
Merged
Merged
Conversation
The plugin fetched a message from glaremasters.me on startup and again for each operator's first join, hovering over a chat line. The backend is no longer maintained, so this is a remote call that can only ever fail, and it fails silently: getAnnouncements catches every exception and returns "Could not fetch announcements!". Removes the fetch itself, the two config keys that gated it, and convert_html, which existed only to translate the response and had already been reduced to a single replace call in #808 once that left one caller. The war announcer is untouched. announceWinner and announceDeath broadcast to the guilds in a war and are unrelated to the backend. Leftover for a follow-up: PremiumFun is now uncalled. It reads SpigotMC's premium-download placeholders and existed only for the announcement URL, but it is the premium-licensing shim rather than part of this feature, so it is left alone here.
Removing the properties from PluginSettings does not remove them from a server owner's existing config.yml. Verified rather than assumed: with the keys gone, a config still carrying settings.announcements.console is read back fine but triggers no migration, so the block would sit there unread indefinitely. A server owner who had set in-game to false would have no way to tell that it no longer does anything. Adds settings.announcements to the deprecated list in GuildsMigrationService, which is what that list is for: its own comment says only paths that no longer exist in any SettingsHolder belong there. ConfigMe rewrites the file when a migration is detected, so the block is dropped once and never again. The rewrite strips admin comments and custom keys, which is the cost of cleaning this up and the reason it happens exactly once. The suite already pins both halves of that: a freshly generated default config must not trigger a migration, and an edited one must not either.
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.
Summary
Removes the announcement system: the plugin fetched a message from
glaremasters.meon startup and again the first time each operator joined,shown as a chat line with a hover. The backend is no longer maintained, so
this is now a remote call that can only fail, and it fails quietly.
Four things go, in dependency order:
Guilds.onEnablePlayerListener.onJoin, plus theinformedset that tracked who had already seen itsettings.announcements.consoleandsettings.announcements.in-game, and the comment above themStringUtils.getAnnouncementsandconvert_htmlconvert_htmlhad already been reduced to a singlereplace('&', '§')in#808, once that left it a single caller. That caller was this.
Not the war announcer
announceWinnerandannounceDeathinChallengeHandlerare unrelated anduntouched. Those broadcast to the guilds in a war; they never touched the
network. The names collide, which is why they are called out here rather than
left for a reviewer to work out from the diff.
Config migration
The second commit. Removing a property from a
SettingsHolderdoes notremove it from a server owner's existing
config.yml, and I checked ratherthan assuming:
So without help, the block would sit there unread forever. A server owner who
had deliberately set
in-game: falsewould have no way to tell that thesetting no longer does anything.
GuildsMigrationServicealready has a list for exactly this, and its owncomment states the rule: only paths that no longer exist in any SettingsHolder
belong here.
settings.announcementsnow qualifies, so it is added.String[] deprecatedProperties = { ... + "settings.announcements",ConfigMe rewrites the file when a migration is detected, which drops the block
once and never again. That rewrite also strips admin comments and custom keys,
which is the cost of cleaning up and the reason it must happen exactly once.
The suite already pins both halves of that, and both still pass:
The third of those is the important one: a fresh install must not migrate, or
every startup would rewrite the file.
Evidence
Nothing in
src/mainreferences the feature any more:$ git grep -c -i "getAnnouncements\|ANNOUNCEMENTS_\|convert_html" -- src/main (no matches)Exactly the four classes that changed appear in the jar, and no entry is added
or removed:
Suites pass, including the two that guard the supported-version floor:
$ ./gradlew check testJava11 legacyApiProbe --rerun-tasks BUILD SUCCESSFUL in 9sThe e2e fixture had
console: falseandin-game: falseset, with a commentexplaining they kept bot chat buffers free of plugin chatter. Those keys no
longer exist, so the block is removed. The tests themselves never asserted on
announcements, only on the absence of other plugin chatter.
Server owner impact
settings.announcementsdisappears fromconfig.ymlon the next boot, once.That rewrite also strips comments and custom keys from the file, which is
worth knowing if you keep notes in there.
Leftover, deliberately
PremiumFunis now uncalled. It reads SpigotMC's premium-download placeholders(
%%__USER__%%and friends) and existed only to build the announcement URL, soremoving the announcements orphaned it. I have left it in place because it is
the premium-licensing shim rather than part of this feature, and deleting it
belongs in its own change. Nothing references it, so it is inert.
Merge Danger
Door: two-way. Two commits, no additions, so revert restores every file
exactly.
Blast Radius: startup output, one operator chat message, and one config
rewrite for existing installs.
a notification channel to a backend that is no longer running.
migration mechanism doing what it is designed to do, it fires once, and it
is the same cost every previously deprecated path already imposed.