PaperMC / Paper

The most widely used, high performance Minecraft server that aims to fix gameplay and mechanics inconsistencies
https://papermc.io/
Other
9.98k stars 2.32k forks source link

Plugin chat prompts are no longer being cancelled from being visible in chat #8911

Closed Sikatsu closed 1 year ago

Sikatsu commented 1 year ago

Expected behavior

When a plugin asks me to type a value within the chat, the message should be cancelled from being visible to everyone.

Observed/Actual behavior

The messages are no longer being cancelled and are visible to every player on the server.

Steps/models to reproduce

You can try this with many different plugins, as they are all broken in this regard:

The issue is there after updating from paper build 399 to 420. I have tested this on my survival, creative AND nearly empty test server with just HDB and the two different paper builds.

Plugin and Datapack List

Plugins (28): AntiPopup, ArmorStandEditor, ArtMap, CMI, CMILib, ChatControlRed, CommandPanels, CoreProtect, ExecuteEverywhere, HeadDatabase, LuckPerms, Multiverse-Core, MyCommand, PlaceholderAPI, PlayerParticles, PlotSquared, PremiumVanish, ProtocolLib, ServerUtils, ShowItem, SupremeTags, TAB-Bridge, VCLink2-Bukkit, Vault, VillagerCraftCore, WorldEdit, WorldGuard and eGlow

Paper version

[11:15:16 INFO]: This server is running Paper version git-Paper-420 (MC: 1.19.3) (Implementing API version 1.19.3-R0.1-SNAPSHOT) (Git: 81d7ff6) You are running the latest version

Other

image

electronicboy commented 1 year ago

Hard to say around premium/closed source plugins, given that the only real change has been in the plugin loader, this would generally scream more towards a plugin issue, primarily around their dependency on event order;

Sikatsu commented 1 year ago

I see. Wouldn't it be suspicious this happens to all plugins though? None of the ones I mentioned are open source?

For now i'll stay on the old version.

electronicboy commented 1 year ago

The only open source plugin is HyperDrive, which does appear to be handling it properly, so it not working would be suspicious, but, nothing has changed with the event that it uses in a long time, so that would make 0 sense, would need somebody to reproduce

Outside of that, we'd need somebody to reproduce this, or a minimal reproduction case, etc; Theres no strong leads here outside of the fact that we just recently changed a system which between plugin devs and spigot was screwed

Akiranya commented 1 year ago

Do you run the server behind a proxy? if so, did you update the proxy/make any changes on the proxy as well before this issue?

My guess is that if some plugins on the proxy were intercepting the messages sent from players and then broadcast it to the backend server, then the message would actually bypass any check done by any plugins installed on Paper.

Take a look at this piece of information: https://william278.net/docs/huskchat/backend-chat-entry

Hope that helps

Sikatsu commented 1 year ago

It indeed runs behind a waterfall proxy. Though nothing was changed or updated on that regard. All that I have actually changed is the paper version on the backend servers.

I'll take a look at the website you provided.

Sikatsu commented 1 year ago

Would installing HuskChat also solve my issue?

On a clean bungee installation with 2 backend servers, running waterfall and paper with vault, luckperms and hyperdrive I am able to reproduce this issue unless I rollback my paper version to an older build.

electronicboy commented 1 year ago

Would need to isolate what build broke it, as that makes zero sense to me

On Wed, 1 Mar 2023, 21:40 Sikatsu, @.***> wrote:

Would installing HuskChat also solve my issue?

On a clean bungee installation with 2 backend servers, running waterfall and paper with vault, luckperms and hyperdrive I am able to reproduce this issue unless I rollback my paper version to an older build.

— Reply to this email directly, view it on GitHub https://github.com/PaperMC/Paper/issues/8911#issuecomment-1450884305, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMAZBOJNRQVPEG4VFVOXDWZ665XANCNFSM6AAAAAAVGW4OTE . You are receiving this because you commented.Message ID: @.***>

Sikatsu commented 1 year ago

Not sure which build exactly, but between 399 and 420. I could try to pinpoint it exactly, but unnfortunately that has to be for monday as i'm out for a few days.

Sikatsu commented 1 year ago

@electronicboy I can confirm that build 404 still works, it breaks on build 405 for ALL plugins that work with a chat prompt.

With this commit: https://github.com/PaperMC/Paper/issues/8108

Omarlatif commented 1 year ago

I can confirm that on my server it also breaks on build 405.

MiniDigger commented 1 year ago

does somebody have code to repro? I am having a hard time trying to understand what you mean by prompts, is it the conversation api that is broken? or just plugins cancelling one of the chat events?

Sikatsu commented 1 year ago

For example in headdatabase there is a button in the GUI ''Search heads..'', when I click on it, a title comes up to enter a value within the chat. When I do that however, the value is visible to everyone on the server, rather then being cancelled.

Any plugin that makes use of chat prompts (enter a value in chat, like prices etc) is broken as of build 405.

Don't really have any code myself to reproduce, as I am not a developer unfortunately.

electronicboy commented 1 year ago

They're using chhat events, not the conversation API; this generally looks like a case of plugins conflicting over priorities and load order

On Thu, 9 Mar 2023, 12:39 MiniDigger | Martin, @.***> wrote:

does somebody have code to repro? I am having a hard time trying to understand what you mean by prompts, is it the conversation api that is broken? or just plugins cancelling one of the chat events?

— Reply to this email directly, view it on GitHub https://github.com/PaperMC/Paper/issues/8911#issuecomment-1461982633, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMAZBTO2ZEFEBQ4NDY3ATW3HFPDANCNFSM6AAAAAAVGW4OTE . You are receiving this because you were mentioned.Message ID: @.***>

NyhmsQuest commented 1 year ago

This is happening on my server too with all my plugins using chat prompts. Something changed or broke with that Paper update

Owen1212055 commented 1 year ago

Please try to provide simple reproduction steps, and if possible, maybe have two isolated dumbed down plugins, as generally, this issue can be a wide variety of things due to how much truly changed with Paper plugins.

This would allow us to do much better debugging and figure out the root cause of the issue here.

kangarko commented 1 year ago

Similar problem also happens with my plugin ChatControl Red,

https://github.com/kangarko/ChatControl-Red/issues/2306

Paper needs to be more clear on the changes and provide developers instructions of what is wrong with their plugins, and what they need to change specifically.

Let me know your BuiltByBit ID and I can grant you license to ChatControl Red if you need to debug it. Or let me know where to find the culprit and I am happy to collaborate on this.

Sikatsu commented 1 year ago

The reproduction steps are quite easy once you have a plugin that makes use of chat prompts. Unfortunately can't provide a simple plugin as that's not my expertise. I also make use of ChatControl Red, so testing with Kangarko's plugin as mentioned above might be an option?

Yertled commented 1 year ago

ChatSentry and ExecutableItems are 2 plugins that we notice this issue with the hardest. ExecutableItems have a free version: https://www.spigotmc.org/resources/custom-items-free-executable-items.77578/

Download the plugin and run it on the server. Use the broken Paper build (405), or in our case, latest PurpurMC build. Then create an item with Executable Items. Write in chat: /ei create TEST Then click on the "Edit Name" or "Edit Lore" It would allow you to type the name of the item, or something random for the lore in chat. Instead of cancelling what you type to chat, it publically broadcasts what I type, even tho it should be cancelled.

This has worked perfectly for months, until we updated our Purpur (which is Paper).

Brennian commented 1 year ago

From what I have heard, this doesn't work anymore.

    @EventHandler(priority = EventPriority.LOW)
    public void onChat(AsyncPlayerChatEvent e) {
        if (e.isCancelled()) {
            return;
        }
        if (enabledSearchNames.contains(e.getPlayer().getName())) {
            openSearchInventory(e.getMessage(), 1, e.getPlayer());
            enabledSearchNames.remove(e.getPlayer().getName());
            e.setCancelled(true);
        }
    }
Owen1212055 commented 1 year ago

Similar problem also happens with my plugin ChatControl Red,

https://github.com/kangarko/ChatControl-Red/issues/2306

Paper needs to be more clear on the changes and provide developers instructions of what is wrong with their plugins, and what they need to change specifically.

Let me know your BuiltByBit ID and I can grant you license to ChatControl Red if you need to debug it. Or let me know where to find the culprit and I am happy to collaborate on this.

Our guess is that this might have something to do with load order. Because recent changes have not touched the event system at all. But, we are not sure.

DaJokni commented 1 year ago

For example in headdatabase there is a button in the GUI ''Search heads..'', when I click on it, a title comes up to enter a value within the chat. When I do that however, the value is visible to everyone on the server, rather then being cancelled.

Any plugin that makes use of chat prompts (enter a value in chat, like prices etc) is broken as of build 405.

Don't really have any code myself to reproduce, as I am not a developer unfortunately.

do you mean the search function in hdb? just tested that on paper build 445 and it appears to be working correctly

Sikatsu commented 1 year ago

No, not the regular search function. That works fine. The one in the GUI which after you click on it, asks you to typ your input in chat. Could be that you have that option disabled. It was just an example of a use case though.

DaJokni commented 1 year ago

No, not the regular search function. That works fine. The one in the GUI which after you click on it, asks you to typ your input in chat. Could be that you have that option disabled. It was just an example of a use case though.

yes, that is exactly what i am using.

Sikatsu commented 1 year ago

It works, but it doesn't cancel the message for the whole server to see. I can reproduce my issue on a clean test server running paper build 445 as well..

emilyy-dev commented 1 year ago

Looking at the event logic, this makes me suspect it's a case of a plugin re-sending the chat event contents to players directly (via sendMessage or broadcast), rather than letting the event to be handled by the server naturally. Also the issue screenshot suggests the message was sent as system message which only makes my suspicion greater.

On a clean server, simply cancelling the chat event works fine.

I assume that is the difference between your server and Jokni's where he is testing hdb

DaJokni commented 1 year ago

It works, but it doesn't cancel the message for the whole server to see. I can reproduce my issue on a clean test server running paper build 445 as well..

it works, and the message isnt sent to the whole server. nothing seems out of the ordinary

kangarko commented 1 year ago

Oh by the way, ChatControl, one of the affected plugins, is sending chat messages as system messages (we use Spigot API for compatibility purposes). I assume this might be a contributing factor?

kangarko commented 1 year ago

@emilyy-dev can it be that we're using EventExecutor instead of Listener for the chat event specifically? https://github.com/kangarko/Foundation/blob/master/src/main/java/org/mineacademy/fo/event/SimpleListener.java

Sikatsu commented 1 year ago

What's the progress on this one? We can still not update beyond build 404, even if we want to update to 1.19.4?

kangarko commented 1 year ago

@Sikatsu can you try opening ChatControl.jar with WinRar or similar and editing its plugin.yml > try removing or adding your conflicting plugin from/to the soft-depend key list to see if any changes occur

Machine-Maker commented 1 year ago

We need an example of this issue using something that we can actually test and interact with and see what the plugin is doing. So far, all reproduction steps have involved a closed source plugin which means we can't easily see what interactions might be causing this. There are different ways plugins can implement such a system of responding to prompts and we don't know which one might be the issue.

kangarko commented 1 year ago

ChatControl-Red-10.17.4.zip

@Machine-Maker here is my premium plugin for debug purposes. I have also invited you to ChatControl repository. We compile the plugin on top of this library.

Thank you so much, let me know how I can help.

Machine-Maker commented 1 year ago

Ok so without any other plugins, the prompt functionality for the /mail command's forward feature does work. I was able to just type in my own player name and the forwarding worked and the message wasn't sent in chat. So that points to it being a conflict with another plugin (probably load order related).

EDIT: I tested CC-Red with the executable items plugin mentioned above, and those interactions seemed fine.

Sikatsu commented 1 year ago

@Sikatsu can you try opening ChatControl.jar with WinRar or similar and editing its plugin.yml > try removing or adding your conflicting plugin from/to the soft-depend key list to see if any changes occur

I could try that, however it happens with ALL plugins, even ones that are not in the soft-depended list..

HyperDrive, DisplayShops, HeadDatabase, AuctionHouse etc.

electronicboy commented 1 year ago

Nothing has changed at all with how events work, this is once again likely to be an order of execution issue, a quick plugin to print out the event handlers order might be an idea, but, I don't recall if enough useful info is retained in the executors to provide anything useful

sandwally commented 1 year ago

Those interactions works fine with chatcontrol, it's when you change the settings.yml for chatcontrol to enable channels as chat formatting it starts to pass through both the text to chat and it executes the chat prompt command.

kangarko commented 1 year ago

@Machine-Maker thank you for taking the time to test users' code.

I have made that quick plugin to print out registered listeners: PrintEvents.zip

With CMI, ChatControl and ItemsAdder this is the order in which they are loaded: 2023-03-24-11 24-208

I unfortunately have no idea from the data provided how to fix this and would appreciate any direction. @Sikatsu I assume you tried changing our Listener_Priority from LOWEST to HIGHEST in ChatControl, right?

Sikatsu commented 1 year ago

It's always been on HIGHEST for me.

kangarko commented 1 year ago

If if its on HIGHEST then by theory we should catch the message as last, thus allowing CMI to properly cancel the event since we ignore canceled events.

@electronicboy can you check if truly nothing has changed for legacy event registration which lets us specify ignoreCancelled flag as well as priority? This is what I am referring to that we are using -- if the plugin with a lower priority cancels the event then we should just not get the method fired under the latest Paper version, or I am mistaken? It worked like this previously.

a

Sikatsu commented 1 year ago

I'm not sure if CMI makes use of chat inputs personally, but it's definitely on highest and causing issues with plugins I mentioned in my original message.

Thanks for looking into it, waiting on this one before I can update bug-free to 1.19.4

kangarko commented 1 year ago

Does CMI allow you to set its priority? I assume setting it to anything lower than HIGHEST should properly let us ignore their canceled chat input.

Another possibility is that something is wrong with the modal conversations in the Conversation API in Paper, that would be out of my reach unfortunately.

Sikatsu commented 1 year ago

I do not think it does, however I disabled CMI's chat entirely. Not sure if that matters at all?

Sikatsu commented 1 year ago

Any update @kangarko?

kangarko commented 1 year ago

I don't know how to proceed with this. I provided the requested information to Paper dev. I gave the source code to @Machine-Maker. I know we are not the only plugin causing issues like this I truly believe something must have changed in Paper. I am open and happy to collaborate on fixing this.

Sikatsu commented 1 year ago

I'm afraid this issue is not being seen though and it's kind of holding me back to update to 1.19.4 as well, just because this bug will go live if I do so. Is there not a way to check the paper commits or try a different method for this? As without ChatControl, it does indeed seem to work..

electronicboy commented 1 year ago

I've already said what the issue probably is; nothing in paper has changed with the event system, cancelled is still literally just aa simple boolean; this is going to be a plugin issue down to the order that event handlers are executed, I'd imagine thhat if you use the flag to use the older dependency order calculations that it might just magically work again (ofc, not a solution)

On Wed, 29 Mar 2023, 19:43 Sikatsu, @.***> wrote:

I'm afraid this issue is not being seen though and it's kind of holding me back to update to 1.19.4 as well, just because this bug will go live if I do so. Is there not a way to check the paper commits or try a different method for this? As without ChatControl, it does indeed seem to work..

— Reply to this email directly, view it on GitHub https://github.com/PaperMC/Paper/issues/8911#issuecomment-1489118143, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJMAZG2YK5WN7MLZKNO2ATW6R7EDANCNFSM6AAAAAAVGW4OTE . You are receiving this because you were mentioned.Message ID: @.***>

Sikatsu commented 1 year ago

I tried using the flag, but it seemed to break a couple of plugins for me. For example chat tags/badges could not be parsed and just showed the placeholder in chat.

Hope kangarko has an idea

kangarko commented 1 year ago

I'm going to try to create a sample plugin combination to debug this, please allow extra 2-5 days as I have other works right now.

Sikatsu commented 1 year ago

It's definitely related to ChatControl, we had someone create a test plugin as well which works unless ChatControl is installed. Hope for a fast fix so we can update to 1.19.4..

Owen1212055 commented 1 year ago

This most likely has been resolved in https://github.com/PaperMC/Paper/pull/9099.

Please feel free to leave a comment if you are still experiencing this issue.