-
-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Loot Table API #54
Loot Table API #54
Conversation
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/LootEntryTypeRegistry.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/mixin/LootManagerMixin.java
Outdated
Show resolved
Hide resolved
...y/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/event/LootTableLoadingCallback.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootTableBuilder.java
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/LootEntryTypeRegistry.java
Outdated
Show resolved
Hide resolved
Add javadoc for QuiltLootPoolBuilder and QuiltLootTableBuilder, use a context object in LootTableLoadingCallback, fix checkstyle errors
...y/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/event/LootTableLoadingCallback.java
Show resolved
Hide resolved
...y/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/event/LootTableLoadingCallback.java
Outdated
Show resolved
Hide resolved
...y/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/event/LootTableLoadingCallback.java
Outdated
Show resolved
Hide resolved
…/event/LootTableLoadingCallback.java Co-authored-by: ADudeCalledLeo <7997354+Leo40Git@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a hopefully helpful review. I see that this API has inherited some of my design mistakes from Fabric's loot API, so I encourage looking at the v2 API here for a much better design: https://github.com/Juuxel/fabric/tree/c72ff1311e686e412691d4344b99bf5677c44bcf
I've detailed the specifics in the comments, but just copy-pasting Fabric's loot API a) includes stuff that Mojang has since exposed and b) misses some stuff added later like bonusRolls
.
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/LootEntryTypeRegistry.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/LootJsonParser.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootPool.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootPoolBuilder.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootTable.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootTableBuilder.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootPoolBuilder.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/mixin/LootPoolMixin.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootTable.java
Outdated
Show resolved
Hide resolved
Thanks for the suggestions, I have updated the pr according to them |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really nice now, just some minor comments
library/data/loot_table/src/testmod/java/org/quiltmc/qsl/loot/test/LootTableTestMod.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootPoolBuilder.java
Outdated
Show resolved
Hide resolved
library/data/loot_table/src/main/java/org/quiltmc/qsl/loot/api/QuiltLootPoolBuilder.java
Outdated
Show resolved
Hide resolved
Co-authored-by: ADudeCalledLeo <7997354+Leo40Git@users.noreply.github.com>
Hi--this PR seems to be unmaintained right now. I'm converting it to a draft to signify that it's not in a mergable state. Feel free to mark it as ready for review once you begin working on this again. |
Due to inactivity and plus the fact that this port of Fabric Loot Tables API v1 would have been made obsolete by the new Fabric Loot API v2 (and plus, direct ports are to be discouraged unless there's a reason for them), I'm closing this PR so it can be succeeded by another one |
A port of Fabric's Loot table API with a few name changes to match current Minecraft code