Paper
Paper copied to clipboard
PaperMC
AbstractChatEvent viewers should be mutable in all cases
Is your feature request related to a problem?
When writing plugins that contain e.g. a team chat the obvious solution would proably be to modify the viewers in AbstractChatEvent with for example removeIf. But when looking at the viewers() method in the event the javadocs say the following:
"Listeners should be aware that modifying the list may throw UnsupportedOperationException if the event caller provides an unmodifiable set."
But you could just cancel the event and send the message to the appropriete players with sendMessage right? Sadly this is not really a good solution since the message will be displayed as a system messsage to the players. And for other plugins monitoring the outcome of the event it will be unclear what players will recieve the message.
Describe the solution you'd like.
I fully understand that there isn't a good method to check if a set or list is mutable. But with how the javadoc is written its really unclear if paper itself calls it with an unmutable set or if its just third party plugins and it does not encurrage developers to provide a mutable set because it could break plugins when they dont.
I think the javadocs should state that a mutable set should be provided because it can cause incompatability if an immutable one is given. And the docs should also say wheather paper itself provides immutable sets in specific cases.
Describe alternatives you've considered.
Catching the exception and using sendMessage in case of faliure. But in my opinion thats not really a good way to solve the issue
Other
No response
Somewhat a duplicate of https://github.com/PaperMC/Paper/issues/6620, which got staled. I don't quite remember, but there was a discussion on Discord server - so the Paper will never provide unmodifiable viewers, yet plugins calling the event can do that. That doesn't sound like a solution, so ehh.
Paper follows upstreams API contract of providing 0 promises over what the caller provides here, idk if there's much inclination to change that. While the implementation, at the moment, will not provide an immutable collection, there is no real inclination to make the promise that that won't ever change
Paper follows upstreams API contract of providing 0 promises over what the caller provides here, idk if there's much inclination to change that. While the implementation, at the moment, will not provide an immutable collection, there is no real inclination to make the promise that that won't ever change
With the changes of 1.19 I think this should be updated though. Or make it clear that whilst the caller could provide an immutable set it is not encurraged and as long as its called by paper its guarateed to be mutable.
Saying it's not encouraged to provide a mutable collection changes nothing in regards to the API contract here. Saying that it'l never be immutable when provided by paper is not something we can promise, nor should we; That's implementation detail, which is irrelevant to the observer
You are right. But there should still be some sort of api to manage recievers. Just canceling is not really a good experience with 1.19 and always having a try catch for this is a pretty ugly solution
I mean, java shamefully doesn't have a good way to represent im/mutable characteristics of a collection, but, I think making any promises around if the viewers list is flawed, it should be 100% up to the caller to determine if people get to modify whom they think should be able to see the result of the thing, especially given that when we go forward, we actually need to care about what's inside of this event in order to ensure that the server is kept properly updated in terms of wider state
it should be 100% up to the caller to determine if people get to modify whom they think should be able to see the result of the thing.
I think the javadocs should say that the set will be mutable due to native calls tho. Like that is useful right? be able to remove viewers from the message.
