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

Clean up mixins #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Daomephsta
Copy link

Below is a list of the changes with justification and an explanation for readers of the repo wondering why the code differs from the video.

  • Replace ItemRendererAccessor with a @Shadow in ItemRendererMixin. A shadow is a simpler and cleaner way to access a field declared by the mixin's target class. Accessors are for inaccessible fields of other classes.
    The shadow is @Final rather than final because
  • Replace ModelLoaderMixin with a ModelLoadingPlugin. FAPI APIs are preferable to mixins, as FAPI will avoid changing APIs if possible, while the internals mixins depend on may change at any time.
  • Make handler methods private when possible. It's standard Java best practice to minimise access levels. This applies to mixins too,
  • Use a constant ModelIdentifier field for the ruby staff. Avoids a typo in the mixin or ModelLoadingPlugin breaking the 3d model.

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.

1 participant