-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
[1.20.2] Compress the entity
library and rename the entity
module to entity_extensions
#337
Conversation
The entity library had way too many 1 or 2 class modules. This compresses most of those modules into the `entity_extensions` library (renamed from `entity` to match `block_extensions` et al)
4832877
to
c232be0
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.
This might be the ideal route for many of these Fabric Object Builder API-derived APIs; few things though:
- it might be better if Quilt Entity Networking API remained split from the rest
library/entity/entity_extensions/src/main/resources/quilt_entity_extensions.accesswidener
Outdated
Show resolved
Hide resolved
library/entity/entity_extensions/src/main/resources/quilt_entity_extensions.accesswidener
Show resolved
Hide resolved
library/entity/entity_extensions/src/testmod/resources/quilt.mod.json
Outdated
Show resolved
Hide resolved
library/entity/entity_extensions/src/testmod/resources/quilt.mod.json
Outdated
Show resolved
Hide resolved
library/entity/entity_extensions/src/testmod/resources/quilt.mod.json
Outdated
Show resolved
Hide resolved
library/entity/entity_extensions/src/testmod/resources/quilt_entity_test.mixins.json
Outdated
Show resolved
Hide resolved
Ennui, I disagree--QuiltExtendedSpawnDataEntity is the definition of an entity extension, and the code footprint is pretty small. |
hm, fair enough |
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.
With Ennui's suggestions its good. Smart idea packing up the modules. Should help with build times everywhere. That is something we should think about more often.
Co-authored-by: Ennui Langeweile <[email protected]>
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.
after this last pass? i believe this is ready for merge;
checkstyle violations can be fixed post-merge
...ions/src/main/java/org/quiltmc/qsl/entity/extensions/mixin/PointOfInterestTypesAccessor.java
Show resolved
Hide resolved
...extensions/src/main/java/org/quiltmc/qsl/entity/extensions/mixin/networking/EntityMixin.java
Show resolved
Hide resolved
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.
Hm, something that I'm wondering; Why replace the AW with the mixin accessor? i'm just curious about the reasoning
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.
IDE import times
Co-authored-by: Ennui Langeweile <[email protected]>
This was merged in manually |
The entity library had way too many 1 or 2 class modules. This compresses most of those modules into the
entity_extensions
library (renamed fromentity
to matchblock_extensions
et al).I feel that this structure more closely follows the size and scope of modules in our other libraries (and the kind of size we want overall), but I'm absolutely open to feedback on reverting parts or organizing it differently.