Fabric API

Fabric API

106M Downloads

Lets Talk: Inconsistent names for classes and mixins.

i509VCB opened this issue ยท 1 comments

commented

This may sound similar to #510 but this specifically for class names.

Why is this an issue?

The naming of classes makes the classes' purpose obvious. The naming style can help with reviews and maintenance since maintainers and reviewers can see very quickly see what the original purpose of the class was.

Across the entire fabric api project, names are quite inconsistent due to many contributors and maintainers. And the changing standards over at yarn also have an influence on the naming of this project.

Finally, a project looks more professional when naming is consistent across the entire board.

What names are used?

For Accessors:

TargetHooks
Example:
EntityHooks

TargetAccessor
Example:
KeyCodeAccessor

AccessorTarget
Example (Inside of a PR):
AccessorRegistry

MixinTarget (Not actually a normal mixin)
Example:
MixinFormat4ResourcePack

Throughout the codebase, TargetAccessor is the most widely used.

For Mixins

Currently, all mixins in the 1.16 branch use MixinTarget. Throughout the codebase, almost every class uses natural language order for names, such as LootEntryTypeRegistryImpl, FabricBakedModel, CommandRegistrationCallback. This naming between classes and mixin class names is inconsistent. For additional consideration, yarn's conventions require the use of natural language order.

For Duck Interfaces

For clarification, a duck interface is an interface which is implemented onto a class using a mixin. Public facing duck interfaces in api packages, are not counted in this section, and should be the exception.

TargetHooks (ish) (Sometimes it refers to an arbitrary name or the target)
Example:
FabricTagHooks

TargetHook (ish) (Similar to above)
Example:
ServerPlayerEntitySyncHook

TargetInternals
Example:
BlockSettingsInternals

AccessTarget
Example:
AccessAmbientOcclusionCalculator

TargetExtensions
Example:
ItemGroupExtensions

TargetAccessor (Not actually an accessor mixin)
Example:
CustomPayloadC2SPacketAccessor

TargetManager
Example:
VanillaParticleManager

Public facing api class names (Not mixin related)

This is the most consistent area, as all classes follow the natural language order and have reasonable names.

Private facing implementation class names (Not mixin related)

Classes here seem to generally follow the natural language order. The infrequent exceptions are due to naming of vanilla counterparts or other reasons.

The steps to a possible solution

  • Debate the naming scheme that fabric api as a project should adopt.
  • Write a PR defining this naming scheme in a CONVENTIONS.MD similarly to Yarn
  • Pass the conventions PR through last call in order to iron out final minor issues
  • Upon merging of the conventions PR, all future PRs shall be subject to the new naming scheme.
  • At some point in the future, some PRs (advise is multiple PRs for quicker reviews) to migrate old code over to the new naming scheme.

Optional:

  • In the future if a CONTRIBUTING.MD is written, link to the CONVENTIONS.MD file.
commented

All of the mixin names are now validated at build time, if you feel like there is still stuff to be done feel free to open a new issue based on the current state of things.