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

lack of partial signature remapping #126

Open
tildejustin opened this issue Mar 8, 2024 · 1 comment
Open

lack of partial signature remapping #126

tildejustin opened this issue Mar 8, 2024 · 1 comment

Comments

@tildejustin
Copy link

In prior versions of loom, or withuseLegacyMixinAp = true, signatures which did not exist in the minecraft version being compiled against could still be written partially with named, partially with intermediary mappings. Example:
https://github.com/RedLime/SleepBackground/blob/31fcebfba49dd36d9edfa2d53981febcbcecaef8/src/main/java/com/redlimerl/sleepbackground/mixin/MixinMinecraftClient.java#L41-L43. With useLegacyMixinAp = false, one has to manually remap the entire signature if it does not exist in the compile target jar, since the named parts of the signature are not remapped (I verified this with vineflower).

@nelind3
Copy link

nelind3 commented Jan 12, 2025

This seems to also affect targets that have a method that shouldn't be remapped on a class that should. For example I ran into a problem using the mixin extension to remap a mixin that looks like this:

package com.example.mixin;

import net.minecraft.nbt.NbtList;
import net.minecraft.world.tick.ChunkTickScheduler;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;

import java.util.function.Function;

@Mixin(ChunkTickScheduler.class)
public class ChunkTickSchedulerMixin {
	@Inject(
		method = "toNbt",
		at = @At(
			value = "INVOKE",
			target = "Lnet/minecraft/nbt/NbtList;add(Ljava/lang/Object;)Z"
		)
	)
	private void notProperlyRemapped(long time, Function<Object, String> typeToNameFunction, CallbackInfoReturnable<NbtList> cir) {
		// The target descriptor in the @At is not properly remapped and this @Inject fails outside dev
	}
}

The target descriptor won't get remapped properly. A fernflower decompilation of the remapped .class file looks like this

package com.example.mixin;

import java.util.function.Function;
import net.minecraft.class_6755;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable;

@Mixin({class_6755.class})
public class ChunkTickSchedulerMixin {
	@Inject(
		method = {"method_39365(JLjava/util/function/Function;)Lnet/minecraft/class_2499;"},
		at = {@At(
			value = "INVOKE",
			target = "Lnet/minecraft/nbt/NbtList;add(Ljava/lang/Object;)Z"
		)}
	)
	private void notProperlyRemapped(long time, Function typeToNameFunction, CallbackInfoReturnable cir) {
		
	}
}

Notice how the NbtList reference did not get remapped. I suspect that this happens because the boolean add(T) method on NbtList is declared higher up in the class hierarchy in the collections API and thus shouldn't be remapped and won't have and entry in the mappings file causing tiny remapper to "skip" the entire descriptor assuming that it doesn't need remapping. That's just a guess though. This seems to be related

To reproduce the bug I made a new loom project using fabric-example-mod, configured loom to use tiny remapper for remapping of mixins using useLegacyMixinAp = false in the build.gradle and made a mixin like the one shown above. Building the project and decompiling the result gave me the decompiled code shown above.

nelind3 added a commit to LoofahMC/Loofah that referenced this issue Jan 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants