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

NeoForge Merges Mojmap Methods Causing Intermediary Mapping Conflicts #206

Closed
Kneelawk opened this issue Apr 22, 2024 · 3 comments · Fixed by #209
Closed

NeoForge Merges Mojmap Methods Causing Intermediary Mapping Conflicts #206

Kneelawk opened this issue Apr 22, 2024 · 3 comments · Fixed by #209
Labels
bug Something isn't working priority: high This needs to be worked on and reviewed ASAP

Comments

@Kneelawk
Copy link

The Issue

Because NeoForge uses mojmap everywhere, they can take two methods in different classes that have the same signature and have both classes implement a new interface that unifies those two methods. However, this breaks in arch-loom because arch-loom needs to map everything to intermediary in order to map to the destination mapping. And in intermediary, because these methods were completely unrelated, they have different signatures.

Example

In this case, ClientCommonPacketListenerImpl and ServerCommonPacketListenerImpl both have a method send(Lnet/minecraft/network/protocol/Packet;)V. NeoForge adds an interface called ICommonPacketListener and has these two classes implement it. ICommonPacketListener also has a method send(Lnet/minecraft/network/protocol/Packet;)V, thus unifying these two methods.

However in intermediary, ClientCommonPacketListenerImpl.send is net.minecraft.class_8673.method_52787 and ServerCommonPacketListenerImpl.send is net.minecraft.class_8609.method_14364, meaning that these two methods have different and incompatible signatures.

How to Reproduce

This issue currently affects Neoforge PR #835. Simply using this version of neoforge in an architectury-loom project will produce mapping conflicts.

Error Log

I have uploaded a copy of the gradle output to patebin.

Environment Details

  • Architectury Loom: 1.6.394
  • NeoForge: 20.5.0-alpha.1.20.5-rc1.20240422.033338
  • Java: openjdk 21.0.2 2024-01-16 LTS
  • OS: Linux
@Jab125 Jab125 added the priority: high This needs to be worked on and reviewed ASAP label Apr 22, 2024
@Jab125
Copy link
Member

Jab125 commented Apr 22, 2024

A possible workaround is loom is to create bridge methods

@Kneelawk
Copy link
Author

I'm attaching the stack trace as well: https://pastebin.com/1A12RgbG

@Jab125
Copy link
Member

Jab125 commented Apr 30, 2024

NeoForge#835 has been merged in NeoForge 20.6.3-beta

TreyRuffy added a commit to TreyRuffy/BetterF3 that referenced this issue May 2, 2024
Waiting on architectury/architectury-loom#206 for NeoForge
Looking into Minecraft Forge support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: high This needs to be worked on and reviewed ASAP
Projects
None yet
2 participants