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

Did someone say reflection? #1561

Merged
merged 17 commits into from
Jul 24, 2022
Merged

Conversation

derklaro
Copy link
Contributor

@derklaro derklaro commented Apr 3, 2022

This is not done yet, just wanted to draft this for now (as it compiles again after days of "pain" xD)

@derklaro derklaro marked this pull request as ready for review May 26, 2022 15:41
@derklaro
Copy link
Contributor Author

Just to say something about the change in general: all reflections are now using MethodHandles which should be much faster as they use generated code internally. Thats also part of the reason why I removed the structure compiler, but the more relevant one is that there are not much packets which have public fields so that they can be accessed via the compiled structure modifer, and defining them as "nest mates" in the packet class directly does not work as the class loader of the packet classes has no access to the protocol lib internal classes (changing some stuff so that it is possible would be doable, but MethodHandles should be enough).

Just to say it: when exploding 1000 TNT blocks - reading the x, y, z values from the explosion packet is less heavy than sending a message to the player:
image

I also removed the class accessors for pre-1.8 classes (as injecting into the netty pipeline is not even a thing anymore), and most of the duplicated reflection code (like access via Accessors, FieldUtils, MethodUtils, Fuzzy etc.).

This change is tested on various minecraft versions between 1.8 and 1.18

@dmulloy2
Copy link
Owner

great work, this is really impressive stuff!

looks like there's a small merge conflict in EnumWrappersTest but otherwise i think it's good to go?

@derklaro
Copy link
Contributor Author

looks like there's a small merge conflict in EnumWrappersTest but otherwise i think it's good to go?

Will fix that tomorrow 👍

dmulloy2
dmulloy2 previously approved these changes Jun 29, 2022
@dmulloy2 dmulloy2 enabled auto-merge (squash) June 29, 2022 23:09
auto-merge was automatically disabled July 23, 2022 08:31

Head branch was pushed to by a user without write access

@dmulloy2 dmulloy2 merged commit c5f0550 into dmulloy2:master Jul 24, 2022
@derklaro derklaro deleted the reflection-cleanup branch August 1, 2022 06:31
@opl- opl- mentioned this pull request Oct 25, 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.

None yet

2 participants