Item updater #871
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package vg.civcraft.mc.civmodcore.inventory.items.custom; | ||
|
|
||
| import java.util.Objects; | ||
| import java.util.function.Supplier; | ||
| import org.bukkit.NamespacedKey; | ||
| import org.bukkit.inventory.ItemStack; | ||
| import org.bukkit.persistence.PersistentDataType; | ||
| import org.bukkit.plugin.java.JavaPlugin; | ||
| import org.jetbrains.annotations.ApiStatus; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
| import vg.civcraft.mc.civmodcore.CivModCorePlugin; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import vg.civcraft.mc.civmodcore.inventory.items.ItemUtils; | ||
|
|
||
| /** | ||
| * Since Minecraft doesn't [yet] offer a means of registering custom item materials, this is the intended means of | ||
| * defining custom items in the meantime. Keep in mind that each custom item must correlate 1:1 with its key, ie, that | ||
| * custom-item keys should be treated like item materials. Do NOT use custom-item keys as custom-item categories, such | ||
| * as compacted items. You must always be able to receive the same item from the same key. | ||
| */ | ||
| public final class CustomItem { | ||
| public static NamespacedKey CUSTOM_ITEM_KEY = new NamespacedKey(JavaPlugin.getPlugin(CivModCorePlugin.class), "custom_item"); | ||
|
|
||
| private static final Map<String, Supplier<ItemStack>> customItems = new HashMap<>(); | ||
|
|
||
| public static void registerCustomItem( | ||
| final @NotNull String key, | ||
| final @NotNull Supplier<@NotNull ItemStack> factory | ||
| ) { | ||
| Objects.requireNonNull(key); | ||
| Objects.requireNonNull(factory); | ||
| customItems.putIfAbsent(key, factory); | ||
| } | ||
|
|
||
| public static void registerCustomItem( | ||
| final @NotNull String key, | ||
| final @NotNull ItemStack template | ||
| ) { | ||
| registerCustomItem(key, template::clone); | ||
| } | ||
|
|
||
| public static @Nullable ItemStack getCustomItem( | ||
| final @NotNull String key | ||
| ) { | ||
| if (customItems.get(Objects.requireNonNull(key)) instanceof final Supplier<ItemStack> factory) { | ||
| final ItemStack item = factory.get(); | ||
| setCustomItemKey(item, key); | ||
| return item; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Just remember that has-then-get is an anti-pattern: use {@link #isCustomItem(org.bukkit.inventory.ItemStack, String)} | ||
| * or {@link #getCustomItemKey(org.bukkit.inventory.ItemStack)} instead. | ||
| */ | ||
| public static boolean isCustomItem( | ||
| final ItemStack item | ||
| ) { | ||
| return !ItemUtils.isEmptyItem(item) && item.getPersistentDataContainer().has(CUSTOM_ITEM_KEY); | ||
| } | ||
|
|
||
| public static boolean isCustomItem( | ||
| final ItemStack item, | ||
| final @NotNull String key | ||
| ) { | ||
| return key.equals(getCustomItemKey(item)); | ||
| } | ||
|
|
||
| public static @Nullable String getCustomItemKey( | ||
| final ItemStack item | ||
| ) { | ||
| if (!ItemUtils.isEmptyItem(item)) { | ||
| return item.getPersistentDataContainer().get(CUSTOM_ITEM_KEY, PersistentDataType.STRING); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| @ApiStatus.Internal | ||
| public static void setCustomItemKey( | ||
| final @NotNull ItemStack item, | ||
| final @NotNull String key | ||
| ) { | ||
| item.editPersistentDataContainer((pdc) -> pdc.set(CUSTOM_ITEM_KEY, PersistentDataType.STRING, key)); | ||
| } | ||
|
|
||
| public static @NotNull Set<@NotNull String> getRegisteredKeys() { | ||
| return Collections.unmodifiableSet(customItems.keySet()); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| package vg.civcraft.mc.civmodcore.inventory.items.custom; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import org.bukkit.Bukkit; | ||
| import org.bukkit.NamespacedKey; | ||
| import org.bukkit.inventory.ItemStack; | ||
| import org.bukkit.persistence.PersistentDataType; | ||
| import org.bukkit.plugin.java.JavaPlugin; | ||
| import org.jetbrains.annotations.Contract; | ||
| import org.jetbrains.annotations.NotNull; | ||
| import org.jetbrains.annotations.Nullable; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
| import vg.civcraft.mc.civmodcore.CivModCorePlugin; | ||
| import vg.civcraft.mc.civmodcore.inventory.items.updater.ItemUpdater; | ||
| import vg.civcraft.mc.civmodcore.inventory.items.updater.listeners.DefaultItemUpdaterListeners; | ||
| import vg.civcraft.mc.civmodcore.inventory.items.updater.migrations.ItemMigration; | ||
| import vg.civcraft.mc.civmodcore.inventory.items.updater.migrations.ItemMigrations; | ||
|
|
||
| public abstract class CustomItemsUpdater implements ItemUpdater { | ||
| protected final Logger logger = LoggerFactory.getLogger(getClass()); | ||
| protected final Map<String, ItemMigrations> targets = new HashMap<>(); | ||
|
|
||
| /** | ||
| * Retrieves a migration list for the given custom key, creating one if it didn't already exist. | ||
| */ | ||
| public @NotNull ItemMigrations getMigrationsFor( | ||
| final @NotNull String customKey | ||
| ) { | ||
| return getMigrationsFor(Objects.requireNonNull(customKey), true); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves a migration list for the given custom key. | ||
| * | ||
| * @param createIfAbsent Whether to create the list if one doesn't already exist. | ||
| */ | ||
| @Contract("_, true -> !null") | ||
| public @Nullable ItemMigrations getMigrationsFor( | ||
| final @NotNull String customKey, | ||
| final boolean createIfAbsent | ||
| ) { | ||
| Objects.requireNonNull(customKey); | ||
| if (createIfAbsent) { | ||
| return this.targets.computeIfAbsent(customKey, CustomItemMigrations::new); | ||
| } | ||
| return this.targets.get(customKey); | ||
| } | ||
|
|
||
| public void removeMigrationsFor( | ||
| final @NotNull String customKey | ||
| ) { | ||
| final ItemMigrations migrations = this.targets.remove(Objects.requireNonNull(customKey)); | ||
| if (migrations != null) { | ||
| migrations.clearMigrations(); | ||
| } | ||
| } | ||
|
|
||
| public void clearMigrations() { | ||
| final List<ItemMigrations> migrations = List.copyOf(this.targets.values()); | ||
| this.targets.clear(); | ||
| for (final ItemMigrations migration : migrations) { | ||
| migration.clearMigrations(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * <p>This is how this class determines whether a given item is a custom item. You can just override this with a simple | ||
| * call to {@link vg.civcraft.mc.civmodcore.inventory.items.custom.CustomItem#getCustomItemKey(org.bukkit.inventory.ItemStack)}, | ||
| * but you may want to add additional logic to support legacy custom items.</p> | ||
| * | ||
| * <pre>{@code | ||
| * // Example | ||
| * @Override | ||
| * public @Nullable String getCustomKeyFrom(@NotNull ItemStack item) { | ||
| * if (CustomItem.getCustomItem(item) instanceof final String customKey) { | ||
| * return customKey; | ||
| * } | ||
| * if (item.getType() == Material.ENDER_EYE | ||
| * && ExampleUtils.hasPlainDisplayName(item, "Player Essence") | ||
| * && ExampleUtils.hasPlainLoreLine(item, "Activity reward used to fuel pearls") | ||
| * ) { | ||
| * return "player_essence"; | ||
| * } | ||
| * if (item.getType() == Material.BONE_BLOCK | ||
| * && ExampleUtils.hasPlainDisplayName(item, "City Bastion") | ||
| * && ExampleUtils.hasPlainLoreLine(item, "City bastions block reinforcements and elytra") | ||
| * ) { | ||
| * return "city_bastion"; | ||
| * } | ||
| * // etc | ||
| * return null; | ||
| * } | ||
| * }</pre> | ||
| */ | ||
| protected abstract @Nullable String getCustomKeyFrom( | ||
| @NotNull ItemStack item | ||
| ); | ||
|
|
||
| @Override | ||
| public boolean updateItem( | ||
| final @NotNull ItemStack item | ||
| ) { | ||
| final String customKey = getCustomKeyFrom(item); | ||
| if (customKey == null) { | ||
| return false; | ||
| } | ||
| final ItemMigrations migrations = this.targets.get(customKey); | ||
| if (migrations == null) { | ||
| return false; | ||
| } | ||
| return migrations.attemptMigration(item); | ||
| } | ||
|
|
||
| // ============================================================ | ||
| // Defaults | ||
| // ============================================================ | ||
|
|
||
| @Contract("_, _ -> param2") | ||
| public static <T extends CustomItemsUpdater> @NotNull T init( | ||
| final @NotNull JavaPlugin plugin, | ||
| final @NotNull T updater | ||
| ) { | ||
| Bukkit.getPluginManager().registerEvents(DefaultItemUpdaterListeners.wrap(updater), plugin); | ||
| return updater; | ||
| } | ||
| } | ||
|
|
||
| final class CustomItemMigrations extends ItemMigrations { | ||
| private static final NamespacedKey VERSION_KEY = new NamespacedKey(JavaPlugin.getPlugin(CivModCorePlugin.class), "item_version"); | ||
|
|
||
| public CustomItemMigrations( | ||
| final @NotNull String customKey | ||
| ) { | ||
| // This is a deliberate 0th migration that ensures that any item | ||
| // being migrated has a custom-item key and item version. | ||
| this.migrations.put(0, (ItemMigration.OfItem) (item) -> { | ||
|
Contributor
There was a problem hiding this comment. is a first migration necessary?
Contributor
Author
There was a problem hiding this comment. My understanding that items like fossils are registered custom items, so they would need a migration like this to be put into the system essentially. The problem is I have no idea how they are targeted by a migration if they lack a custom item key. IIRC the migration code just looks for items with a custom item key?
Contributor
There was a problem hiding this comment. There are two purposes to the item updater system:
The 0th migration just ensures that EVERY registered custom item has both a custom-item key and migration version, as stated explicitly in the comments. It establishes a baseline of what data can be accessed from the item without concern for NPEs. It also prevents stacking issues where one person's item has an explicit 0th migration-version set, and another whose 0th migration-version is implied by its absence (related: The 0th migration is not strictly necessary but it does offer a benefit, "speculative" as that may be. That said, I maintain that the vast majority of this migration system would be utterly unnecessary if Okx simply agreed to permit ephemeral display data (ie, setting names and lore to items at network-time): this whole migration system exists to fix typos in lore, or to remove outdated parts of lore, etc. It's silly. |
||
| CustomItem.setCustomItemKey(item, customKey); | ||
| setMigrationVersion(item, 0); | ||
| }); | ||
| } | ||
|
|
||
| @Override | ||
| public @Nullable Integer getMigrationVersion( | ||
| final @NotNull ItemStack item | ||
| ) { | ||
| // This is a readonly PDC | ||
| return item.getPersistentDataContainer().get(VERSION_KEY, PersistentDataType.INTEGER); | ||
| } | ||
|
|
||
| @Override | ||
| public void setMigrationVersion( | ||
| final @NotNull ItemStack item, | ||
| final int version | ||
| ) { | ||
| item.editPersistentDataContainer((pdc) -> pdc.set(VERSION_KEY, PersistentDataType.INTEGER, version)); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
How is this class initialised?
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.
Its really weird. The class is meant to be extended, by a migrator for an item, which then calls
Civ/plugins/civmodcore-paper/src/main/java/vg/civcraft/mc/civmodcore/inventory/items/custom/CustomItemsUpdater.java
Line 123 in d101628
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.
Yup. Give #754 a read through. It started off as a really simple predicate-with-side-effects: it takes an item and returns a boolean of whether the item was modified or not. You'd do all your checks within that predicate, returning early wherever possible.
Then I added event handler classes which handle specific events and updating their respective items. For example,
UpdatePlayerItemsOnJoinlistens for player joins and then invokes the predicate for each item in the player's inventory and ender chest.UpdateInventoryItemsOnOpenlistens for players opening chests (and other contains), updating its items.UpdateContainerItemsOnLoadlistens for chunk loads and updates all the items in its container blocks.If you wanted your item-updater to run whenever a chest was opened, all you needed to do was uncomment that implements part. All the work was done for you.
However, Okx wanted more of a DataFixer-esque system. Take a look at
net.minecraft.util.datafix.DataFixersfor what that looks like. As mentioned here: instead of entirely replacing what was already built, I instead created the CustomItemUpdater as a layer of abstraction on-top of it. This meant there was a simpler system if you wanted it, and a complexer system if you needed it.However, I do admit that the type shenanigans were a mistake. It seemed clever at the time but it's definitely a struggle to read, even for me now and I wrote it. Basically, since
UpdatePlayerItemsOnJoinextends ItemUpdater but does not implement it, you can effectively combine/union them with an implementation of ItemUpdater via casting, eg: