Minecraftforge: [1.14.3] Unable to modify vanilla LootTable pools

[Wabbit0101]

Minecraft: 1.14.3
Forge: 1.14.3-27.0.50

The LootPool modification APIs are commented out (addEntry & removeEntry). Not sure if they're coming back due to changes in vanilla loot table inner workings. If these hooks are removed, there is no way to modify _existing vanilla loot table pools_. Adding a new pool to a table does not have the same effect, and adding an entire loot table/file also isn't a substitute as that implies a mod must copy the entire existing vanilla table to add one or two entries (unlike tags you can't just define a merge operation for your loot table).

[KnightMiner]

What problem are you having when adding a new pool? I have successfully added one to modify vanilla block drops and entities before.

[Wabbit0101]

A mod's additional pool is something that will be evaluated _in-addition-to_ the original pools. For example altering the fishing treasure by adding a custom pool. My item would be an additional bit of treasure not one-of-the-possible treasures. (This is just an easy example of pools, I know the fishing code will always just return 1 treasure item for this particular case.)

Another case would be adding something to a mob's killed-by-player pool (eg a guardian's or vindicator's drop pool). To ensure my item is dropped I would add my own pool with my own conditions, but if I want either my item or one of the standard items to be dropped, I can no longer do this w/o replacing the entire thing.

So are pools going to be the only way to alter tables programmatically then?

[LexManos]

Yes, it is a known thing that vanilla's data layering mechanics are bad/non-existant.
I am not sure how we're going to adapt to the loot pool changes in vanilla. The system we had before was a MAJOR hack due to vanilla not having unique names.
If someone wants to look into it then we can address it.
however, it's currently a low priority as very few people actually used it. And I would like to redesign the system into something that data drive instead of in code.

So, suggestions are welcome.

[Alpvax]

What about merging the JSON data from all datapacks before loading the tables? Maybe adding a remove operation like we do with tags?

[LexManos]

waves magic wand Done!
No seriously that's magic buzzwords that mean nothing. HOW do you 'merge' things?

[Alpvax]

Sorry, that was an unhelpful comment.
I was thinking of js style merging (i.e. if the keys match, create a new object with all of the subkeys), but recursive (at least for certain keys). Thinking about it that doesn't really work for complex structures like loot pools

[LexManos]

There are no keys...

[Alpvax]

It's JSON... it's all keys. And aren't they referenced by ResourceLocation?

[LexManos]

No it's not all keys, its random object indexes. None of the data inside the table/pools/entries are indexable in any fashion. So there is no way to detect what to merge. Or what to remove/add/modify.
We hacked in a system to generate fake names, but that go exploded in the 1.14 update.
Please... actually put some thought into your next comment.

[Alpvax]

My mistake, I thought the only array was "entries" which could be concatenated, but I just double checked the format and I see the issue now.
Are all pools applied, or is the first one that the condition matches picked? Could we concatenate the pools? Although that still leaves the problem of removing them.

[PrincessOfEvil]

Maybe use some existing way to patch JSON, like RFC 6902?

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.