[Issue] - Fix HudElementRegistry.replaceElement() for subtitles during gameplay. #5012
Conversation
Liam-Broome
changed the title
|
The test |
yes, MC-304369 |
modmuss50
added
the
status: merge me please
|
Wait don't merge yet |
| HudElementRegistryImpl.getRoot(VanillaHudElements.PLAYER_LIST).render(context, tickCounter, (ctx, tc) -> renderVanilla.call(instance, ctx, tc)); | ||
| } | ||
|
|
||
| @WrapOperation(method = "render", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/gui/Gui;renderSubtitleOverlay(Lnet/minecraft/client/gui/GuiGraphics;Z)V")) |
There was a problem hiding this comment.
I disagree with this mixin, see https://discord.com/channels/507304429255393322/807617700734042122/1440940666829013103.
I think it would better to instead mixin into the renderSubtitleOverlay method (and the lambda) to wrap the subtitleOverlay#render (as shown in the image).
As stated by @cassiancc, we can obtain the DeltaTracker/RenderTickCounter from the client so my suggested change may actually be better.
There was a problem hiding this comment.
@ekulxam - I have made some changes, thanks for the feedback 😄
| @Inject(method = "render", at = @At("HEAD"), cancellable = true) | ||
| private void wrapSubtitleRender(GuiGraphics context, CallbackInfo ci) { | ||
| if (!fabric_renderingThroughHud) { | ||
| DeltaTracker deltaTracker = Minecraft.getInstance().getDeltaTracker(); | ||
|
|
||
| ci.cancel(); | ||
|
|
||
| HudElementRegistryImpl.getRoot(VanillaHudElements.SUBTITLES) | ||
| .render(context, deltaTracker, (ctx, tc) -> { | ||
| fabric_renderingThroughHud = true; | ||
|
|
||
| try { | ||
| fabric_callOriginalRender(ctx); | ||
| } finally { | ||
| fabric_renderingThroughHud = false; | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| @Unique | ||
| private void fabric_callOriginalRender(GuiGraphics context) { | ||
| ((SubtitleOverlay) (Object) this).render(context); | ||
| } |
There was a problem hiding this comment.
why does this need to be recursive? I would think that this should be a WrapMethod.
There was a problem hiding this comment.
I said to wrap it and not inject. It should actually be a WrapOperation into both the lambda and the renderSubtitleOverlay method with the render call as the target.
There was a problem hiding this comment.
You're right actually; WrapMethod is cleaner here and does accomplish what we wanted.
modmuss50
removed
the
status: merge me please
|
@Liam-Broome are you still working on this? If not, then I'd like to implement the requested changes myself. No worries if you're busy. |
|
@ekulxam - Feel free to take over 😀 |
|
@Liam-Broome I've made a pull request to your fork Liam-Broome#1 |
cputnam-a11y
left a comment
cputnam-a11y
left a comment
There was a problem hiding this comment.
I still don't really understand the actual issue here, but the mixins do look much cleaner now
#5012) * [Issue] - Enhance subtitle overlay rendering in GuiMixin. * Fix indentation in GuiMixin subtitle rendering * Refactor subtitle overlay rendering to new mixin * Clean up formatting in SubtitleOverlayMixin * Cleanup SubtitleOverlayMixin * Fix checkstyle --------- Co-authored-by: modmuss <[email protected]> Co-authored-by: SkyNotTheLimit <[email protected]> (cherry picked from commit 7edacff) # Conflicts: # fabric-rendering-v1/src/client/java/net/fabricmc/fabric/mixin/client/rendering/GuiMixin.java
5 participants
Fixes issue #4933 where
HudElementRegistry.replaceElement(VanillaHudElements.SUBTITLES, ...)did not work correctly during normal gameplay, only when the game was paused. This prevented mods from properly scaling, positioning, or otherwise modifying subtitle rendering during active play.Screen.Recording.2025-11-19.at.16.17.20.mov