Add missing information to instances of LootContext

[Daomephsta]

Changes in this PR
This PR adds any missing info to LootContext instances if that info is available. For example, the minecart loot context does not provide the player, though the player may be available. A list of the changes made to each LootContext follows:

• AdvancementRewards LootContext - Added player and luck
• EntityMinecartContainer LootContext - Added looted entity (the minecart) and player
• EntityFishHook LootContext - Added looted entity (the fish hook) and player
• TileEntityLockableLoot LootContext - Added player

Usecases
The additional information allows LootFunctions and LootConditions to provide functionality that they could not provide before, or expands the range of scenarios they can work in. For example, biome dependent fishing loot is made possible by this PR and is demonstrated in the test mod. Additional use cases include depth dependent loot in loot containers and minecarts, and items that change loot from loot containers, minecarts and fishing when in the inventory.

Testing
This PR includes a test mod(disabled by default) which:

• Adds acacia boats to the fishing loot table, with a high weight and a condition that makes the loot only appear in a Savannah biome.
• Adds blaze powder to the simple_dungeon chest loot table, with a high weight and a condition that makes the loot only appear if the player is on fire.
• Adds an advancement that gives the player loot from the simple_dungeon loot table.

A test world is attached to this post. It contains command blocks that help test that this PR works correctly, as well as a pond in the savannah biome. All the command blocks are labeled with their function using signs. Note that the in biome condition checks that biome that the looted entity is in, with fishing the looted entity is the fish hook.

Vanilla Compatibility
As loot tables are solely server-side, this PR does not compromise compatibility between Forge and Vanilla Minecraft. However due to the additional information provided, it does cause certain vanilla loot conditions to work in scenarios that they would not work in in Vanilla MC. By "work" I mean that they will not return early, or they will have a visible effect. A list of these conditions and the scenarios in which their behaviour will change follows:

• entity_scores and entity_properties conditions with "killer_player" as the target will work for advancement loot rewards, minecart loot, chest loot and fishing loot.
• entity_scores and entity_properties conditions with "this" as the target will work for advancement loot rewards, minecart loot and fishing loot.
• killed_by_player will work for advancement loot rewards, minecart loot, chest loot and fishing loot.

Are these changes in behaviour an issue?
Improved Loot Contexts Test World.zip

[stale]

This has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[Daomephsta]

@mezz Any issues with this PR?

[mezz]

I overlooked this before.
Seems fine to me and the changes are small, I wonder why they didn't just include more context by default.

[mezz]

Ah I see, the changes in the vanilla conditions is a problem.
Forge should work with vanilla worlds without any changes.

[Daomephsta]

@mezz The issue goes in the other direction. Vanilla loot tables will work fine with Forge. It's Forge loot tables that will not work in vanilla, but only if they use the mentioned conditions in the mentioned scenarios.

[mezz]

Ok, I see. That is less of an issue, but it does seem like it would be confusing for modders that run across those situations though.
Is it possible to make entity_scores, entity_properties, and killed_by_player smarter? Is there some other workaround?

[Daomephsta]

it does seem like it would be confusing for modders that run across those situations though.

killed_by_player is the only affected condition that would behave oddly. It'll always return true in the mentioned scenarios, unless it's inverted, then it'll return false. Without this patch it'll behave in the opposite manner. The other two affected conditions should behave correctly. I haven't actually tested the affected conditions in the mentioned scenarios, just looked at the code to see which conditions would be affected. I'll try to do that sometime, so we know how they would behave for sure. Considering that using killed_by_player in any of the affected scenarios makes no sense, and that the other conditions should work properly, I doubt that anyone would be confused.

Is it possible to make entity_scores, entity_properties, and killed_by_player smarter? Is there some other workaround?

Loot conditions have no awareness of what table they're in.

[williewillus]

I don't think minecart/fish hook should have "looted entity", you have to remember that that method name is only assigned by MCP and is our guess at which it actually is. But in vanilla, "looted entity" is only used in tables for drops when the "looted entity" is dying and spawning its own drops, iirc.

It's kind of weird to think of fish as the drops of the fish hook, imo.

[williewillus]

"requirements" can be omitted if all of the criteria are needed, as it is in this case.

[Daomephsta]

@williewillus Vanilla passes the player as the looted entity for advancement loot rewards.

[williewillus]

huh, okay. looks good then

[Tmtravlr]

This will be very useful in the mod I'm working on. Nice work!

I could use this to get the position of the loot in every case except when a container generates loot without a player... I can live with that though.

[Daomephsta]

@Tmtravlr I don't think adding new information to the LootContext is in scope for this PR. You could make your own PR for that though.

[stale]

This has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mezz]

I have looked into this more and think it makes sense.
There are some odd caveats for modders to look out for, but generally this seems to be a useful addition.