neoforged / NeoForge

Neo Modding API for Minecraft, based on Forge
https://projects.neoforged.net/neoforged/neoforge
Other
1.06k stars 141 forks source link

[1.20.5+] Replace the GLM master list with a priority system #697

Open Shadows-of-Fire opened 4 months ago

Shadows-of-Fire commented 4 months ago

The Global Loot Modifiers system has one very weird component - the master list located at /data/neoforge/loot_modifiers/global_loot_modifiers.json. This file was introduced to allow modpacks to control the ordering of individual GLMs, but falls short of this goal for the following reasons:

  1. No way to generate it - modpack authors cannot generate the joined file.
  2. Incurs a race condition when adding new content. a. This happens if a modpack overrides the file with a static list, and then adds or updates a mod which adds a new GLM. That new GLM would be invisibly broken by not being in the override list.

It also places a burden on mod authors, as they have to provide this file on top of providing the actual GLM file, increasing the complexity for no gain (due to the above issues).

I propose we replace this master list with a simple priority system on the individual loot modifier objects. This will allow ordering without the pitfalls of the current system, and will mean that adding a GLM only needs one file.

KnightMiner commented 4 months ago

The other purpose of the master list is being able to disable a specific GLM. Unlike recipes which have load conditions, GLMs have loot table conditions which are computed based on the table context. If we removed the master list, I think we would want to also include load conditions on GLMs, which by extension would allow using a GLM from another mod with a load condition on that mod being loaded without triggering datapack errors.

TelepathicGrunt commented 1 month ago

Copying what I said in discord which basically parallels what shadow was saying here. I don’t want the GLM master file. And I don’t want it replaced with another file. The existence of the second file is the whole problem imo. An extra step for everyone to keep track of and annoying to deal with.

My propose solution is to follow the same idea behind biome modifier jsons where they only need 1 file to exist, has a priority in them (generation stage), and can be replaced with a no-op biome modifier (neoforge:none type)

Just copy that to loot modifiers. Add a "priority" optional int field to the loot modifier json that defaults to 1000 if not in the json. Higher numbers is ran first as if higher priority. Lower numbers is ran last. And then add a no-op loot modifier of neoforge:none.

Benefits:

Con:

(At knightminer, don’t we already have a condition field that can be appended to all json files including loot modifiers to only load them if another certain mod is present?)