This bug is an interaction between ChatTabs and ChatModel.

The reason for this bug is that only messages from ChatMessageReceivedEvent are injected into chat tabs, messages from ChatPacketReceivedEvent are not. The reason for is that I thought we handle every case with either ChatMessageReceivedEvent or a NpcDialogEvent event in ChatModel. We do not, but I really think we should. In the future I would prefer to NOT use ChatPacketReceivedEvent anywhere except ChatModel (this is basically the case now, the event is only used in ChatTabFeature to be cancelled and ActionBarModel which needs to use it because action bar data is sent with the packet), process the message and post an appropriate event EVERY case.

What we really need to do is either make ChatMessageReceivedEvent post on NPC dialogue, if it is not handled with dialogue extraction. Alternatively, we can introduce a new event, although I don't think I would really like it.

This is something I would need to consult @magicus with. How can/should we (miss)use ChatMessageReceivedEvent to fit NPC dialogue messages if extraction is disabled? Should we not?

Related to this issue, I have no idea why the assumption seems to be made that the only multiline messages are dialogues. I think we should try processing multi-line messages every time, and handle whatever happens with dialogues in the method.

        if (dialogExtractionDependents.stream().anyMatch(Feature::isEnabled)) {
            handleMultilineMessage(message);
            e.setCanceled(true);
        }

The problem we have is really that Wynncraft (probably) has an internal chat system, where like each message is sent once, and everything is nice and dandy. But then they have a "vanilla tweak layer" to be able to have like NPC messages stay on screen. And when they activate that mode, all chat messages in the chat history is resent, over and over.

We basically want to cancel that layer, so we once again get just one new event per actual new message. This is the bulk of the work done by ChatManager.

Problem is, it is bugged. Chat history is sometimes parsed multiple times again (if you remember the bug with soul point messages reappearing when progressing in dialogue). Then there are the non-progress-able NPC dialogues that are still not parsed.

I think your reasoning is sound. If I understand you correctly, you are basically saying that all chat messages should be "funneled" through the ChatManager, which should "re-exported" as our "cooked" messages, rather than the "raw" chat from ChatPacketReceivedEvent.

I think your reasoning is sound. If I understand you correctly, you are basically saying that all chat messages should be "funneled" through the ChatManager, which should "re-exported" as our "cooked" messages, rather than the "raw" chat from ChatPacketReceivedEvent.

Yes, the question is whether we should ChatMessageReceivedEvent for NPC dialogues or not. Maybe this is just an implementation detail..

The problem we have is really that Wynncraft (probably) has an internal chat system, where like each message is sent once, and everything is nice and dandy. But then they have a "vanilla tweak layer" to be able to have like NPC messages stay on screen. And when they activate that mode, all chat messages in the chat history is resent, over and over.

We basically want to cancel that layer, so we once again get just one new event per actual new message. This is the bulk of the work done by ChatManager.

But if we let the ChatManager "strip off" the vanilla tweak layer, the user will de facto see a regression, where NPC messages do not stay on screen. So we need to counteract that with the NPC overlay. But what should we do if the user disables the NPC overlay?

Although I hope Christmas time will be great enough for you that you can spend a few hours on coding Artemis. I really need you to fix some bugs in ChatModel and most fixes require architectural changes to it. We've kind of discussed this, if you remember (you even have an open an PR for it iirc).

I'm looking forward toward spending more time with Artemis, too. :-)

So what you are saying is that not only are we missing functionality to fully remove the "tweak layer" from Wynncraft, the current implementation is buggy as well?

(This can be slightly related; the code in Chat Manager needs to be more complicated to account for the fact that sometimes we should not intercept messages. If we accept that we will always process messages, then the logic will be simpler, and hopefully easier to get bug-free)

So what you are saying is that not only are we missing functionality to fully remove the "tweak layer" from Wynncraft, the current implementation is buggy as well?

(This can be slightly related; the code in Chat Manager needs to be more complicated to account for the fact that sometimes we should not intercept messages. If we accept that we will always process messages, then the logic will be simpler, and hopefully easier to get bug-free)

Yes. #261

I think I started addressing this, but it was a long time. :-( I found this half-finished branch on my personal fork, but I don't even know if it was this that was supposed to help:
https://github.com/magicus/Artemis/tree/non-confirmation-npc

I think I started addressing this, but it was a long time. :-( I found this half-finished branch on my personal fork, but I don't even know if it was this that was supposed to help:

https://github.com/magicus/Artemis/tree/non-confirmation-npc

Different issue in that branch. Once you are back I can hook you up with a list of changes.