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

Add FormattedComponentArgument for /cwe #464

Open
wants to merge 7 commits into
base: fabric
Choose a base branch
from

Conversation

xpple
Copy link
Collaborator

@xpple xpple commented Nov 13, 2022

I rewrote FormattedTextArgumentType. The previous implementation was too verbose and moreover not working as one might have expected it to. This new syntax is less error prone and probably more intuitive to work with. The availability of some text styles have been dropped, as firstly the previous implementation was incomplete and secondly because this argument type is meant to provide Formattinged text, not Styled text. Aside from what's available in the Formatting class, I did decide to add hex colour support through the # prefix with lenient hex parsing. Do note that Styled text is still available using the /ctellraw command.

@Earthcomputer
Copy link
Owner

Hmm, could we leave in some of the old syntax as an alternative to the new syntax, so that those of us who haven't memorized formatting codes can still use it?

@xpple
Copy link
Collaborator Author

xpple commented Nov 20, 2022

I think that will get a bit confusing. What about making this into a separate class? Something like LegacyFormattedTextArgumentType. Then the /cnote command would just remain as is. My intentions for this class were to use this formatting syntax to be able to write formatted messages over C2C packets in a very compact and non distracting way. Being spammed with formatting suggestions all the time would get annoying if one just wanted to send a simple message.

…gument-rewrite

# Conflicts:
#	src/main/resources/assets/clientcommands/lang/en_us.json
@xpple xpple added enhancement New feature or request awaiting response This issue awaits a response labels Jul 3, 2023
…gument-rewrite

# Conflicts:
#	src/main/java/net/earthcomputer/clientcommands/command/NoteCommand.java
#	src/main/java/net/earthcomputer/clientcommands/command/arguments/FormattedTextArgumentType.java
#	src/main/resources/assets/clientcommands/lang/en_us.json
@xpple xpple changed the title Rewrite FormattedTextArgumentType Add FormattedComponentArgument for /cwe Sep 25, 2024
@xpple
Copy link
Collaborator Author

xpple commented Sep 25, 2024

I have updated the PR. Instead of replacing the existing StyledComponentArgument (which I still think sucks), I have now separated this new argument type. I have furthermore added it to /cwe. In the C2C packet the string is not encoded as formatted text usually is, which is very lengthy. Instead the raw argument string is sent, with the use of the WithStringArgument.
image
image

@xpple xpple requested a review from Earthcomputer September 25, 2024 13:33
Copy link
Owner

@Earthcomputer Earthcomputer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#464 (comment) you still haven't addressed this comment, but I want to look into rewriting the formatted component argument type to look more like markdown

@xpple
Copy link
Collaborator Author

xpple commented Nov 20, 2024

Well I addressed that by creating a separate one as opposed to rewriting the current one. And to be honest most clientcommands users will already be familiar with this & formatting due to their common usage in Spigot servers. And if not, this syntax is nondistracting so it will not get in the way if one wants to send a simple message and also (surprisingly perhaps) easy to learn. I know the syntax is not established in the modded server community but it is definitely in the Spigot server community, which most of the clientcommands users (to my experience) are in. I would say it is a decent syntax because it is good at its purpose.

Markdown is also an option, but I do not think a great one. For one it has no notion of colouring and we would only be able to support a perhaps arbitrary to the user subset of it, unless of course we implement our own client formatting, which I find to be beyond the scope of clientcommands c.q. this PR. I do know of the existence of certain Markdown specs that do support coloured text, but they are not standard at all, so the user would have to learn a new syntax which we probably do not want to introduce.

Lastly, as with all choices, it could be made configurable.

Copy link
Owner

@Earthcomputer Earthcomputer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrote a new argument type in #673 which is now merged, so this needs to be modified to use that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting response This issue awaits a response enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants