Fix Fabric fluid sprite reload before Forge texture stitch event dispatch - #2267
Draft
yu1745 wants to merge 1 commit into
Draft
Fix Fabric fluid sprite reload before Forge texture stitch event dispatch#2267yu1745 wants to merge 1 commit into
yu1745 wants to merge 1 commit into
Conversation
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The Issue
Fabric's
SimpleFluidRenderHandlercreates its sprite array before the entries are populated. The entries are initialized later throughFluidRenderHandler.reloadTextures(...)during Fabric's fluid renderer reload.On Forge,
TextureStitchEvent.Postcan be dispatched before that Fabric reload has run. Forge mods may query Connector's bridgedIClientFluidTypeExtensionsfrom this event and receive an array whose still and flowing entries are stillnull.BuildCraft exposes this ordering issue by querying the still texture from its
TextureStitchEvent.Postlistener. Connector then dereferencessprites[0], causing resource loading to fail before the main menu is reached.Downstream report and stack trace: yu1745/ic2-fabric#18
The Proposal
Reload Fabric fluid render handlers at the head of
ForgeHooksClient.onTextureStitchedPost(...), before Forge dispatchesTextureStitchEvent.Postto mod event buses.The implementation:
FluidRenderHandlerRegistrywhen they are used, instead of permanently caching a possibly earlynullresult;This initializes real Fabric fluid sprites before any Forge mod can query the bridge, while retaining safe behavior for genuinely incomplete fluid render handlers.
Possible Side Effects
Each unique Fabric fluid render handler receives an additional
reloadTextures(...)call when the block atlas finishes stitching. This is the lifecycle operation the Fabric API expects for cached fluid sprites.The reload runs before Forge's post-stitch event subscribers rather than later during Fabric's fluid renderer reload. Handlers are deduplicated, and non-block atlases are ignored.
Invalid or missing handlers now produce a missing texture/default tint instead of crashing resource loading.
Alternatives
Registering a
TextureStitchEvent.Postlistener withEventPriority.HIGHESTwas considered, but Forge dispatches mod-bus events to mod containers in sorted mod order. Listener priority therefore cannot guarantee that Connector runs before another mod such as BuildCraft.Reloading textures from every still/flowing texture getter was also considered, but that performs lifecycle mutation during ordinary queries and may repeat the reload unnecessarily.
Only returning the missing texture prevents the crash but leaves valid Fabric fluids visually broken and does not address the resource-loading order.
Additional Notes
Base: current
1.20.1branch at7411b51ef26b335887810c0c8769187886a54fba.Validation:
PUBLISH_RELEASE_TYPE=BETA ./gradlew clean buildsucceeds.