Mixin cleanup #5118

Earthcomputer · 2026-01-02T18:19:39Z

Todo:

(not all of them will need fixing)

…es require annotations in these modules

modmuss50 · 2026-01-04T14:07:35Z

I will wait for @sylv256 review before merging. There is no rush with this.

LlamaLad7

Gave up individually commenting but you get the idea. I know these were problems before but it we're changing them may as well fix them.

Additionally:

Using names for argsOnly locals seems to me less stable than using ordinals / implicits, since it seems more likely for the internal code to change than the external api
Implicit mode seems potentially more stable than names when possible for normal locals, though I feel less strongly about this

LlamaLad7 · 2026-01-04T14:13:37Z

fabric-block-api-v1/src/main/java/net/fabricmc/fabric/mixin/block/LivingEntityMixin.java


 @Mixin(LivingEntity.class)
 abstract class LivingEntityMixin {
+	@Definition(id = "getBlockState", method = "Lnet/minecraft/world/level/Level;getBlockState(Lnet/minecraft/core/BlockPos;)Lnet/minecraft/world/level/block/state/BlockState;")


Either define the local or switch to MEV

LlamaLad7 · 2026-01-04T14:14:52Z

...pi-v2/src/client/java/net/fabricmc/fabric/mixin/blockview/client/RenderRegionCacheMixin.java


 	@Inject(method = "createRegion", at = @At(value = "INVOKE", target = "Lnet/minecraft/client/renderer/chunk/RenderRegionCache;getSectionDataCopy(Lnet/minecraft/world/level/Level;III)Lnet/minecraft/client/renderer/chunk/SectionCopy;"))
-	private void copyDataForChunk(Level level, long packedChunkPos, CallbackInfoReturnable<RenderSectionRegion> cir, @Share("dataMap") LocalRef<Long2ObjectOpenHashMap<Object>> mapRef, @Local(ordinal = 11) int x, @Local(ordinal = 10) int y, @Local(ordinal = 9) int z) {
+	private void copyDataForChunk(Level level, long sectionNode, CallbackInfoReturnable<RenderSectionRegion> cir, @Share("dataMap") LocalRef<Long2ObjectOpenHashMap<Object>> mapRef, @Local(name = "regionSectionX") int regionSectionX, @Local(name = "regionSectionY") int regionSectionY, @Local(name = "regionSectionZ") int regionSectionZ) {


Can any of these locals be avoided by switching injectors, if they're involved in the call?

LlamaLad7 · 2026-01-04T14:19:16Z

...ies-v0/src/main/java/net/fabricmc/fabric/mixin/content/registry/PathfindingContextMixin.java

-	@Inject(method = "getPathTypeFromState", at = @At(value = "INVOKE_ASSIGN", target = "Lnet/minecraft/core/BlockPos$MutableBlockPos;set(III)Lnet/minecraft/core/BlockPos$MutableBlockPos;"), cancellable = true)
-	private void onGetNodeType(int x, int y, int z, CallbackInfoReturnable<PathType> cir, @Local BlockPos pos) {
+	@Definition(id = "set", method = "Lnet/minecraft/core/BlockPos$MutableBlockPos;set(III)Lnet/minecraft/core/BlockPos$MutableBlockPos;")
+	@Expression("? = ?.set(?, ?, ?)")


Again either define local or use another injector

LlamaLad7 · 2026-01-04T14:20:03Z

...ries-v0/src/main/java/net/fabricmc/fabric/mixin/content/registry/WalkNodeEvaluatorMixin.java

 	@Inject(method = "getPathTypeFromState", at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/state/BlockState;getBlock()Lnet/minecraft/world/level/block/Block;"), cancellable = true)
-	private static void getCommonNodeType(BlockGetter level, BlockPos pos, CallbackInfoReturnable<PathType> cir, @Local BlockState state) {
-		PathType pathType = LandPathTypeRegistry.getPathType(state, level, pos, false);
+	private static void getCommonNodeType(BlockGetter level, BlockPos pos, CallbackInfoReturnable<PathType> cir, @Local(name = "blockState") BlockState blockState) {


Is the local avoidable with ModifyReceiver?

LlamaLad7 · 2026-01-04T14:28:52Z

...entity-events-v1/src/main/java/net/fabricmc/fabric/mixin/entity/event/LivingEntityMixin.java

-	@ModifyVariable(method = {"lambda$stopSleeping$0", "startSleeping"}, at = @At(value = "INVOKE_ASSIGN", target = "Lnet/minecraft/world/level/Level;getBlockState(Lnet/minecraft/core/BlockPos;)Lnet/minecraft/world/level/block/state/BlockState;"))
-	private BlockState modifyBedForOccupiedState(BlockState state, BlockPos sleepingPos) {
-		InteractionResult result = EntitySleepEvents.ALLOW_BED.invoker().allowBed((LivingEntity) (Object) this, sleepingPos, state, state.getBlock() instanceof BedBlock);
+	@ModifyVariable(method = {"lambda$stopSleeping$0", "startSleeping"}, at = @At(value = "MIXINEXTRAS:EXPRESSION", shift = At.Shift.AFTER))


LlamaLad7 · 2026-01-04T14:30:25Z

...on-v0/src/client/java/net/fabricmc/fabric/mixin/event/interaction/client/MinecraftMixin.java

 	)
-	private void injectUseEntityCallback(CallbackInfo ci, @Local InteractionHand hand, @Local EntityHitResult hitResult, @Local Entity entity) {
-		InteractionResult result = UseEntityCallback.EVENT.invoker().interact(player, player.level(), hand, entity, hitResult);
+	private void injectUseEntityCallback(CallbackInfo ci, @Local(name = "hand") InteractionHand hand, @Local(name = "entityHit") EntityHitResult entityHit, @Local(name = "entity") Entity entity) {


Locals seem avoidable again

LlamaLad7 · 2026-01-04T14:32:20Z

...-v0/src/main/java/net/fabricmc/fabric/mixin/event/interaction/ServerPlayerGameModeMixin.java

 	@Inject(at = @At(value = "INVOKE", target = "Lnet/minecraft/world/level/block/Block;playerWillDestroy(Lnet/minecraft/world/level/Level;Lnet/minecraft/core/BlockPos;Lnet/minecraft/world/level/block/state/BlockState;Lnet/minecraft/world/entity/player/Player;)Lnet/minecraft/world/level/block/state/BlockState;"), method = "destroyBlock", cancellable = true)
-	private void breakBlock(BlockPos pos, CallbackInfoReturnable<Boolean> cir, @Local BlockEntity entity, @Local BlockState state) {
-		boolean result = PlayerBlockBreakEvents.BEFORE.invoker().beforeBlockBreak(this.level, this.player, pos, state, entity);
+	private void breakBlock(BlockPos pos, CallbackInfoReturnable<Boolean> cir, @Local(name = "blockEntity") BlockEntity blockEntity, @Local(name = "state") BlockState state) {


Some locals here seem avoidable, same below

LlamaLad7 · 2026-01-04T14:33:58Z

...t-api-v1/src/main/java/net/fabricmc/fabric/mixin/gametest/StructureTemplateManagerMixin.java


 	@Inject(method = "<init>", at = @At(value = "INVOKE", target = "Lcom/google/common/collect/ImmutableList$Builder;add(Ljava/lang/Object;)Lcom/google/common/collect/ImmutableList$Builder;", ordinal = 2, shift = At.Shift.AFTER))
-	private void addFabricTemplateProvider(ResourceManager resourceManager, LevelStorageSource.LevelStorageAccess storageAccess, DataFixer dataFixer, HolderGetter<Block> blockLookup, CallbackInfo ci, @Local ImmutableList.Builder<StructureTemplateManager.Source> builder) {
+	private void addFabricTemplateProvider(ResourceManager resourceManager, LevelStorageSource.LevelStorageAccess storage, DataFixer fixerUpper, HolderGetter<Block> blockLookup, CallbackInfo ci, @Local(name = "builder") ImmutableList.Builder<StructureTemplateManager.Source> builder) {


Avoidable local

Earthcomputer · 2026-01-04T19:58:56Z

Cleaning up the unnecessary locals was honestly more work than I signed up for

modmuss50 · 2026-01-05T17:57:00Z

There is no need to get them all perfect as part of this PR, as long as its not making things worse.

sylv256

LGTM except for ~~one concern/question I have as well as~~ Llama's comments.

sylv256 · 2026-01-07T04:03:11Z

...der-v1/src/client/java/net/fabricmc/fabric/mixin/resource/client/CreateWorldScreenMixin.java


 	@Inject(method = "getDataPackSelectionSettings",
-			at = @At(value = "INVOKE", target = "Lnet/minecraft/server/packs/repository/PackRepository;reload()V", shift = At.Shift.BEFORE))
+			at = @At(value = "INVOKE", target = "Lnet/minecraft/server/packs/repository/PackRepository;reload()V"))


What does this change do?

the before wasn't needed, so removing it aligns with mixin best practices

It's worse than not needed. Shifting before shifted 1 instruction too early. For each of these changes I manually verified that removing the brittle shift didn't affect behaviour

Earthcomputer added 13 commits January 2, 2026 18:18

Fix all ambiguous mixin references

05ac5a4

Fix all invalid shadow accesses

197fd28

Fix all incorrect usages of shift

988dd8f

Replace all usages of INVOKE_ASSIGN with expressions

3e018c3

Add argsOnly = true to all @Locals where possible

09440ab

Add all missing field opcodes

889d6c5

Remove unused cancellable = true

9643030

Update Fabric loader to 0.18.4

c734a4c

Remove all uses of maxShiftBy (it's unused), and add missing overwrit…

1759b3e

…es require annotations in these modules

Use local variable names for all mixin locals

33028f0

Rename parameters based on what they're called in the target class

1525e42

Fix indentation

0927c75

Remove another unnecessary shift

4fa53af

Earthcomputer marked this pull request as ready for review January 2, 2026 21:59

sylv256 self-requested a review January 2, 2026 22:50

modmuss50 approved these changes Jan 4, 2026

View reviewed changes

modmuss50 added the status: merge me please Pull requests that are ready to merge label Jan 4, 2026

LlamaLad7 suggested changes Jan 4, 2026

View reviewed changes

modmuss50 added cleanup and removed status: merge me please Pull requests that are ready to merge labels Jan 5, 2026

sylv256 approved these changes Jan 7, 2026

View reviewed changes

Mixin cleanup #5118

Are you sure you want to change the base?

Mixin cleanup #5118

Conversation

Earthcomputer commented Jan 2, 2026

Uh oh!

modmuss50 commented Jan 4, 2026

Uh oh!

LlamaLad7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Earthcomputer commented Jan 4, 2026

Uh oh!

modmuss50 commented Jan 5, 2026

Uh oh!

sylv256 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sylv256 left a comment •

edited

Loading