Fix for Issue #72

[Glyceri]

Hello there 👋

Issue #72 has been marked as closed, but someone mentioned that Pet Nicknames breaks Tidy Chat in the exact same way as Issue #72 does.

Relevant here is the Dalamud Sourcecode.

Personally, I have no clue why this happens, as the ref keyword is used and that exact value gets send to the next plugin afterwards. But only the last plugin that sets isHandled for a chat messages gets taken into account causing heaps of issues. However, Dalamud has a seperate; earlier; call that is specifically used for isHandled.

My fix is to move Tidy Chat from ChatMessage to CheckMessageHandled.

(The real fix would be to fix this issue in Dalamud, but I'm not touching that)

[NadyaNayme]

I appreciate you digging into the issue and finding the root cause of the problem. I haven't touched C# in a while - but wouldn't this then have the same issue with any plugins that use CheckMessageHandled (if there even are any)?

Personally I'd rather not kick the can down the road so to speak or potentially have more/different incompatibility issues with other plugins if ChatMessageHandled is being used by other plugins.

[Glyceri]

That is fair and I agree. The real fix would be to fix it in Dalamud. However, most plugins dont interact with ChatMessageHandled which is why I proposed this as a temporary fix (Because Tidy Chat completely breaks in combination with my plugin and others that use isHandled in ChatMessage) until I, or someone else, finds time to fix the bug in Dalamud itself.

Still I feel this PR is somewhat appropriate, because after the bug in Dalamud itself is resolved it would still be proper etiquette to hook into the new ChatMessageHandled compared to ChatMessage due to Tidy Chats reliance on isHandled. But I also feel like a real nitpick for even saying that, so I will leave it here.

[NadyaNayme]

No - I think that's actually a fair argument and not really a nit. Consider me convinced. 👍

[Jaksuhn]

This change broke compatibility with Gatherbuddy because CheckMessageHandled does not seem to propagate messages in the same way that ChatMessage does, thus when gathering messages are filtered it prevents Gatherbuddy's systems from working as intended (fish timers, as an example)

[Glyceri]

That is exactly how the dalamud devs intended it, see here. ̷A̷s̷ ̷I̷ ̷s̷e̷e̷ ̷a̷ ̷m̷i̷l̷l̷i̷o̷n̷ ̷p̷l̷u̷g̷i̷n̷s̷ ̷b̷r̷e̷a̷k̷i̷n̷g̷ ̷d̷u̷e̷ ̷t̷o̷ ̷i̷s̷H̷a̷n̷d̷l̷e̷d̷ ̷b̷e̷i̷n̷g̷ ̷s̷e̷t̷;̷ ̷m̷o̷s̷t̷l̷y̷ ̷T̷i̷d̷y̷C̷h̷a̷t̷,̷ ̷b̷u̷t̷ ̷n̷o̷t̷ ̷l̷i̷m̷i̷t̷e̷d̷ ̷t̷o̷ ̷T̷i̷d̷y̷C̷h̷a̷t̷;̷ ̷I̷ ̷w̷i̷l̷l̷ ̷b̷r̷i̷n̷g̷ ̷i̷t̷ ̷u̷p̷ ̷w̷i̷t̷h̷ ̷t̷h̷e̷ ̷D̷a̷l̷a̷m̷u̷d̷ ̷t̷e̷a̷m̷.̷ Edit: Brought it up with the Dalamud team, who were very speedy in responding. If a plugin relies on a message even if it supposed to be hidden they shouldn't hook into ChatMessage but instead into CheckMessageHandled which GatherBuddy does not do.

This change broke nothing, just brought to light stuff that was already broken (or working as intended depending on how you look at it).

[Ottermandias]

Shit didn't even exist when GatherBuddy was written.

[Glyceri]

Yeah, same for when Pet Nicknames and Tidy Chat were written. It came somewhere recently and I think a lot of plugins are now playing catch-up after this change had been made because of this plugins reliance on the isHandled flag.