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

Layered remapping failure in some cases #1209

Open
LambdAurora opened this issue Nov 6, 2024 · 1 comment
Open

Layered remapping failure in some cases #1209

LambdAurora opened this issue Nov 6, 2024 · 1 comment
Milestone

Comments

@LambdAurora
Copy link

The Problem

Since loom 1.7 layered mappings cannot be used to combine Mojang mappings with any kind of mappings that remap only a base method or a base class and not the bridge variants or the sub-classes.

So far this creates several issues:

  • Mappings that remap instead of only documenting cannot be combined with the Mojang mappings layer.
  • Mappings that include duplicates for bridge variants (like Parchment) cannot be combined with mappings that only remap base methods.

Reproduction

Reproduction is rather simple: create a standard loom-based project, add layered mappings with first the Mojang mappings layer, add on top either Yarn or YALMM.

To complete further the reproduction, add as middle layer Parchment, we would then end up with:

  • Mojang mappings
  • Parchment
  • Other mappings

Hacky Solution

To avoid being stuck in my own projects I've pioneered a temporary and hacky solution, which consists in having my own Mojang mappings layer to exclude the bridge variants of methods:

This is not a suitable solution for production though: Parchment cannot be used with it as Parchment still include bridge variants of the methods, thus still creating conflicts because then the remapper doesn't understand anymore that Parchment is not trying to rename, only to provide arguments names and javadocs.

More Information

Various information I've gathered from discussions on Discord:

  • Incomplete hierarchy awareness work in tiny remapper might address handling these mappings cases.
  • There is currently no error handling if a class gets remapped to a name already taken, the error will only get visible when running the game, when compiling while using affected classes, or when decompiling.
  • The use of layered mappings in this way seemed to worked with loom 1.6 which adds some weirdness to this issue.
@modmuss50 modmuss50 added this to the 1.9 milestone Nov 10, 2024
@modmuss50
Copy link
Member

Another case worth looking at is laying an empty tiny ontop of Mojamps, as reproduced here: https://github.com/embeddedt/fabric-mappings-repro-project

See: https://discord.com/channels/507304429255393322/842691768175951942/1305164539251130502

@modmuss50 modmuss50 modified the milestones: 1.9, 1.10 Nov 28, 2024
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