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

Dependency Overrides are confusing #447

Open
OroArmor opened this issue Aug 10, 2024 · 7 comments
Open

Dependency Overrides are confusing #447

OroArmor opened this issue Aug 10, 2024 · 7 comments
Labels
discussion proposal for a change enhancement new feature or improvement help wanted extra attention is needed

Comments

@OroArmor
Copy link
Member

A couple issues with dependency overrides:

  1. Fuzzy matching is only enabled for path based overrides. Id and pattern overrides need to match exactly. Maybe making this an option in the json that has to be enabled.

  2. Documentation in the loader wiki is currently out of date. It does not mention the path or pattern options at all.

  3. Documentation example aren't useful. Providing some complete override files for common examples (ie replacing a dep, removing, etc) would make things easier for users.

  4. Maybe also some tests to provide some better coverage.

@OroArmor OroArmor added enhancement new feature or improvement help wanted extra attention is needed discussion proposal for a change labels Aug 10, 2024
@AlexIIL
Copy link
Contributor

AlexIIL commented Aug 10, 2024

This comment only addresses the first point. I agree with the other 3.

Generally, dependency overrides are expected to be commonly used for fixing broken or slightly incorrect dependencies.

The most common examples are:

  • Overriding a minecraft dependency, if a mod was built for an earlier version of minecraft but the latest version only has minor changes.
  • Overriding a fabric loader dependency, if the changes in the latest version are unnecessary.

The "simplest" way to fix these, as an end user, would be to remove every mod dependency on minecraft, with a pattern matching every mod. Such an override would look like this:

{
  "pattern": ".+",
  "fuzzy": true, // Theoretical field
  "depends": {
    "remove": "minecraft"
  }
}

This would work for most mods, but would be horribly broken for multi-version mods - where a single mod depends on a "compatibility layer" mod, and the mod includes multiple compatibility layer versions. For example a mod might look like this:

"id": "my-mod",
"version": "1.4.1",
"depends": { "my-compatibility-layer" }

and include 3 mods like this:

"id": "my-compatibility-layer-mc-1.20",
"version": "1.4.1",
"provides": "my-compatibility-layer",
"depends": { "id": "minecraft", "version": "=1.20" }
"id": "my-compatibility-layer-mc-1.21",
"version": "1.4.1",
"provides": "my-compatibility-layer",
"depends": { "id": "minecraft", "version": "=1.21" }
"id": "my-compatibility-layer-mc-1.21.1",
"version": "1.4.1",
"provides": "my-compatibility-layer",
"depends": { "id": "minecraft", "version": "=1.21.1" }

Now, if the above dependency override was applied to the whole mod the actual compatibility layer loaded would be random. (There would probably be a warning like Two options have identical weight when choosing between them! in the log indicating this).

This is the primary reason why I don't like the idea of allowing both fuzzy and pattern matching.

@AlexIIL
Copy link
Contributor

AlexIIL commented Aug 10, 2024

My main issue with dependency overrides is that we expect the end user to know quite a lot about multiple json formats – some of which differ in very subtle, but important, ways.

  • When overriding a quilt mod the user needs to know the original depdnency. For simple cases (like minecraft depdnencies) this is generally just “depends”: { “id”: “minecraft”, “version”: “=1.21” }. Some mods might have a more complex format, and so the user needs to open the mod file, find the quilt.mod.json file, open it with a text editor, and then find the relavent section, then copy it over into the dependency override, just to remove it. (At this point a user is more likely to just remove it from the mod’s json file, then save it).
  • If the user wants to override a fabric mod json then they need to translate the fabric json format into a quilt format – which has subtle differences when specifying versions, and might not work quite how a user expects. (For example a fabric versions array is “all”, but quilt loader versions array is “all” by default, and needs an object with “all” specified explicitly. In addition the handling of “bare” versions – like “1.20” is handled differently in fabric vs quilt – fabric treats it as “=1.20”, but quilt treats it as “^1.20”. Plus, fabric allows versions to be separated by spaces ">=1.19.0 <1.19.4", but quilt requires these be translated into an object { “all”: [ “>=1.19.0”, “<1.19.4” ] })

I think a much better solution is to offer a GUI in quilt-loader to modify the override json file for them – that way quilt-loader can deal with all the json translation, and the user can just view all dependencies that mods have, and modify them easily. (I’m not sure exactly what this gui should look like though – using the existing tree system would probably be the easiest, but not look particularly good).

(Adding a gui doesn't stop us from better documenting the format though)

@OroArmor
Copy link
Member Author

This is the primary reason why I don't like the idea of allowing both fuzzy and pattern matching.

Would making fuzzy work for only id then make sense? Or could that still have issues with what you suggested.

@AlexIIL
Copy link
Contributor

AlexIIL commented Aug 10, 2024

It depends on how a compatibility layer mod is setup - if they differ only in version (id is my-compatibility-layer, version is 1.4.5-1.20, 1.4.5-1.21, 1.4.5-1.21.1) then it would cause problems if we were trying to load on minecraft 1.20 - the solver would see the mods having the same dependencies (as the minecraft dependency was removed) so would incorrectly pick 1.4.5-1.21.1

@OroArmor
Copy link
Member Author

Hm, what if fuzzy id only worked if there was just one load option providing the id (print a warning that it found multiple potential matches or something)? Might make things more complicated (and is that even possible with the solver setup?) but could make the feature more accessible to users.

@OroArmor
Copy link
Member Author

I think a much better solution is to offer a GUI in quilt-loader to modify the override json file for them – that way quilt-loader can deal with all the json translation, and the user can just view all dependencies that mods have, and modify them easily. (I’m not sure exactly what this gui should look like though – using the existing tree system would probably be the easiest, but not look particularly good).

My main issue against a GUI is that I worry people would get over zealous with their overrides to make things work and that would cause more issues. I do think it should be used sparingly.

You could also only enable the GUI with a specific set of ids (say Minecraft, loaders, qsl/fapi), but hardcoding loader like that probably isn't a good idea, or with a flag. You could also have something show up if there is such a conflict with a link to the dependency override wiki page (with its overhaul) and that would explain what they are and that they should only be used if the user knows what they are doing (aka Here be dragons!)

@AlexIIL
Copy link
Contributor

AlexIIL commented Aug 13, 2024

Honestly, I'm not that worried about people over-using dependency overrides to get around genuinely correct dependencies - because they will quickly learn than no, just adding an override for an old mod on its minecraft dependency doesn't let you launch the game with that mod. (Plus, if a user added a dependency themselves, and it crashes immediately afterwards, they will probably be aware of what they did).

I'm more worried by users trying to learn (potentially) a new file format (json), the specification for that file, and trying to do something that has consequences that aren't easy to understand (like "override all minecraft dependencies") and it crashing in odd or unexpected ways, or if they added a new mod much later that has these complex versioning system.

What does worry me about a gui is users removing genuinely correct "breaks" clauses that don't cause crashes, but do cause other obvious issues that cause uses to complain to mod developers that added those breakages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion proposal for a change enhancement new feature or improvement help wanted extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants