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 AdventureComponentConverter#fromComponentObject #3127

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

iiAhmedYT
Copy link
Contributor

Helps when relocating Adventure on your plugin and still listening to packets containing adventure components

@Ingrim4
Copy link
Collaborator

Ingrim4 commented Jul 17, 2024

What are the use cases if you can already do: fromComponent((Component) component);?

@iiAhmedYT
Copy link
Contributor Author

iiAhmedYT commented Jul 17, 2024

It's used incase you can't do that, i think that should be clear o.O

ProtocolLib is a hacky reflective workaround for packets so using reflections with it and other hacky code is totally expected (especially with what paper does to packets nowadays)

@Ingrim4
Copy link
Collaborator

Ingrim4 commented Jul 17, 2024

It's used incase you can't do that, i think that should be clear o.O

ProtocolLib is a hacky reflective workaround for packets so using reflections with it and other hacky code is totally expected (especially with what paper does to packets nowadays)

Firstly, why is this necessary when the method you added doesn't add any new functionality? Secondly, I don't see a clear use case for this addition. All it does is cast an object to a component, which you can also do in your own code. Can you clarify the specific scenarios where this would be beneficial?

@iiAhmedYT
Copy link
Contributor Author

Im not home to explain it with high detail but here is simple one that requires you to read the description of the pr again

If i cast my component object to my.relocated.adventure.Component

that won't work with net.kyori.adventure.Component

especially that i got that component from ProtocolLib from the system chat packet so it uses the default location not the relocated one

because thats what relocation does 😄

@Ingrim4
Copy link
Collaborator

Ingrim4 commented Jul 17, 2024

Ah your talking about the kind of relocation while shading something into your jar. That was the missing piece of information that made this PR look kind unnecessary to me. Ok than it's a valid change, nvm the previous comments.

@dmulloy2 dmulloy2 enabled auto-merge (squash) July 23, 2024 02:57
@dmulloy2 dmulloy2 merged commit fb08567 into dmulloy2:master Jul 23, 2024
3 checks passed
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.

3 participants