Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't add empty action #15

Closed
wants to merge 1 commit into from
Closed

Don't add empty action #15

wants to merge 1 commit into from

Conversation

Rothes
Copy link

@Rothes Rothes commented Jun 21, 2022

Adding empty action may cause exception on other things. We should ensure plugin compatibility, e.g Bungee-Chat API cannot parse the json, because there's no empty action.
Fixes Rothes/ProtocolStringReplacer#31

@Aust1n46
Copy link
Owner

The root cause of your issue seems to stem from the API not being able to handle this valid json payload. The client parses it out just fine.

Your suggested change also does not cover the case where someone types in an invalid click_action. Empty is only one invalid case; the config input is a string field were anything can be typed.

My preferred "fix" to VentureChat would be input validation on the click_action in the json_attributes section of the config. This should take place before reaching down into the formatter. Perhaps during the initialization of the jsonFormats with logging for invalid formats.

@LOOHP
Copy link
Contributor

LOOHP commented Jun 24, 2022

I agree that simply not sending empty click action is not a good solution. Input validation (i.e. limiting the user to only select from the available set of click action) is a much better way of doing this. Perhaps any invalid click actions set in the config will be treated as if there is no click action and warn the user about this.

Side note: Not only the bungeecord chat API doesn't handle this, the new Adventure API also doesn't handle this. It is much more difficult to get changes into these libraries. An issue I submitted a year ago is still there waiting, also related to how something is accepted in the vanilla client.

Currently, I'm offering this plugin as a "temporary workaround".

@Rothes
Copy link
Author

Rothes commented Nov 8, 2022

Close as fixed in the other pr.

@Rothes Rothes closed this Nov 8, 2022
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.

Can not replace chat-type on 1.19
3 participants