itzg / mc-image-helper

This tool does the complicated bits for the itzg/minecraft-server image
MIT License
59 stars 26 forks source link

consider `relationType` when resolving CurseForge mod dependencies #463

Closed escalobba closed 3 months ago

escalobba commented 3 months ago

When resolving mods through CURSEFORGE_FILES, I occasionally get warnings like this

minecraft-server  | [mc-image-helper] 14:26:08.377 WARN  : The mod file Farmer's Delight Refabricated 2.1.2 - 1.20.1 depends on mod project ID 239197, but that is not listed
minecraft-server  | [mc-image-helper] 14:26:08.379 WARN  : The mod file Farmer's Delight Refabricated 2.1.2 - 1.20.1 depends on mod project ID 696251, but that is not listed
minecraft-server  | [mc-image-helper] 14:26:08.379 WARN  : The mod file Farmer's Delight Refabricated 2.1.2 - 1.20.1 depends on mod project ID 310111, but that is not listed
minecraft-server  | [mc-image-helper] 14:26:08.379 WARN  : The mod file Farmer's Delight Refabricated 2.1.2 - 1.20.1 depends on mod project ID 580555, but that is not listed
minecraft-server  | [mc-image-helper] 14:26:08.379 WARN  : The mod file Frostiful 1.0.10 depends on mod project ID 981400, but that is not listed

Looking through the callback for the modfile resolver in CurseForgeFilesCommand, it looks like it doesn't check against a dependency's relationType to check if it's required or optional and the warning message only indicates that a dependency isn't included.

 if (!requestedModIds.contains(dependency.getModId())) {
    log.warn("The mod file {} depends on mod project ID {}, but that is not listed",
    modFile.getDisplayName(), dependency.getModId()
    );
 }

Could this be configurable in the docker-compose file or maybe add an option to suppress optional dependencies?

itzg commented 3 months ago

It's a warning and not an error for a reason. But you bring up a good point that optional dependencies probably shouldn't be mentioned at all.

itzg commented 3 months ago

Latest image now logs optional dependencies at debug level. I also had the warning for required dependencies resolve the actual mod name instead of just project ID.