MinecraftForge / ForgeGradle

Minecraft mod development framework used by Forge and FML for the gradle build system
GNU Lesser General Public License v2.1
508 stars 436 forks source link

Index 0 out of bounds for length 0 in reobfJar #937

Open BlayTheNinth opened 8 months ago

BlayTheNinth commented 8 months ago

When using this multiloader template, ForgeGradle is running into an odd exception in reobfJar during build. I've tried experimenting with removing certain parts of the template, but couldn't pinpoint what exactly is causing the issue.

org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':forge:reobfJar'.
        ...
Caused by: java.lang.IndexOutOfBoundsException: Index 0 out of bounds for length 0
        ...
        at net.minecraftforge.srgutils.InternalUtils.loadNamed(InternalUtils.java:39)
        at net.minecraftforge.srgutils.InternalUtils.load(InternalUtils.java:28)
        at net.minecraftforge.srgutils.IMappingFile.load(IMappingFile.java:27)
        at net.minecraftforge.srgutils.IMappingFile.load(IMappingFile.java:22)
        at net.minecraftforge.gradle.userdev.tasks.RenameJarInPlace.apply(RenameJarInPlace.java:66)

Full Stacktrace

LexManos commented 8 months ago

It seems that you are passing in no mappings to the reobf task. Unsure how you're managing that as we set them for everything You'd need to provide the full workspace so I could try locally, or try without this multi-loader template as I have no idea what it does elseware that would screw with the mappings.

You can also try just cleaning your workspace and starting fresh.

jaredlll08 commented 8 months ago

This has been fixed in the template itself, the actual error was that due to conflicting version requirements of DiffPatch between ForgeGradle (using the forked net.minecraftforge:DiffPatch:2.0.12) and NeoGradle (using the upstream latest codechicken:DiffPatch:1.5.0.29, which changed a method signature), I moved the plugin version declarations for FG and NG to their respective build.gradle files (FG used to be in the root build.gradle file with apply(false), as that is what is recommended by gradle), doing this caused another conflicting version issue of srgutils, between FG and I believe (I was having trouble finding exactly what was pulling it in) VanillaGradle, where VG was pulling in 0.4.13/15 (I forgot which exactly), which is missing these lines, which is what led to the error that got reported.

I ended up fixing it on the template by just moving all plugin version declarations to their own project's build.gradle files instead, so as far as ForgeGradle is concerned, nothing is wrong, your code is fine.

Saying that, I would appreciate if ForgeGradle could move to the upstream DiffPatch or if the DiffPatch fork could be updated to at-least this commit, as that commit changes the method signature that started this issue.

LexManos commented 7 months ago

Interesting, it's always a pain to manage dependencies across multiple large projects. I don't see migrating back to upstream DiffPatch. At least not anytime soon as I am working on other parts of the project. Doing so is just not viable right now.

As for updating my fork to that commit. If you'll notice that commit was based on my fork, except I maintained binary compatibility while he didn't. So updating to that commit would be a breaking change for anything downstream for FG.

I'll look into what options I can to see if I can mitigate this, but I don't have many ideas at the moment.

jaredlll08 commented 7 months ago

So updating to that commit would be a breaking change for anything downstream for FG. That is completely understandable, for what it's worth, the issue has been "solved" in the template itself, so there is no need for an immediate solution

but I don't have many ideas at the moment.

One idea I do have to solve this issue, would be to add the LogLevel class from Cover's commit, and add an overload method accepting the LogLevel and mapping it to Level (or implementing all the changes from that commit and adding the overload method for Level and map that to LogLevel). This will make the code a bit messy though, and I am not sure how long term of a solution it is, as the upstream could make other binary breaking changes that aren't as easy to fix.