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

Add mod loading from maven-style repository #282

Closed
dscalzi opened this issue Jul 19, 2020 · 30 comments · Fixed by #470
Closed

Add mod loading from maven-style repository #282

dscalzi opened this issue Jul 19, 2020 · 30 comments · Fixed by #470

Comments

@dscalzi
Copy link

dscalzi commented Jul 19, 2020

Premise:
I have a repository of mods stored maven-style which can be used throughout many different launch configurations. I want to pass the maven identifiers to the mod loader so that it only loads the mods I specify. The loader needs two things:

  1. The path to the maven root: ex. C:/repo
  2. A list of maven identifiers: ex. com.author:mymod:1.1.1;com.author2:anothermod:4.1.2

This feature is supported by other mod loaders. In my opinion the implementation for fabric should look like Forge's 1.13+ system. The fabric implementation should take input from a file so that the cli length limit is not exceeded when loading a large number of mods.

  1. Forge 1.13+ [1.13.2] Feature Request: --modListFile equivalent MinecraftForge/MinecraftForge#5495 (comment)
  2. Forge 1.12- https://github.com/MinecraftForge/FML/wiki/New-JSON-Modlist-format
  3. Liteloader http://develop.liteloader.com/liteloader/LiteLoader/issues/34

If more information is required, please let me know. Looking to add fabric support to a popular launcher I maintain and this would be a prerequisite.

@modmuss50
Copy link
Member

Passing all the mods over the command line might not be ideal solution as I believe (please prove me wrong) some OS's have a max lenght of a command line.

I do see the uses of being able to run with specific mod jars from elsewhere.

@Scotsguy
Copy link

On Windows, commandlines can be up to 8191 characters, Linux has limits around the 2MiB range (according to xargs --show-limits on my machine)

@dscalzi
Copy link
Author

dscalzi commented Jul 19, 2020

I havent had a problem with the command line only interface for forge 1.13, but granted there are not many large modpacks available so it could be a potential issue in the future.

@jordanamr
Copy link

A path to a json file providing the maven identifiers works too.

@ydainna
Copy link

ydainna commented Jul 20, 2020

indeed it would be good

@Yousinski
Copy link

It would be really cool

@modmuss50
Copy link
Member

Im failing to see the benfit of taking in mods maven sytle? Why not just a list of paths?

@jordanamr
Copy link

Im failing to see the benfit of taking in mods maven sytle? Why not just a list of paths?

This make it so launchers and/or toolsets can have a local maven-like repo for caching purposes I guess ?

@jordanamr
Copy link

It is actually useful for a launcher with multiple public modpacks, instead of redownloading the mods that are present in different modpacks, the launcher just maintains a maven-like repo in it's folder and then check if a mod is already in cache instead of redownloading it for each modpack that have that same mod. This also makes instance reinstallation way quicker

@modmuss50
Copy link
Member

The launcher could still do that, why does the loader need to enforce a maven style dir structure when it could just take in a list of paths to jars (that could stored in what ever way the launcher intended)

@jordanamr
Copy link

It does not need to be enforced, it could support more ways of loading mods from a different folder, I am just seeing that the Forge --fml.mavenRoots and --fml.mods works very good.

For example, using the fml style:
--altModsDir "C:\Blabla\repo" --altMods com.author:mymod:1.1.1;com.author2:anothermod:4.1.2

Using a list of paths to jars (considering the loader will scan subfolders too, to make any folder structure works):
--altModsDir "C:\Blabla\repo" --altMods mymod-1.1.1+build.1-1.16.1.jar;anothermod-4.1.2+build.2-1.16.1.jar

Is this what you meant?
The only issue I see is that maven identifiers are smaller than having the whole file name.

@sfPlayer1
Copy link
Contributor

I am not a fan of the maven layout either, it should suffice to specify a path list, absolute or relative to the working dir. Something like --fabric-add-mods /some/path/to/MyMod.jar:/some/path/to/MyOtherMod.jar (note the os specific path separator). This could be complemented by --fabric-args-file /some/file.txt to pass additional program args (fabric or mc) beyond the os limit.

Alternatively you could just hard- or symlink the mods into mods/, which would be quite a bit nicer for the users to handle imo.

@jordanamr
Copy link

jordanamr commented Jul 20, 2020

Alternatively you could just hard- or symlink the mods into mods/, which would be quite a bit nicer for the users to handle imo.

I really don't like this idea... I approve the rest of your comment tho

@modmuss50
Copy link
Member

I think having both methods of passing paths stright over the command line, or via a text file should be fine. The symlink stuff is already there (fixed in 0.9) so can be used if you want to.

@sfPlayer1
Copy link
Contributor

The args file approach I just outlined is a bit more generic, which may have some uses outside this specific feature.

@Dysta
Copy link

Dysta commented Jul 20, 2020

really great idea, hope we will see it one day

@dscalzi
Copy link
Author

dscalzi commented Jul 20, 2020

I would say that loading them maven style should be the way to go. It's a solid standard for storing versioned files in a repository

@Photoelasticimetrie
Copy link

Seems like a great idea! Hoping to have it finished soon

@jordanamr
Copy link

Any news on this @modmuss50? Have you decided on what to do with this issue? :)

@modmuss50
Copy link
Member

It seems like its a well recieved idea, the next stage is for someone (me) to find time and open a pull request with this.

@dscalzi
Copy link
Author

dscalzi commented Aug 2, 2020

Just a quick note, this should take input from a file. I can see issues coming up where the cli length limit is exceeded.

@jordanamr
Copy link

Bump ?

@jordanamr
Copy link

I would love to help implementing this feature, can I get some directions on where to start exactly? I'll gladly try and PR.

@sfPlayer1
Copy link
Contributor

I think I'll get this moving soon, not too sure how PR-worthy it is since the feature itself is quite simple but rather opinionated.

@jordanamr
Copy link

I think I'll get this moving soon, not too sure how PR-worthy it is since the feature itself is quite simple but rather opinionated.

Hello, do you need help in any way ?

@sfPlayer1
Copy link
Contributor

I'm about to touch that code, the refactors are starting to hit closer ;)

@sfPlayer1
Copy link
Contributor

See #470 for what I intend to do for this.

@dscalzi
Copy link
Author

dscalzi commented Jul 17, 2021

@sfPlayer1 I'm concerned with this implementation because if many mods are added it may exceed the command line length limit. Would it be possible to accept an argfile containing the paths also? Whether that be json or new line separated entries doesn't matter too much. As long as the information can be passed from a file.

@sfPlayer1
Copy link
Contributor

The PR implements arg files as well, just giving it @someFile with someFIle containing a\nb has the same effect as providing a:b/a;b in the first place

@dscalzi
Copy link
Author

dscalzi commented Jul 17, 2021

The PR implements arg files as well, just giving it @someFile with someFIle containing a\nb has the same effect as providing a:b/a;b in the first place

Ah I was looking at the PR on my phone and missed that bit. All good, thank you for the addition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants