Skip to content

Add --mixins flag to enable automatic mixin remapping - #44

Open
LexManos wants to merge 5 commits into
MinecraftForge:masterfrom
LexManos:mixin-refmaps
Open

Add --mixins flag to enable automatic mixin remapping#44
LexManos wants to merge 5 commits into
MinecraftForge:masterfrom
LexManos:mixin-refmaps

Conversation

@LexManos

@LexManos LexManos commented Jul 30, 2026

Copy link
Copy Markdown
Member

Currently only supports refmaps.json
This removes the need for the runtime mixin.env.remapRefMap and mixin.env.refMapRemappingFile flags which are poorly documented and only support slow SRG string replacements. As well as only supporting SRG format. This can mess up if the mappings file contains anything that would get simple replaced when it shouldn't. Such as during obf->mcp renaming where fields are often called 'a'.
Example:
FD: A/a Client/field
a:Ljava/lang/String; -> field:Ljfieldvfield/lfieldang/String;
This is a concerned noted in its javadocs which shouldn't be an issue in versions that use SRG names. However I think it's better to address this during the remapping to prevent this issue in the first place.

Now there is a much larger task of actually bypassing the need for the AnnotationProcessor at all.
This would require going through all the possible paths that the AP outputs 'extra' mappings for. Which is a lot. I believe I have found all the cases in my work on Srg2Source. However it would be better if there was some official unit test set. Or if someone else could double check my work.

This Pr is mainly opened as a sanity check, I need someone who actually uses a dep with Mixins to test this. And as an opening to talk about the specifics of going full ham into finding the inheritance info ourselves for mixins.

Note: This adds a dependency on gson, which adds ~200kb(from 450kb) to the jar.

@PaintNinja PaintNinja left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some minor comments, how do I test this?

Comment thread src/main/java/net/minecraftforge/renamer/internal/MixinRenamer.java Outdated
Comment thread settings.gradle
library 'srgutils', 'net.minecraftforge', 'srgutils' version '0.6.0'
library 'powermock', 'org.powermock', 'powermock-core' version '2.0.9'
library 'toml', 'com.github.jezza', 'toml' version '1.2-java-8'
library 'gson', 'com.google.code.gson', 'gson' version '2.14.0'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use Jackson Core or Jackson Jr for a significantly smaller dep than GSON

@LexManos LexManos Jul 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into that jackson jr objects is 150kb and it requires core which is ~200kb. So no, gson is smaller.

Comment on lines +163 to +175
if (this.owner != null)
buf.append('L').append(this.owner).append(';');
if (this.name != null)
buf.append(this.name);
if (this.quantifier != null)
buf.append(this.quantifier);
if (this.desc != null) {
if (this.desc.charAt(0) != '(')
buf.append(':');
buf.append(this.desc);
}
if (this.tail != null)
buf.append(this.tail);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (this.owner != null)
buf.append('L').append(this.owner).append(';');
if (this.name != null)
buf.append(this.name);
if (this.quantifier != null)
buf.append(this.quantifier);
if (this.desc != null) {
if (this.desc.charAt(0) != '(')
buf.append(':');
buf.append(this.desc);
}
if (this.tail != null)
buf.append(this.tail);
if (owner != null)
buf.append('L').append(owner).append(';');
if (name != null)
buf.append(name);
if (quantifier != null)
buf.append(quantifier);
if (desc != null) {
if (desc.charAt(0) != '(')
buf.append(':');
buf.append(desc);
}
if (tail != null)
buf.append(tail);

Omitting the this is a tad cleaner and avoids reading from untrusted final fields on the heap multiple times when the same values are already available on the stack as local variables.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last i looked it compiles to literally the same thing.

Comment thread src/main/java/net/minecraftforge/renamer/internal/MixinRenamer.java Outdated
Comment thread src/main/java/net/minecraftforge/renamer/internal/MixinRenamer.java Outdated
LexManos and others added 3 commits July 30, 2026 14:01
….java

Co-authored-by: Paint_Ninja <PaintNinja@users.noreply.github.com>
….java

Co-authored-by: Paint_Ninja <PaintNinja@users.noreply.github.com>
….java

Co-authored-by: Paint_Ninja <PaintNinja@users.noreply.github.com>
@LexManos

Copy link
Copy Markdown
Member Author

I havent exposed the gradle side. But to test it you should just be able to run the jar with --input, --output, --mappings and --mixins specified. And then shove the resulting jar in your dev mods folder without renamer.enableMixins() and it should work.

We need a good SIMPLE mixin test mod. But I dont gave any and havent dug into mixin for a while to write one.

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

Successfully merging this pull request may close these issues.

2 participants