-
-
Notifications
You must be signed in to change notification settings - Fork 106
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
feat(api): virtual components #842
base: main/4
Are you sure you want to change the base?
Conversation
kashike
commented
Dec 1, 2022
•
edited by zml2008
Loading
edited by zml2008
- look over component flattening with virtual components
- figure out how to do a builder that's useful for outside people
259561c
to
b3c5c43
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm!
do we need to make any changes to serializers to make sure they throw errors if any VirtualComponent
instances get through unrendered?
additionally this likely needs MM changes too to add this component type (unless we're happy with it just throwing errors?)
api/src/main/java/net/kyori/adventure/text/renderer/AbstractComponentRenderer.java
Outdated
Show resolved
Hide resolved
api/src/main/java/net/kyori/adventure/text/renderer/AbstractComponentRenderer.java
Show resolved
Hide resolved
b3c5c43
to
702cbeb
Compare
api/src/main/java/net/kyori/adventure/text/renderer/AbstractComponentRenderer.java
Outdated
Show resolved
Hide resolved
@kezz right now the virtual component is treated as/turns into a text component whenever its serialized -- when we get a public custom serializer API on MM we will have to make sure user-provided serializers can claim virtual components before the built-in serializers get a say. |
702cbeb
to
b6052d1
Compare
After fiddling around with this a bit to use the API in MM, I think this will need some more iteration before it's ready to push out. Some key things:
|
Would it make sense to, instead of having the 2-layer VirtualComponent -> VirtualComponentHolder<?> -> boxed data, as well as the need for a custom renderer, have the component itself have a "virtual component renderer" instead? A way to do that would be to make it so An example of a simple component which would show each player their own name: Main issue I see (correct me if i'm wrong) is context is generic in adventure, and only in platform does it get a specific type, so one platform may use pointered and others may use something else? i have not looked into that one.
I think that just goes out the window, simply because it no longer becomes adventure's territory, it's a "whatever that 3rd party plugin decided" territory. How relevant is it to adventure how internally the renderer of the 3rd party plugin decides to be? Do equal-ness checks break adventure in significant ways if they're not respected for the renderer of virtual components? |
2786555
to
23fcd20
Compare
api/src/main/java/net/kyori/adventure/text/VirtualComponentImpl.java
Outdated
Show resolved
Hide resolved
23fcd20
to
ec35df3
Compare
ec35df3
to
d78274a
Compare
Hey, any updates on this? It'd be great to see the functionality finally landing on adventure |
I would like it if you could add a builder wrapper for use with the virtual component system. Currently the abstract builders are private (for good reasons). However we are willing to deal with breaking changes when needed, the virtual components however would solve this ongoing issue and prevent any more breaking changes from affecting us. |
f656eb5
to
336905a
Compare