Kir-Antipov / mc-publish

🚀 Your one-stop GitHub Action for seamless Minecraft project publication across various platforms.
MIT License
216 stars 19 forks source link

[Enhancement] Add support for merged JARs #114

Open Steveplays28 opened 3 months ago

Steveplays28 commented 3 months ago

Current Behavior

Currently, in multiloader projects that use Forgix/ModFusioner to merge JARs, the resolving of the loader metadata files fails, due to the metadata reader looking at the primary file path for loader metadata. https://github.com/Kir-Antipov/mc-publish/blob/c0f30ad683340ba9a5374ba42157179f30171107/src/program.ts#L105-L106

This will not be in a subdirectory of the module for merged JARs.

Desired Behavior

It'd be nice if we could select the path of the loader metadata files, or if these could be automatically resolved from the modules by enabling a flag on the workflow, such as merged-jar: true. Mod dependency info from all supported loaders would also need to be merged together.

Alternative Solutions

I could also not merge the JARs and use 2 publish workflows.

Other Information

N/A

Kir-Antipov commented 3 months ago

Hi! :) It seems you are a bit confused here.

It'd be nice if we could select the path of the loader metadata files

Why? It doesn't make sense, because a metadata file must be present at the expected location strictly specified by a mod loader of your choosing. Otherwise, the mod is deemed invalid and won't be loaded. Thus, I don't see a reason why mc-publish should treat something as a mod when it isn't. I would argue even that it's counterintuitive.

Just to reiterate, /fabric.mod.json, /quilt.mod.json, and /META-INF/mods.toml are always present at those exact locations. Otherwise, what you are trying to publish is not, in fact, a mod. So, your problem is not that you cannot configure a metadata file path (because you shouldn't), but rather that you are trying to publish a wrong jar that doesn't contain metadata files.

In your original post, you mentioned the project you're having troubles publishing - Noisium, but then deleted this information for some reason. Please, don't do that. Such information is actually quite helpful when I try to identify the real issue behind your problems with mc-publish!

Anyway, I looked into your workflow and here's what I found. After 407edef your files input looks like this:

files: |
  **/build/libs/!(*-@(sources|dev|dev-shadow|javadoc).jar)
  **/build/libs/*-@(sources|dev|dev-shadow|javadoc).jar

And, thanks to your other workflow, which creates the compiled_jar_files.zip artifact, we know that the workspace looks like this after you run ./gradlew build:

build
  -- libs
    -- noisium-merged-2.0.0+mc1.20.x.jar

common
  -- build
    -- libs
      -- noisium-common-2.0.0+mc1.20.x-sources.jar
      -- noisium-common-2.0.0+mc1.20.x-transformProductionFabric.jar
      -- noisium-common-2.0.0+mc1.20.x-transformProductionForge.jar
      -- noisium-common-2.0.0+mc1.20.x.jar

fabric
  -- build
    -- libs
      -- noisium-fabric-2.0.0+mc1.20.x-dev-shadow.jar
      -- noisium-fabric-2.0.0+mc1.20.x-sources.jar
      -- noisium-fabric-2.0.0+mc1.20.x.jar

forge
  -- build
    -- libs
      -- noisium-forge-2.0.0+mc1.20.x-dev-shadow.jar
      -- noisium-forge-2.0.0+mc1.20.x-sources.jar
      -- noisium-forge-2.0.0+mc1.20.x.jar

Note that the file you want to publish is noisium-merged-2.0.0+mc1.20.x.jar. This is a valid mod that can be consumed by either Fabric or Forge and, consequentially, by mc-publish. Everything else is just temporary build artifacts. So, the primary glob should only match that one file.

Let's see what actually happens using DigitalOcean's Glob Tool. And whoops, we have 6 matches instead of 1! Glob tools do not order matches they return. Well, at least there's no strict specification on that matter - implementations are free to do whatever they want. That's why it's crucial for the primary glob to match only one file - the file your users need to download to enjoy your mod. And in your case something like noisium-common-2.0.0+mc1.20.x.jar tries to sneak as a primary file instead. That's why mc-publish is not able to find a metadata file - there's none, because all those jars are not, in fact, valid mods.

So, yeah, everything above is just a very long way to say that this is just a small skill globbing issue. To fix it, I recommend using these globs:

files: |
  build/libs/!(*-@(dev|sources|javadoc)).jar
  **/build/libs/*-@(sources|dev|javadoc).jar

Check with the Glob Tool I mentioned above to see which files they match and if this is the result you are expecting! :)

I could also not merge the JARs and use 2 publish workflows.

Let me enter my salty arc here for a second - just continue doing that :D But yeah, I understand that amalgamating JARs and creating new problems for yourself and others in the process, while simultaneously forcing players to download larger binaries for no good reason, is The New Cool Thingâ„¢ in the modding community, so I need to address it anyways. Thus, let's get back to discussing better support for merged JARs on mc-publish's side.

Due to the implementation details, mc-publishcurrently treats multiloader JARs as Fabric mods. While mc-publish is not entirely wrong on that matter, it's not entirely right either. What I can do is read all the metadata files present in a provided JAR and then, as you suggested, "merge" them. However, it's not as easy as it sounds. For example, merging something like a synthetic loaders property is easy - we need a union here, i.e., a mod supports all mod loaders defined across different metadata files. However, what about dependencies? Should we use a union there too? Okay, let's imagine that we do. The other day, the author of "Friends & Foes" asked how they could disable auto-detection of dependencies because they didn't like Fabric API being listed as one for their multiloader mod. And they have a point - seeing Fabric API as a required dependency for a mod, which (Neo)Forge should be able to load, is definitely strange. So, an intersection then? Or maybe even a complement?! I have no good answer to this question, because every time I try to think of one, I keep getting back to this innovative yet simple solution - just split the JAR and distribute Forge/Fabric/Whatever versions of your mod separately :D

You are a pretty longtime mc-publish user, so I would love to hear your thoughts on the topic :)

Steveplays28 commented 3 months ago

Hi Kir-Antipov, thanks for the super detailed response! I'll read through it in full again and post a more lengthy response this weekend, and try out your suggestions.

Massively appreciated. Also, sorry about editing out the links to my project, rewrote the issue to be clearer and didn't think of adding the links back in.

Good to see MC Publish supports merged JARs though, even if they're recognized as Fabric JARs, I think that's going to work super nicely for my usecase! Thanks again.

Steveplays28 commented 3 months ago

I'm not entirely sure on merged JARs either yet, but so far it seems to be working well for me. It does increase the complexity of dependency listings and supported game versions (e.x. NeoForge not supporting 1.20 and below, but Forge being able to, and I still don't quite know how to handle modloader specific APIs). In my case this all works out nearly perfectly so I don't experience many of the downsides at all, but I can see this being a bit of an issue in the case of complex mods.

And of course, the increased JAR size can be an issue, especially with bigger mods, such as Distant Horizons. I think it's quite project-dependent. It can help visibility/discoverability of the mod and ease of install quite a bit I'd imagine. All in all, there's upsides and downsides. I'll be trying it out for a while and seeing how it goes. If I run into any issues I'll look into solutions. We'll see what happens!

Steveplays28 commented 3 months ago

Hi again, might switch away from Forgix due to some issues with JiJ dependencies. Say I have 3 versions of my mod, one for each modloader that's supported, for 1 Minecraft version. Is it possible to release all the artifacts for those in one GitHub release? Does mc-publish simply put the JARs into the existing release?

I'm not sure if that's a good way to handle it either way, I'm fine with having 1 GitHub release per modloader per version as well.

Kir-Antipov commented 2 months ago

It does increase the complexity of dependency listings and supported game versions [...] And of course, the increased JAR size can be an issue [...]

That's why I'm skeptical about seriously using a jar merging tool like Forgix. You seem to agree with my points about it bringing unnecessary overhead (even though for some folks this looks like nitpicking). However, the only positive outcome of using such a tool that you mentioned, i.e., increasing discoverability and ease of installation for users, is disputable, in my honest opinion. How do I, as a user, discover and install mods now? I go to Modrinth, search via keywords describing my interest, filter the results by the mod loader of my preference, which is Fabric, then click on an entry that grabs my attention, and click the download button on one of the latest versions that has my mod loader listed. And in case the mod uses Forgix or something like that... nothing changes; the whole process is literally the same for me, except now I download a slightly bigger binary (once again, some developers would argue that's not a big deal because "memory is cheap," but those clearly don't acknowledge that non-"first world" countries do, in fact, exist). Also, somebody did mention that it's easier for them to test their own mod in a non-dev environment; however, since you mention that you now have some development-related issues, I cannot call it a good QoL tool for developers either.

So, yeah, I still don't see a good reason to make your workflow more complex and now worry if your otherwise perfectly functional mod was correctly patched and merged by a new third-party tool, because it brings little to no benefits to you and no benefits at all to the players.

However, I will still implement proper support for such scenarios because trends, even if they make no practical sense, die hard XD

Say I have 3 versions of my mod, one for each modloader that's supported, for 1 Minecraft version. Is it possible to release all the artifacts for those in one GitHub release? Does mc-publish simply put the JARs into the existing release?

Yup, it does. mc-publish is designed (well, at least this is one of my main goals with this project) to produce results that users would expect. If it doesn't in your case, you either have a pretty esoteric setup, or it's worth creating an issue :)

In your case, what would you expect to happen if you asked mc-publish to publish to an already existing release? I don't think you are expecting an exception here, because why even try if it would end up unsuccessfully anyway? Rather, mc-publish should update the existing release - update the name and the specified changelog, if any, and attach new artifacts to it, shouldn't it? And so it does exactly that :)

Steveplays28 commented 2 months ago

Yup, it does. mc-publish is designed (well, at least this is one of my main goals with this project) to produce results that users would expect. If it doesn't in your case, you either have a pretty esoteric setup, or it's worth creating an issue :)

Ah neat, thanks. I wasn't sure what to expect so I thought I'd double check. Currently have it set up with a different action to make 1 GitHub release for each modloader, but I'll simplify it to only use mc-publish if that just adds to the existing release with the same tag.