Custom Enchantment API#401
Conversation
|
Please excuse the inactivity. I am still working on this, just not here. This is mainly because I don't have a lot of experience with Git when it comes to properly working with patches and merging them when they get added to the main branch. While working on this, I somehow managed to locally reorder commits when merging from upstream. Sadly, there's not much documentation regarding this (things like making sure that patches are up-to-date and properly applied while working on a PR on a separate branch), and there's no way for me to get in contact to get proper support for this. |
|
Okay, I made some progress regarding custom enchantments: 2021-06-25_11-04-56.mp4But before I can move any further, I need to ask the team (mainly @BillyGalbreath @jpenilla) about their thoughts on this PR. Will this get merged once it's finished, or is it something that will be on hold forever (Paper moment™)? What is the general opinion of the team regarding this? I remember speaking earlier with Billy and it appeared that they were okay with this PR, but I just want confirmation if that is still the case. |
|
This PR looks great so far, I'm excited to see what you create. As a member of the team, I think that you should keep going on this PR and that it will almost definitely get merged (with some critique). |
NickAcPT
force-pushed
the
feat/custom-enchantments
branch
from
64dbad7 to
58ae05b
Compare
|
Oops, accidentally closed this by mistake when I reset with Purpur's branch. |
| + // Purpur start - intercept lore and display custom enchantments | ||
| + if (ItemStack.shouldShowInTooltip(stack.getHideFlags(), ItemStack.TooltipPart.ENCHANTMENTS)) { | ||
| + java.util.Set<Map.Entry<net.minecraft.world.item.enchantment.Enchantment, Integer>> enchantments = net.minecraft.world.item.enchantment.EnchantmentHelper.deserializeEnchantments(stack.getEnchantmentTags()).entrySet(); | ||
| + for (Map.Entry<net.minecraft.world.item.enchantment.Enchantment, Integer> enchantment : enchantments) { | ||
| + if (enchantment.getKey() instanceof net.pl3x.purpur.enchantments.CraftCustomEnchantment customEnchantment) { | ||
| + org.bukkit.inventory.ItemStack itemStack = stack.asBukkitMirror(); | ||
| + java.util.List<net.kyori.adventure.text.Component> loreList = itemStack.lore(); | ||
| + if (loreList == null) loreList = new java.util.ArrayList<>(); | ||
| + | ||
| + loreList.add(customEnchantment.getCustomEnchantment().displayName(enchantment.getValue()).decoration(net.kyori.adventure.text.format.TextDecoration.ITALIC, false).append(net.pl3x.purpur.enchantments.CraftCustomEnchantment.customEnchantmentMagicSuffix)); | ||
| + | ||
| + itemStack.lore(loreList); | ||
| + } | ||
| + } | ||
| + } | ||
| + // Purpur end |
There was a problem hiding this comment.
I understand that this might be a bit "sketchy", if there's any other ideas that don't involve me messing with raw NBT, let me know.
| + // Purpur start - intercept lore and remove custom enchantments if they come from itemstacks | ||
| + org.bukkit.inventory.ItemStack bukkitStackMirror = itemstack.asBukkitMirror(); | ||
| + if (ItemStack.shouldShowInTooltip(itemstack.getHideFlags(), ItemStack.TooltipPart.ENCHANTMENTS)) { | ||
| + java.util.List<net.kyori.adventure.text.Component> loreList = bukkitStackMirror.lore(); | ||
| + if (loreList != null) { | ||
| + loreList.removeIf(c -> { | ||
| + net.kyori.adventure.text.Component customEnchantmentMagicSuffix = net.pl3x.purpur.enchantments.CraftCustomEnchantment.customEnchantmentMagicSuffix; | ||
| + return c.contains(customEnchantmentMagicSuffix, net.kyori.adventure.text.Component.EQUALS); | ||
| + }); | ||
| + bukkitStackMirror.lore(loreList); | ||
| + } | ||
| + } | ||
| + // Purpur end |
There was a problem hiding this comment.
I understand that this might be a bit "sketchy", if there's any other ideas that don't involve me messing with raw NBT, let me know.
| - }).then(Commands.argument("targets", EntityArgument.entities()).then(Commands.argument("enchantment", ItemEnchantmentArgument.enchantment()).executes((context) -> { | ||
| + }).then(Commands.argument("targets", EntityArgument.entities()).then(Commands.argument("enchantment", ItemEnchantmentArgument.enchantment()).suggests(SUGGEST_ENCHANTMENTS).executes((context) -> { // Purpur - Make enchantment suggestions server side |
There was a problem hiding this comment.
This change is required to let the client know that the server is proving the completion. Client still shows command in red, although the enchantment is valid server-side.
There was a problem hiding this comment.
You will probably have to edit brigadier node for enchant command and redo the argument completion for that specific argument. Shouldn't be too hard afaik
| + }, | ||
| + TOOL { | ||
| + @Override | ||
| + public boolean canEnchant(Item item) { | ||
| + return item instanceof net.minecraft.world.item.ShovelItem || item instanceof net.minecraft.world.item.PickaxeItem || item instanceof net.minecraft.world.item.AxeItem || item instanceof net.minecraft.world.item.HoeItem; | ||
| + } |
There was a problem hiding this comment.
This was added to bring the EnchantmentCategory enum into parity with it's NMS version
| + //Invisible suffix used to skip custom enchantment lore lines | ||
| + public final static net.kyori.adventure.text.Component customEnchantmentMagicSuffix = net.kyori.adventure.text.Component.space().color(TextColor.color(0xfa02ff)).append(net.kyori.adventure.text.Component.space().color(TextColor.color(0x26b8ff))); |
There was a problem hiding this comment.
This is a bit questionable, I know.
This is here because the lore needs to be filtered when reading items because creative players override the items on the server when interacting with them.
There was a problem hiding this comment.
Maybe see where creative packets are parsed and do some funky filtering logic there? Seems like filtering this out asap could be a good idea, would love some input from others tho.
There was a problem hiding this comment.
Items are read and written to the protocol in multiple places.
I think having the filtering/processing being done once on the read/writeItemStack method is for the best since that means that we only have to do it in one central place. I'll be waiting for other reviewers to get their opinion on this.
(cc: @YouHaveTrouble)
|
Closing PR as development of these changes is now being done downstream. |
3 participants
This PR aims to allow plugins that depend on the Purpur API to create custom
NamespaceKeybased enchantments.As it's clear by the PR status, this is still very much WIP and I'd like to receive feedback along the way. More so when it comes to git stuff, as I'm not very used to rebasing and reordering commits.
Goals
There are a few goals that I want this PR to achieve before it gets merged:
Considerations
Since this feature is supposed to be flexible for a lot of use-cases, there are some things that need to be considered:
IsIt is on a purpur package now.org.bukkit.enchantments.customeven good enough? Should it be on a Purpur package?Should plugins have to implement a separate interface or extend from a Bukkit class? Should ourCustom Enchantments extend ourCustomEnchantmentclass/interface extend/implement any Bukkit class/interface?CustomEnchantmentclass and are registered within the server. OurCustomEnchantmentclass extends Bukkit's Enchantment class.Show custom enchantments follow the same layout as regular enchantments?Should this change introduce a way to modify the layout of the enchantments as a configuration setting? Custom Enchantments can have their display name fully customised. Still unsure about the second question.