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

Inner class mappings are broken on pre-1.8 versions #265

Open
makamys opened this issue Feb 23, 2023 · 2 comments
Open

Inner class mappings are broken on pre-1.8 versions #265

makamys opened this issue Feb 23, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@makamys
Copy link

makamys commented Feb 23, 2023

On 1.7.10 (and I assume any previous versions), inner classes use intermediary names despite there existing yarn mappings for them. This is ostensibly because the tooling assumes official inner class names start with <parent_name>$, which is only true starting from 1.8.

Example (1.7.10)

class_1253$class_1254 is mapped as MineshaftPieces$MineshaftCorridor...

CLASS class_1254 MineshaftCorridor

But in minecraft-merged-named.jar after importing the example mod, the intermediary name is used, and the class is not an inner class:

StructurePieceManager.registerPiece(class_1254.class, "MSCorridor");

Examining mappings.tiny in the Gradle cache (~/.gradle/caches/fabric-loom/1.7.10/net.legacyfabric.yarn.1_7_10.1.7.10+build.458-v2/mappings.tiny), we can see the following weirdness:

c	asy	net/minecraft/class_1254	net/minecraft/structure/class_1254

The class is missing the yarn name. And later in the file, the class appears again, this time with the intermediary name where the official name should be:

c	net/minecraft/class_1253$class_1254	net/minecraft/class_1253$class_1254	net/minecraft/structure/MineshaftPieces$MineshaftCorridor

The class is also treated like a non-inner class in the intermediary mapping:

CLASS	asy	net/minecraft/class_1254

Example (1.8.9)

Let's compare this with 1.8.9, which works correctly.

        StructurePieceManager.registerPiece(MineshaftCorridor.class, "MSCorridor");

In mappings.tiny, there is only a single class containing class_1254 in its name:

c	aqg$a	net/minecraft/class_1253$class_1254	net/minecraft/structure/MineshaftPieces$MineshaftCorridor

We can see that its official name contains the parent class's name before a $ character. This is aqg for reference:

c	aqg	net/minecraft/class_1253	net/minecraft/structure/MineshaftPieces

Intermediary also correctly shows the class as an inner class:

CLASS	aqg$a	net/minecraft/class_1253$class_1254

Fixing it

Fixing this will possibly require changes in Enigma, since it ignores what comes before $ in yarn names, and establishes the parent class relationship by checking the name that comes before the $ in the official name (which, in 1.7.10's case, does not exist). I wasn't able to fix the issue by manually changing the intermediary mapping to CLASS asy net/minecraft/class_1253$class_1254 for this reason.

@makamys makamys added the bug Something isn't working label Feb 23, 2023
@thecatcore
Copy link
Member

It's because before 1.8.2, Mojang was stripping inner class info from the inner classes so for those versions they appear as independent classes with weird synthetic fields.

@thecatcore
Copy link
Member

We may use Ornithe Nests in the future to fix this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants