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

chat api improvements for probably after 1.20 because i took too long #311

Closed
wants to merge 35 commits into from
Closed

Conversation

SilverAndro
Copy link
Contributor

@SilverAndro SilverAndro commented Jun 5, 2023

This took a while, probably lots of nitpicking to do (especially with javadoc)

Changes:

  • Test mod is significantly improved
  • TypedChatApiHook is no longer api
  • Errors from events now try to show where the error is from
  • Support for chat signing
  • Support for commands
  • Support for message removal (this isnt used in vanilla so, i think my impl works?)

Testing:

This PR is mostly tested locally, but because i only have 1 minecraft account and no friends, i couldn't test several sections of this PR, notably:

  • Cancelling INBOUND CLIENT CHAT
  • How this works with commands like /msg, especially between players not privy to the conversation
  • Probably other things i dont remember

All squashed commits, newest first:
Cleanup some line lengths, signing rollback support
Add getters and `withX` methods to CommandC2SMessage
remove some unused shadows
mappings note, server side message removal (i think)
whoops missed a `with` method
client side message removal events
minor cleanup
Honestly just need to save this work
Server side commands and verification
Move chat hook to impl, add hook origin in errors
minor cleanup
Honestly just need to save this work
Server side commands and verification
Move chat hook to impl, add hook origin in errors
All squashed commits, newest first:
Cleanup some line lengths, signing rollback support
Add getters and `withX` methods to CommandC2SMessage
remove some unused shadows
mappings note, server side message removal (i think)
whoops missed a `with` method
client side message removal events
minor cleanup
Honestly just need to save this work
Server side commands and verification
Move chat hook to impl, add hook origin in errors
minor cleanup
Honestly just need to save this work
Server side commands and verification
Move chat hook to impl, add hook origin in errors
@EnnuiL EnnuiL added library: management Related to the management library. t: refactor This proposes a refactor. labels Jun 9, 2023
@EnnuiL EnnuiL added t: new api This adds a new API. s: waiting for test This pull request is waiting to be tested, the PR cannot be put in FCP until it has been tested. labels Jun 9, 2023
@EnnuiL
Copy link
Contributor

EnnuiL commented Sep 14, 2023

i thought i could have merged this pr quickly..
but yeah.. i don't feel safe handling this pr's review process.. i never really felt safe after all of that..

@EnnuiL EnnuiL closed this Sep 14, 2023
@SilverAndro
Copy link
Contributor Author

would you mind elaborating? im a little confused on what youre referring to or why it involves closing my pr

@LambdAurora LambdAurora reopened this Sep 14, 2023
@EnnuiL EnnuiL closed this Sep 14, 2023
@LambdAurora
Copy link
Contributor

i thought i could have merged this pr quickly.. but yeah.. i don't feel safe handling this pr's review process..

There is multiple QSL maintainers. This PR can be handled by another team member.

@LambdAurora LambdAurora reopened this Sep 14, 2023

/**
* Provides a method to get a {@link ChatSecurityRollbackSupport} from a {@link MessageChain.Packer} or {@link MessageChain.Unpacker}.
* This class has been provided as API in the case that a user wishes to replace or extend the builtin chat signing system.
Copy link
Contributor

Choose a reason for hiding this comment

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

Dumb question: Why?
When might a mod want to replace/extend the chat signing system?

Copy link
Contributor Author

@SilverAndro SilverAndro Sep 15, 2023

Choose a reason for hiding this comment

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

Hm i think that might be some off docs on my part

essentially there isnt any way to automatically track the state behind custom (un)packers since theyre a FunctionalInterface, so the idea was that if you create a custom (un)packer for any reason, i.e a custom verification system (e2e) or just adding logging, other small tweaks, ect, you could easily register it here

the lookup has to exist anyways, so i didnt see much reason to not just, allow people to register custom ones easily

@SilverAndro SilverAndro closed this by deleting the head repository Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
library: management Related to the management library. s: waiting for test This pull request is waiting to be tested, the PR cannot be put in FCP until it has been tested. t: new api This adds a new API. t: refactor This proposes a refactor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants