Skip to content

Implement loot table registry#13657

Draft
Lulu13022002 wants to merge 3 commits intoPaperMC:mainfrom
Lulu13022002:feat/loot-table
Draft

Implement loot table registry#13657
Lulu13022002 wants to merge 3 commits intoPaperMC:mainfrom
Lulu13022002:feat/loot-table

Conversation

@Lulu13022002
Copy link
Contributor

No description provided.

public static org.bukkit.loot.LootTable minecraftToBukkit(ResourceKey<LootTable> minecraft) {
return (minecraft == null) ? null : Bukkit.getLootTable(CraftLootTable.minecraftToBukkitKey(minecraft));
}
private static final org.bukkit.loot.LootTable EMPTY = new CraftLootTable(Holder.direct(LootTable.EMPTY)); // todo expose?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposing this boils down to, I think, if there is a difference between using null and EMPTY in different places. Like its possible there is a distinction in different places, a loot table being set to empty vs not having one. If there is a difference, then we probably need to expose it.

this.key = key;
public static org.bukkit.loot.LootTable minecraftToBukkit(LootTable minecraft) {
if (minecraft == LootTable.EMPTY) {
return EMPTY;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, well ignore my other comment. If we are going to return the EMPTY one, they we probably should also expose it. People need to be able to check if they one they have is the empty one. Does vanilla do a == check in places to compare it? regardless, we need to have an easy way to know if its the empty, or use null to mean empty (don't really like that)

makes it consistent with the upcoming loot context api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Awaiting review

Development

Successfully merging this pull request may close these issues.

2 participants