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

BlockView API v2 #3268

Merged
merged 5 commits into from
Sep 3, 2023
Merged

Conversation

PepperCode1
Copy link
Member

@PepperCode1 PepperCode1 commented Aug 16, 2023

This pull requests adds the BlockView API v2 module, which solves the issues with and supersedes the existing Rendering Data Attachment v1 module and adds additional features.

The original goal was to interface inject the two interfaces provided by Rendering Data Attachment v1, RenderAttachmentBlockEntity and RenderAttachedBlockView, to allow for an easier to use API. It proved to be impossible to achieve this and still provide a clean API without making a new module, as explained below.

The documentation of RenderAttachedBlockView states that it is guaranteed to be implemented on all instances of BlockRenderView, but this is not the case. The module implementation only guarantees that all WorldView instances implement the interface. RenderAttachedBlockView extends BlockRenderView, meaning that changing the mixin to satisfy the stated contract is not possible; thus, it can be deduced that the mixin is correct but the stated contract is not. However, this does not lead to an easy to use API and makes interface injection of RenderAttachedBlockView impossible.

The BlockView API v2 module was made to solve these issues, solve other issues with Rendering Data Attachment v1, and provide additional features.

  • The object that used to be inconsistently termed "Render Data Attachment" or "Render Attachment" has been renamed to "Block Entity Render Data" or simply "Render Data" where sufficient. Only the new terms are used in the new module.
  • The documentation was rewritten to be more concise, provide more examples, remove mentions of implementation details, and add concrete requirements for use of certain methods.
  • The new FabricBlockView interface supersedes the RenderAttachedBlockView interface.
    • Instead of being implemented on all BlockRenderView instances, as the current contract of RenderAttachedBlockView states, it is implemented on all BlockView instances to provide more flexibility.
    • It contains two new methods, hasBiomes and getBiomeFabric, to allow biome retrieval. This feature is the only way to create biome-dependent block models, as the BlockRenderView provided normally does not have this functionality.
  • The new RenderDataBlockEntity interface supersedes the RenderAttachmentBlockEntity interface.

The Rendering Data Attachment v1 module has been deprecated and its documentation has been updated to state the contract that is actually satisfied and link to the v2 replacements. Perfect backward and forward compatibility would result in degraded performance. To compromise, it was observed that the BlockRenderView extension interface is invoked much more frequently than it is implemented, and that the BlockEntity extension interface is implemented much more frequently than it is invoked. As such, it was decided that the v1 RenderAttachedBlockView#getBlockEntityRenderAttachment should proxy to the v2 FabricBlockView#getBlockEntityRenderData and the v2 RenderDataBlockEntity#getRenderData should proxy to the v1 RenderAttachmentBlockEntity#getRenderAttachmentData.

@PepperCode1 PepperCode1 mentioned this pull request Aug 16, 2023
@Technici4n Technici4n self-assigned this Aug 16, 2023
Copy link
Member

@Technici4n Technici4n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks amazing! Left a few comments.

@Technici4n Technici4n requested a review from a team August 18, 2023 08:29
@modmuss50 modmuss50 self-requested a review August 23, 2023 19:54
@PepperCode1 PepperCode1 added enhancement New feature or request new module Pull requests that introduce new modules labels Aug 26, 2023
@Technici4n Technici4n added the last call If you care, make yourself heard right away! label Aug 28, 2023
@Technici4n Technici4n removed their assignment Aug 28, 2023
@modmuss50 modmuss50 added the merge me please Pull requests that are ready to merge label Sep 3, 2023
@modmuss50 modmuss50 merged commit 92a0d36 into FabricMC:1.20.1 Sep 3, 2023
5 checks passed
modmuss50 pushed a commit that referenced this pull request Sep 3, 2023
* Fabric BlockView API v2

* Fix dependency on nonexistent module

* Add test for biome getter

* Improve getBiomeFabric documentation

* Simplify javadoc

(cherry picked from commit 92a0d36)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request last call If you care, make yourself heard right away! merge me please Pull requests that are ready to merge new module Pull requests that introduce new modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants