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

Remove unneeded dependencies #119

Closed
HyperSoop opened this issue Dec 23, 2022 · 9 comments
Closed

Remove unneeded dependencies #119

HyperSoop opened this issue Dec 23, 2022 · 9 comments

Comments

@HyperSoop
Copy link

Take this version for example.
It suggests Lithium, Sodium, and Memory Leak Fix. This is completely wrong as this tag should only ever be applied to mods which your mod integrates with unlocking actual additional content.
It requires Cloth Config API. This is terribly wrong as there actually isn't a requirement for it for the mod to run - it runs flawlessly without it. This also confuses many applications such as packwiz or ATLauncher forcing the user to install an actually unneeded dependency.

@offbeat-stuff
Copy link
Contributor

The suggests is for mods that give better performance, you could also call it a bit of self advertising

@offbeat-stuff
Copy link
Contributor

sodium is under recommends

@offbeat-stuff
Copy link
Contributor

diff --git a/src/main/resources/fabric.mod.json b/src/main/resources/fabric.mod.json
index 41ae79d..7a3f315 100644
--- a/src/main/resources/fabric.mod.json
+++ b/src/main/resources/fabric.mod.json
@@ -30,17 +30,20 @@
   "depends": {
     "fabricloader": ">=0.14.10",
     "minecraft": ">=1.19.3",
-    "java": ">=17",
-    "cloth-config": ">=9.0.94"
+    "java": ">=17"
   },
   "recommends": {
     "memoryleakfix": "*",
-    "sodium": ">=0.4.5"
+    "sodium": ">=0.4.5",
+    "cloth-config": ">=9.0.94"
   },
   "suggests": {
     "memoryleakfix": "*",
     "lithium": ">=0.8.0"
   },
+  "breaks": {
+    "cloth-config": "<9.0.94"
+  },
   "custom": {
     "mc-publish": {
       "curseforge": 630104,

would this be sufficient?

@HyperSoop
Copy link
Author

I think all mentions of any other mods should be removed, it's a confisung situation overall but anyway

@FxMorin
Copy link
Owner

FxMorin commented Dec 24, 2022

In the latest build I moved sodium into suggests. I do recommend these mods as it has a large impact on performance

@HyperSoop
Copy link
Author

It's just not how you'd excpect those fields to be used. Most users wouldn't even notice, but it would look weird for power users and developers who already know about the whole thing. You ideally should only use those fields for mods unlocking additional functionality

@HyperSoop
Copy link
Author

(Why is cloth config a required dependency, still? It works fine without it)

@FxMorin
Copy link
Owner

FxMorin commented Jan 27, 2023

does it?

@FxMorin FxMorin reopened this Jan 27, 2023
@HyperSoop
Copy link
Author

It appears MoreCulling has Cloth Config JiJd, so you should probably remove this as a required dependency and put it as "embedded" instead

FxMorin added a commit that referenced this issue Mar 17, 2023
Fixes #119

(cherry picked from commit 453b939)
FxMorin added a commit that referenced this issue Mar 17, 2023
Fixes #119

(cherry picked from commit 453b939)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants