Skip to content

Improvement: Split chat filter "Others" toggle into individual filters - #6554

Open
badblobb wants to merge 5 commits into
hannibal002:betafrom
badblobb:split-chat-filter-others
Open

badblobb wants to merge 5 commits into
hannibal002:betafrom
badblobb:split-chat-filter-others

Conversation

@badblobb

@badblobb badblobb commented Sep 16, 2026

Copy link
Copy Markdown

What

Split the others Chat Filter Types toggle into 12 toggles, made sure to match the existing pattern used by every other filter in FilterTypesConfig.kt.
others had a // TODO remove and bundled a bunch of unrelated chat messages together in one big toggle without allowing choice between them, lowkey bugged me personally so I decided to fix it myself.
A few of the splits were a bit oudated, so I made some edits:

  • The old "Useless Notification" bucket got split into three separate toggles: Useless Notifications, Mining Abilities, Deployables. Also removed ability messages hiding "ability ready again" since that's legit useful to players.
  • Mining Abilities now covers used/expired messages for Mining Speed Boost, Maniac Miner, Tunnel Vision, Gemstone Infusion, Sheer Force, and Pickobulus.
  • Deployables now uses a colorless and repo pattern backed match on "was removed" rather than an enumerated name list, this should cover all power orbs, mining lanterns, flares, black holes, umberella and totem of corruption now.
  • Replaced the old "Annoying Spam" catch-all with Ability Damage, Blocked Actions, and System/Dev Noise.
  • Entirely dropped hiding the "GOOD CATCH!" fishing treasure messages since as of recently this is actual relevant gameplay info.
  • Cleaned up old, obsolete code like the wardrobe in combat warning, blessing enchant doubling drops, the ad for the 12th anniversary Youtube video, and a feast chef ted line that was already covered by an earlier toggle.

Mining Abilities and Deployables default to off and don't inherit from the old others value on migration because this is potentially useful information and were only incidentally bundled into others.

This PR only handles the toggle-split half of #195, not the second part (a ConfigEditorDraggableList for picking individual messages within each filter). I couldn't find a clean existing precedent for a raw string message list and some of these groups are regex-based rather than fixed strings, so "every message" didn't work with existing config patterns.

Images Before: image

After:
image
image
image

Changelog Improvements

  • Split the "Others" Chat Filter option into 15 individual filters, so each type of message can be hidden independently instead of all of them being toggled at once. - badblobb
    • Existing users who had "Others" enabled will automatically have 13 of the 15 toggles enabled, with the exception of Mining Abilities and Deployables toggles as they may contain useful information.

The "others" toggle in Chat Filter Types bundled 10 unrelated message
groups (Bazaar/AH/Bank progress spam, Slayer, useless drops/notifications/
warnings, party separator, Winter Island, annoying spam) behind a single
switch, so users couldn't disable one without disabling all. Splits it
into 10 dedicated toggles matching the existing pattern used by every
other filter in FilterTypesConfig.kt, with a config migration so users
who had "others" enabled get all 10 new toggles enabled.
@badblobb

Copy link
Copy Markdown
Author

Actually, I'm noticing now that under the existing "Useless Messages", the mining abilities only included "Mining Speed Boost" and deployables only included Plasmaflux. Splitting these into "Mining Abilities" and "Deployables" toggles (Deployable Expiration was a bit long and description fits well). The rest I chose to stay with "Useless Notifications" despite it possibly reading as a "Useless Option" joke entry.

I'm going to expand on Mining Abilities to include all abilities as of 16 Sep 2026 and also add a Foraging Abilities toggle with the new HoTF abilities.
Deployables will now include all deployables, including black holes, dwarven lanterns, flares, totem of corruption, and umberella. I didn't include the miniature nuke because lmfao???

ALSO these three options (Mining Ability, Foraging Ability, Deployables) should not be enabled by default anymore since they could potentially be useful and were lumped into the old "Other" category regardless, will update PR once I figure out all the regex matching

@badblobb

Copy link
Copy Markdown
Author

Reading now that the old code would just not work if you had a skin applied, Maybe tracking "was removed." might just be easier and cleaner

@lunaynx

lunaynx commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

This is a good idea, but I really want to refactor this to all be colorless and backed by repo patterns first. I suppose your current changes aren't too difficult to reconcile, but I'd prefer if you'd hold off on changing the actual filter patterns until after that is done.

@badblobb

Copy link
Copy Markdown
Author

I've made some pretty good progress worth keeping, but I'll hold off on adding anything else for now. Stuff like deployables I used a "§eYour previous §r.*§r§ewas removed!" pattern for now. Fixing things up and capturing some images before finishing this PR ^^

@badblobb
badblobb marked this pull request as ready for review September 16, 2026 16:08
@Expose
@ConfigOption(
name = "Annoying Spam",
desc = "Hide miscellaneous spammy messages, e.g. ability damage numbers, fishing catches, dev warnings.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want to split this up too?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good call. Making the changes now. This whole thing might need a proper refactor soon since scrolling through 30 toggles lowkey sucks

@hannibal002
hannibal002 self-requested a review September 17, 2026 06:21
@badblobb

Copy link
Copy Markdown
Author

As per @zumbiepig's suggestion, I'm going to edit this to split "Annoying Spam" up a bit more:

Ability Damage - Hide ability damage notifications for abilities like Wither Impact, Guided Bat, etc.
Blocked Actions - Hide messages when an action is blocked, e.g. Instant Transmission having blocks in the way, insufficient Breaking Power.
NPC Announcements - Hide NPC announcement messages, e.g. Jacob's contest starting, Booster cookie required for action.
System/dev noise - Hide internal system and debug messages that sometimes leak into the chat.

I'm also cleaning things up a bit by removing these:

  • "Good" treasure catches: new updates have made this relevant to gameplay now
  • "Your Blessing enchant got you double drops!": it works differently now
  • 12th Anniversary Youtube premier message (lol???)
  • "You can't use the wardrobe in combat!": This doesn't exist anymore

The old damage notification code hard matched stuff like implosion and spirit sceptre directly, I've cleaned it up and used the new colorless regex matching §7Your .* hit (.*) for §r§c(.*) §r§7damage. and wildcards to generalise it to the new standard.

…C Announcements, and System/Dev Noise. Removed "Good" treasure catch notifications and other various obselete filters.
@github-actions github-actions Bot added the Merge Conflicts There are open merge conflicts with the beta branch. label Sep 17, 2026
@github-actions

github-actions Bot commented Sep 17, 2026

Copy link
Copy Markdown

Merge conflicts detected

Show previous conflicts

⚠️ Merge conflicts detected ⚠️

This pull request has conflicts with the base branch. Please resolve them before this PR can be merged.

…thers

# Conflicts:
#	src/main/java/at/hannibal2/skyhanni/features/chat/ChatFilter.kt
@github-actions github-actions Bot removed the Merge Conflicts There are open merge conflicts with the beta branch. label Sep 17, 2026
@zumbiepig

Copy link
Copy Markdown
Contributor

Yeah, good call. Making the changes now. This whole thing might need a proper refactor soon since scrolling through 30 toggles lowkey sucks

Make the whole thing a multiselect menu

@badblobb

Copy link
Copy Markdown
Author

Yeah, good call. Making the changes now. This whole thing might need a proper refactor soon since scrolling through 30 toggles lowkey sucks

Make the whole thing a multiselect menu

I disagree for now, UX wise, the rest of the options are not a multi select menu too and there's too many competing detection types, the whole thing would need to be refactored. Not to mention people accessing this menu are usually doing it through search functionality and these toggles benefit from a longer description. Happy to get other feedback on this, but as it stands IMO the best thing we can do is to chuck them into well organised groups for people to open and peruse

@hannibal002

Copy link
Copy Markdown
Owner

Make the whole thing a multiselect menu

whats the benefit of a multiselect?
imo its fine in the current apporach

@zumbiepig

zumbiepig commented Sep 17, 2026 via email

Copy link
Copy Markdown
Contributor

Comment thread src/main/java/at/hannibal2/skyhanni/features/chat/ChatFilter.kt Outdated
Comment thread src/main/java/at/hannibal2/skyhanni/features/chat/ChatFilter.kt Outdated
@lunaynx

lunaynx commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Confirmed the rough plan with Hannibal on Discord:

  1. This PR: split up existing "others" / "annoying spam" filters, and possibly minor fixes/extensions to existing filters, but preferably no major additions
  2. My PR: full colorless + RepoPattern refactor
  3. If needed, other PRs adding further filters

@badblobb

Copy link
Copy Markdown
Author

Made some changes this morning:

  • Fixed the regex tests i drafted originally without checking ingame, now it matches expected game behaviour (sorry!!! I'll remember this for future PRs!!)
  • Ability damage pattern is now colourless, it's just a special case for now I've done ahead of your refactor, should be okay.
  • The capturing groups were dropped from ability damage as suggested.
  • Removed it from the generic isPresent() mapping system since the matching logic was different now that we're using clean message.
  • I actually tightened .* to .+ on two other patterns (mining ability used / deployable replaced) since those fields can never be empty either, just to clean things up a bit.

Thanks for the feedback!

@badblobb
badblobb requested a review from lunaynx September 18, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants