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

Druidcraft using poor Forge versioning syntax #119

Open
ReidZ3 opened this issue May 13, 2021 · 8 comments
Open

Druidcraft using poor Forge versioning syntax #119

ReidZ3 opened this issue May 13, 2021 · 8 comments

Comments

@ReidZ3
Copy link

ReidZ3 commented May 13, 2021

General Information

Describe the bug:
In its mods.toml file, Druidcraft is not using valid Forge versioning syntax as specified here. Consequently, a crash will and does occur whenever a mod lists Druidcraft as an optional dependency while using proper versioning syntax on Forge v36.1.10+. Please see "Additional context." Additionally, mcversion in mods.toml is out of date and says "[1.16.3,]" rather than "[1.16.5]."

To Reproduce:

  1. Download Forge v36.1.10 or newer
  2. Install Druidcraft v0.4.52
  3. Install Scythd v0.1.8
  4. Run instance
  5. Crash

Expected behavior: Not crash lol.


Environment Versions

Mystic Mods Versions: None

Other Versions:

  • Conflicting mod (if regarding mod integration): N/A
  • Forge: 36.1.10+
  • Minecraft: 1.16.5
  • Modpack (if available): N/A

Logging Information

Crash Report (if available):
crash.txt via Gist

Latest Log (if necessary):
log.txt via Gist (not really necessary, but 🤷‍♂️)


Additional Information

Screenshots (if available): None

Additional context (optional):
One very real example of this bug (and the way I stumbled across it) is the mod: Scythd. Scythd lists Druidcraft as an optional dependency with versionRange="[1.16.5-0.4.52,)". The Forge version interpreter identifies Druidcraft's version as being lower due to its mods.toml file stating version="0.4.52" when it should state version="1.16.5-0.4.52". This results in Forge throwing an error and a crash occurring.

@noobanidus
Copy link
Collaborator

That's interesting. I think this might be a build-script error, as the version variable in the gradle properties shouldn't also be specifying the Minecraft version for ease of use -- it should be glued together at some point in the future. Case in point, my noobutil library results in artifacts which I'm pretty sure properly glue in the Minecraft version.

It may be a matter of comparing the build.gradle implementations to see what might be missing, or it might be that the version variable in the gradle.properties should be mod_version or something similar, and that it is automatically overwriting whatever is being set-up automatically.

@noobanidus
Copy link
Collaborator

I'd also like to note that this "valid Forge syntax" you speak of is a guideline at best and is by no means enforced. It's really on the author of the mod in question to ensure that they're using the correct versioning of the mod they wish to be compatible with in their mods.toml.

I note this "crash" also occurs with The Undergarden, indicating that the versioning system you speak of isn't as widely spread as you might think.

@ReidZ3
Copy link
Author

ReidZ3 commented May 17, 2021

I believe it is an issue with the build.gradle file as I see nothing inconsistent in gradle.properties as compared to other mods'.

As for your issue with "valid Forge syntax," you are completely correct. I can definitely see how what I said indicates that it is enforced. That was not my intention, and Forge does merely encouraged the use of mcversion-modversion syntax.

I do wish that Forge would add some form of versioning enforcement, as I have seen many mods cause crashes due to this issue. From what I have seen most have begun settling on the mcversion-modversion syntax previously mentioned, likely because of Forge's versioning guidelines.

I would like to point out; however, that due to Druidcraft's major version being less than Minecraft's (0.4.52<1.16.5), if Druidcraft were to use the mcversion-modversion syntax, a crash would never occur due to this issue! I.e., 1.16.5-0.4.52 is considered within the version range of both [1.16.5-0.4.52,) as well as [0.4.52,). Of course, this does introduce the issue that 1.16.5-0.4.52 would be considered to be in the range of [0.4.53,) which would obviously be undesirable.

Nevertheless, switching to mcversion-modversion seems to resolve more issues than it introduces.

@noobanidus
Copy link
Collaborator

noobanidus commented May 17, 2021

Oh, thank you for reminding me to check this.

And while I obviously agree that it's a better versioning system, I just wanted to point out that the authors of mods really ought to check that their dependencies function properly instead of making assumptions about it, as it otherwise leads to situations like this.

EDIT: Yep, it's definitely an issue with build.gradle, ${mod_version} isn't being used correctly as far as I can see. I'm not sure whether this is the standard "latest" forge build.gradle or not, but I've basically just replaced it with the version that I've been using in my projects.

This means the ${version} variable should now always be ${minecraft_version}-${mod_version}. I'm not sure how long until the next release, though, so in the mean-time it would be wise for Scythd to update their requirements if possible.

@noobanidus
Copy link
Collaborator

I think this is resolved but I'll wait on Vulpie's next release to see.

@ReidZ3
Copy link
Author

ReidZ3 commented May 19, 2021

Thanks a bunch! I'm familiar enough with Gradle to tell that build.gradle looked off, but not familiar enough to try to fix it myself. Appreciate the help!

I definitely agree that mod creators need to be careful to make sure their dependencies actually work with the mod in question. In fact, I've got another issue thread under Scythd. I'd previously closed it since I thought the crashing was more of an issue with Druidcraft, but my mind's been changed, plus they're now having the same issue with the mod, Undergarden. So, I'll be reopening that thread. Hopefully, doing so will result in fewer of these crashes going forward. Thanks again for helping out!

@ReidZ3 ReidZ3 changed the title Druidcraft using invalid Forge versioning syntax Druidcraft using poor Forge versioning syntax May 19, 2021
@ReidZ3
Copy link
Author

ReidZ3 commented May 19, 2021

Not super important, but I just noticed that Druidcraft has a mismatch between its published jar's MANIFEST.MF version syntax and its mods.toml version syntax. The first uses mcversion-modversion while the latter only uses modversion (as of now ofc)! I think this is what caused the issue for lunekiska (Scythd's developer). MANY, MANY mods specify their mods.toml version as ${file.jarVersion} which pulls the Implementation-Version property from MANIFEST.MF when Forge does its version comparisons.

Although Forge actually checks the mods.toml file, since so many mod creators refer to the MANIFEST.MF file anyways AND the Implementation-Version should match the mods.toml version in theory, you can usually get away with just checking the Implementation-Version. Ergo, I believe this is that trap for which lunekiska fell and why this error occurred. I wouldn't really blame her for it.

As long as your fix works properly, though, this too should no longer be an issue!

@noobanidus
Copy link
Collaborator

I'll triple-check to make sure the manifest is being setup properly.

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

2 participants