Respect the archivesName #46
Conversation
|
Default maven path before this change: |
There was a problem hiding this comment.
Per the Gradle User Guide, the default value of artifactId is based off Project#getName() rather than the base archive name.
Should we consider setting the project name to the mod ID in addition to (or perhaps in replacement of) this change?
|
I wasn't sure if there would be other implications of fiddling with the project name (does it change the name displayed in an IDE?) Also, the project name is seldom the modid, rather it is usually the mod name without spaces (due to repository naming schemes). |
It does; if I recall correctly, IDEA displays the folder name and then in parentheses the project name (or was it the other way around?).
Good point. I think, though, the more pertinent issue (as I've just remembered) is a problem with case-insensitive file-systems, where a case difference between the mod ID and the repository/folder name might cause issues. (I think I remember having experienced this issue on Windows when I used Eclipse, and possibly with IDEA.) |
|
The motivating case was when I was trying to set the archivesName like so: This is, I think, fairly typical, where the project id/name is the mod id (or folder name?) but the archive name is set to a capitalized/de-spaced friendly mod name, with Minecraft version included (so that it's part of the filename but not part of the actual mod version). Not sure if the MDK should do that by default too, but a random sampling of curseforge mods suggests it's commonplace. Side note: the Forge MDK published a sources jar by default, while the NeoForge MDK does not. That's probably not important for most mods but they're nice to have for any mod that gets addons made for it (which are the type to end up published in mavens in the first place), so perhaps also might be helpful by default anyway? |
|
I think I can agree with those changes: creating/publishing the sources JAR by default (via |
|
How's this look? I changed the archive name to use the project name by default (rather than the mod id, which may or may not be the same), mostly because the |
| java { | ||
| withSourcesJar() | ||
| } |
There was a problem hiding this comment.
Move this above the comment (which is for the publishling block), and add its own comment explaining that this enables the creation of the -sources JAR.
build.gradle
Outdated
| @@ -113,7 +113,7 @@ tasks.withType(ProcessResources).configureEach { | |||
| minecraft_version : minecraft_version, minecraft_version_range: minecraft_version_range, | |||
| neo_version : neo_version, neo_version_range: neo_version_range, | |||
| loader_version_range: loader_version_range, | |||
| mod_id : mod_id, mod_name: mod_name, mod_license: mod_license, mod_version: mod_version, | |||
| mod_id : mod_id, mod_name: mod_name, file_name: file_name, mod_license: mod_license, mod_version: mod_version, | |||
There was a problem hiding this comment.
I don't think we need to supply the file name as a property to the mods.toml properties expansion. Note that mod_group_id is also not provided as a property here.
| @@ -14,7 +14,7 @@ repositories { | |||
| } | |||
|
|
|||
| base { | |||
| archivesName = mod_id | |||
| archivesName = "${file_name}-${minecraft_version}" | |||
There was a problem hiding this comment.
This PR has set the base archives name to be the name of the published module -- however, you're now publishing the artifact at com.example.examplemod:ExampleMod-1.20.4. By convention, module names are lowercase, so this is not great. The inclusion of the minecraft version in the module name is also not optimal - it means that someone could depend on both ExampleMod-1.20.4 and, say, ExampleMod-1.20.2 and gradle would just pull in both -- whereas publishing every version of a mod under the same module name wouldn't allow this to happen.
The other solution to the latter problem is the use of capabilities but that is way to complicated for the MDK.
There was a problem hiding this comment.
Almost every mod file I've ever seen has published using WordCase names.
There was a problem hiding this comment.
Yes, and that's fine for CF/MR publishing but a bad pattern for maven publishing (which is all this block is for) because it's the opposite of what is done in the wider gradle and maven world. Let's set a good example instead of propagating a problematic pattern. The use of the MC version in the module name is an even bigger issue, as it can lead to issues during dependency resolution and getting two copies of a module.
| @@ -29,6 +29,8 @@ loader_version_range=[2,) | |||
| mod_id=examplemod | |||
| # The human-readable display name for the mod. | |||
| mod_name=Example Mod | |||
| # The mod filename, usually the mod_id or the mod_name WithoutSpaces | |||
| file_name=ExampleMod | |||
There was a problem hiding this comment.
See above: the file name can be this for the local file and that's fine, but for the published module (the bit in the publishing block) it should be a normal, conventional module name -- in this case, probably com.example.examplemod:examplemod. A simple way to do this is to set the rootProject.name in the settings.gradle.
I'm not a gradle expert, so I'm not sure if there's a better way to do this, but at least with userdev 7.0.97 (as is referenced here), changes to the
base.archivesNamedon't actually do anything without a line like this.