ModDevGradle icon indicating copy to clipboard operation
ModDevGradle copied to clipboard

Allow using a VersionFactory/Rich Version in for neoForge.version

Open Tslat opened this issue 1 year ago • 2 comments

The build.gradle line for version in the neoForge block explicitly requires a string.

This is a problem for people who use version catalogues, because it doesn't allow resolution of the version artifact from the catalogue, and forces you to explicitly type the version again as a string

E.G.

neoForge {
  version = "21.1.57" // I have to explicitly type the version string here (or use gradle properties, etc..)
}

I want to be able to use the version I already defined in my version catalogue (as is gradle's recommendation)

neoforge {
  version = libs.versions.neoForge // Version catalogue VersionFactory, same as you'd use for a dependency
}

Tslat avatar Sep 22 '24 08:09 Tslat

Note that you can use catalogs it by calling get() on the dependency (see https://docs.gradle.org/current/userguide/platforms.html#ex-using-a-version-declared-in-a-version-catalog)

neoForge {
    version = libs.neoforge.get()
}

Matyrobbrt avatar Sep 22 '24 08:09 Matyrobbrt

Note that you can use catalogs it by calling get() on the dependency (see https://docs.gradle.org/current/userguide/platforms.html#ex-using-a-version-declared-in-a-version-catalog)

neoForge {
    version = libs.neoforge.get()
}

No this returns the required version, not the preferred version

Meaning you can't specify a lower required version than preferred version (which is a problem)

I.E. Using this rich version: image Causes MDG to use 21.1.44, not 21.1.57

and worse than that, it doesn't actually return 21.1.44, it returns [21.1.44,) as this is the string equivalent of that rich version notation, so you end up having to manually parse out the symbols yourself to make it even work

Here's what it looks like when you try that:

Caused by: java.net.URISyntaxException: Illegal character in path at index 60: https://maven.neoforged.net/releases/net/neoforged/neoforge/[21.1.44,)/neoforge-[21.1.44,)-userdev.jar

Tslat avatar Sep 22 '24 10:09 Tslat

Just to be clear here: libs.versions.neoForge (without the .get()!) will always only allow you to get the required version, not the preferred version. The libs.versions.xyz getters return just Provider<String>, not a VersionConstraint; the full rich version is only available either through a version catalog dependency or with the VersionCatalog#findVersion method (so not the generated getters). Nothing that MDG could possibly do would make

neoForge {
  version = libs.versions.neoForge
}

Work as you expect because the information just isn't there in that getter.

lukebemish avatar Apr 16 '25 02:04 lukebemish

If you're looking for an actual solution to your issue -- I make a module dependency in your version catalog, or otherwise obtain the version constraint, and pull the preferred version from that. About the only thing MDG could do is let the setter take a version constraint -- though that still would not allow the syntax you want to work right, since that's just not how version catalog versions work.

lukebemish avatar Apr 16 '25 02:04 lukebemish

How do you even get the VersionConstraint? Like this?

versionCatalogs.named("libs").findVersion("neoForge").get()

We could potentially add a helper method that will perform these steps from the version name "neoForge".

Technici4n avatar Apr 27 '25 10:04 Technici4n

Like that, or from the module dep directly. All that said -- I'm not sure I really see that sort of helper as worth it. You'd lose the type safe ness of the version catalog anyways, etc. -- what Tslat is asking for here just isn't how version catalogs work. That said, having a method that takes a version constraint instead of a string for the neo version is probably fine and quite possibly desired -- someone can use a module dep from the version catalog to get a version constraint with relative ease anyways.

lukebemish avatar Apr 27 '25 17:04 lukebemish

How do you even get the VersionConstraint? Like this?

versionCatalogs.named("libs").findVersion("neoForge").get()

We could potentially add a helper method that will perform these steps from the version name "neoForge".

That loses type safety completely and I honestly don't see the benefit over just using a gradle.properties entry at that point. They're both addressed by strings then.

shartte avatar Apr 27 '25 18:04 shartte

So looking into it: We'd need to remove setForgeVersion(String) and replace it with setForgeVersion(Object) to prevent Groovy from coercing the version. We can't have both setters simultaneously as Groovy refuses to recognize it as a property.

shartte avatar Apr 27 '25 18:04 shartte

How do you even get the VersionConstraint? Like this? versionCatalogs.named("libs").findVersion("neoForge").get() We could potentially add a helper method that will perform these steps from the version name "neoForge".

That loses type safety completely and I honestly don't see the benefit over just using a gradle.properties entry at that point. They're both addressed by strings then.

The whole point is that my versions aren't managed in my gradle.properties. They're managed by a version catalogue. That I want to use without having to have a separate version catalogue entry that's basically just a string specifically for this line

Tslat avatar Apr 27 '25 18:04 Tslat

Right but Tslat that doesn't even work in normal old gradle. The typesafe version getters return a Provider<String> and that it.

lukebemish avatar Apr 27 '25 23:04 lukebemish

You can't have a getter that accepts a VersionConstraint and calls getVersion on it?

Tslat avatar Apr 28 '25 03:04 Tslat

@Tslat I am confused what you are talking about, I think -- if I do libs.versions.xyz, the resulting object is a Provider<String>, not anything related to a VersionConstraint -- where is the VersionConstraint even coming from here?

lukebemish avatar Apr 28 '25 05:04 lukebemish

@lukebemish When I debugged this (by changing the setter to take an Object), I did get a rich version constraint object wrapped in a Provider, but we can't actually do anything about it. We're not going to break the API to change it to setVersion(Object)

shartte avatar Apr 28 '25 08:04 shartte

You definitely get a Provider<String> from libs.versions.xxx. Just getting a VersionConstraint is nontrivial, and at that point you might as well call .version on it?

Technici4n avatar Apr 28 '25 09:04 Technici4n

I am considering closing this issue since we just can't do anything about Gradle not exposing this info using the obvious syntax.

Technici4n avatar May 05 '25 08:05 Technici4n