-
Notifications
You must be signed in to change notification settings - Fork 413
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
modmuss50
merged 5 commits into
FabricMC:1.20.1
from
PepperCode1:pr/1.20.1/block-view-api
Sep 3, 2023
Merged
BlockView API v2 #3268
modmuss50
merged 5 commits into
FabricMC:1.20.1
from
PepperCode1:pr/1.20.1/block-view-api
Sep 3, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
Technici4n
reviewed
Aug 18, 2023
...ic-block-view-api-v2/src/main/java/net/fabricmc/fabric/api/blockview/v2/FabricBlockView.java
Show resolved
Hide resolved
Technici4n
reviewed
Aug 18, 2023
...ic-block-view-api-v2/src/main/java/net/fabricmc/fabric/api/blockview/v2/FabricBlockView.java
Show resolved
Hide resolved
Technici4n
reviewed
Aug 18, 2023
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.
Looks amazing! Left a few comments.
Technici4n
reviewed
Aug 22, 2023
...ic-block-view-api-v2/src/main/java/net/fabricmc/fabric/api/blockview/v2/FabricBlockView.java
Outdated
Show resolved
Hide resolved
modmuss50
approved these changes
Aug 24, 2023
PepperCode1
added
enhancement
New feature or request
new module
Pull requests that introduce new modules
labels
Aug 26, 2023
Technici4n
approved these changes
Aug 28, 2023
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
andRenderAttachedBlockView
, 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 ofBlockRenderView
, but this is not the case. The module implementation only guarantees that allWorldView
instances implement the interface.RenderAttachedBlockView
extendsBlockRenderView
, 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 ofRenderAttachedBlockView
impossible.The BlockView API v2 module was made to solve these issues, solve other issues with Rendering Data Attachment v1, and provide additional features.
FabricBlockView
interface supersedes theRenderAttachedBlockView
interface.BlockRenderView
instances, as the current contract ofRenderAttachedBlockView
states, it is implemented on allBlockView
instances to provide more flexibility.hasBiomes
andgetBiomeFabric
, to allow biome retrieval. This feature is the only way to create biome-dependent block models, as theBlockRenderView
provided normally does not have this functionality.RenderDataBlockEntity
interface supersedes theRenderAttachmentBlockEntity
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 theBlockEntity
extension interface is implemented much more frequently than it is invoked. As such, it was decided that the v1RenderAttachedBlockView#getBlockEntityRenderAttachment
should proxy to the v2FabricBlockView#getBlockEntityRenderData
and the v2RenderDataBlockEntity#getRenderData
should proxy to the v1RenderAttachmentBlockEntity#getRenderAttachmentData
.