search

finallion / The-Graveyard-FABRIC

Adds structures, blocks and mobs surrounding the graveyard theme.
GNU General Public License v3.0
11 stars 12 forks source link

Fixed #40 #41

Closed CleverNucleus closed 2 years ago

CleverNucleus commented 2 years ago

Changes

-Removed custom AttributeContainer field from relevant entity classes. -Removed overridden LivingEntity#getAttributes method from relevant entity classes. +Added static DefaultAttributeContainer$Builder method for relevant entity classes. Also removed redundant addition of EntityAttributes#GENERIC_MAX_HEALTH where the value was 20.0, as LivingEntity sets this anyway. +Registered the DefaultAttributeContainer$Builder methods to the relevant entity types with the DefaultAttributeRegistry using FabricDefaultAttributeRegistry#register.

Rationale

When setting a custom entity's attributes, the DefaultAttributeContainer$Builder object should be registered with the entity type to the DefaultAttributeRegistry via Fabric API, and as such handled like vanilla.

Since LivingEntity already handles access to the AttributeContainer, the getAttributes() method should not be overridden, as properly registering a DefaultAttributeContainer to the entity type as described above is sufficient.

Regardless of the incompatibility with Data Attributes, it is not optimal to assign an AttributeContainer to an entity on the first time it is needed via a null check - it is better to do this when the entity is initialised/created.

Further Notes

The incompatibility with Data Attributes is resolved with this merge. This is applicable for 1.18.2 as that is the version the issue (#40 ) was presented with; however, these changes should also be made to any 1.19+ versions as they are also incompatible with Data Attributes.

I hope that this is acceptable, Thanks.

finallion commented 2 years ago

I really appreciate your fix and your explanation. Thank you very much!