-
Notifications
You must be signed in to change notification settings - Fork 22
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
[Enhancement] Add support for merged JARs #114
Comments
mods.toml
and fabric.mod.json
Hi! :) It seems you are a bit confused here.
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 Just to reiterate, 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 Anyway, I looked into your workflow and here's what I found. After 407edef your 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
Note that the file you want to publish is 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 So, yeah, everything above is just a very long way to say that this is just a small 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! :)
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 Due to the implementation details, You are a pretty longtime |
Hi Kir-Antipov, thanks for the super detailed response! Massively appreciated. 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! |
I'm not entirely sure on merged JARs either yet, but so far it seems to be working well for me. 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. |
Hi again, might switch away from Forgix due to some issues with JiJ dependencies. 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. |
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
Yup, it does. In your case, what would you expect to happen if you asked |
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 |
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.
mc-publish/src/program.ts
Lines 105 to 106 in c0f30ad
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
The text was updated successfully, but these errors were encountered: