-
Notifications
You must be signed in to change notification settings - Fork 42
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
FG2 Support #63
base: dev/0.10.0
Are you sure you want to change the base?
FG2 Support #63
Conversation
Impressive, I will have to look into #36 first, but doesn't legacy forge patches require using pack200? |
idk, it just seemed to work, i thought it needed a --pack200 arg with the binpatcher tho but it didn't throw any errors when I ran in the test mod's build. maybe it didn't actually apply the patch... |
yeah, comparing to a FG env, it appears the patches didn't actually apply, I'll look into it, probably just need to add the pack200 arg |
The issue is that pack200 has been removed since J14, and we would need to bundle it somewhere |
isn't arch-loom still built against java 11? really expecting fully modern java on old versions seems secondary |
We now fully require J16. |
Hmm, I'd argue that expecting outdated Java makes no sense. |
yeah, packaging pack200 would be needed then, unless it's provided as a separate dependency somewhere... |
pack200 appears to use native code, so I was thinking find someone's "modern" implementation of pack200 in java and kinda jury rig it onto the old pack200 interface, (or pre-unpack it for binpatcher if we don't need --legacy) |
ReplayMod's fork of FG works with Java 16 and appears to package pack200. Maybe it would be useful as a reference. |
Yes, we mentioned that on discord i think... they simply re-pack openjdk's pack200 class iirc |
because of module stuff, it looks like I have to fork binpatcher and do it there... |
f606772
to
a4e9363
Compare
at this point I know there are wrong checksums, but I don't know why |
ok figured it out. now I just have to figure out how to remap it properly... |
now the final patched mappings are stuck at srg and not remapping to MCP... or even official |
f129aaa
to
689eb98
Compare
how I actually ended up doing it, I can pull pack200 back out into arch-loom and go back to official binpatcher? should I? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please import the checkstyle for the imports (there is an idea plugin for checkstyle), the diff is abnormally big, I can fix the actual checkstyle issues later on, but the imports changes are annoying
Also please keep the indents consistent, Loom uses tabs, not spaces
please also keep unrelated changes to a minimum
I am not too fond of having FG2 code basically copy and pasted here, but I will look into that in the future
...main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingsProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java
Outdated
Show resolved
Hide resolved
src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java
Outdated
Show resolved
Hide resolved
import net.fabricmc.loom.configuration.providers.forge.McpConfigProvider.RemapAction; | ||
import net.fabricmc.loom.util.FileSystemUtil; | ||
import net.fabricmc.loom.util.ThreadingUtils; | ||
|
||
public class SpecialSourceExecutor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is hard to review, did any functionality here change? Or is it a refactor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor, look between it and the old one and it's pulling the stripping stuff out to another function so I can use it separately
6e0dfd3
to
9e81355
Compare
Signed-off-by: shedaniel <[email protected]>
Signed-off-by: shedaniel <[email protected]>
Signed-off-by: shedaniel <[email protected]>
7d41150
to
718c0be
Compare
add fg2 support to arch-loom, kinda needs #36 but using legacy intermediaries essentially as stubs works so...
the 1.12.2 fg3 versions are still borked, but using
14.23.5.2847
as fg2 works so... good enough for meTodo