Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ private String findExtras(String string) {
* <li>Enchantments (Will add {@value ENCHANTED_INDICATOR})</li>
* <li>Skull Owner/Texture (Texture is prioritized)</li>
* <li>CustomModelData (custom_model_data on newer MC versions).</li>
* <li>item_model</li>
* </ul>
*
* @param itemStack The Item to convert into a HologramItem.
Expand Down Expand Up @@ -203,10 +204,8 @@ public static HologramItem fromItemStack(ItemStack itemStack) {
}
}

float customModelData = NbtApiHook.extractCustomModelData(itemStack);
if (customModelData > 0.0) {
stringBuilder.append("{CustomModelData:").append(customModelData).append('}');
}
NbtApiHook.ItemNbtData nbtRead = NbtApiHook.readData(itemStack);
stringBuilder.append(nbtRead.getJson());
return new HologramItem(stringBuilder.toString());
}

Expand Down
103 changes: 102 additions & 1 deletion plugin/src/main/java/eu/decentsoftware/holograms/hook/NbtApiHook.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package eu.decentsoftware.holograms.hook;

import com.google.gson.Gson;
Copy link
Member

Choose a reason for hiding this comment

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

Unused import

import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import de.tr7zw.changeme.nbtapi.NBT;
import de.tr7zw.changeme.nbtapi.iface.ReadWriteNBT;
import de.tr7zw.changeme.nbtapi.iface.ReadWriteNBTList;
import de.tr7zw.changeme.nbtapi.utils.DataFixerUtil;
import eu.decentsoftware.holograms.api.utils.Log;
import eu.decentsoftware.holograms.api.utils.PAPI;
import eu.decentsoftware.holograms.api.utils.reflect.Version;
import lombok.Data;
import lombok.experimental.UtilityClass;
import org.bukkit.entity.Player;
import org.bukkit.inventory.ItemStack;
Expand Down Expand Up @@ -59,19 +63,69 @@ public static ItemStack applyNbtDataToItemStack(ItemStack itemStack, String nbt,
*/
modifiableNBT.mergeCompound(originalNBT);

// Load the nbt data
ItemNbtData nbtData = ItemNbtData.fromJson(nbt);
Copy link
Member

Choose a reason for hiding this comment

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

At line 50, the value of nbt is used to parse NBT and merge the resulting compound with modifiableNBT. Considering the value of nbt can be a JSON instead of actual NBT, won't that just fail or go wrong?

I also don't like how one variable nbt is used for two different things. It can either be NBT or your JSON with some data that will be added to NBT. Can you somehow separate these?

When you separate them, you may not even need the JSON at all. You could just use the ItemNbtData object in both HologramItem and here, which is much nicer in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You bring up a good point. This is something I got stuck on as I was trying to think of a good way to integrate this into the current data fixer solution you use for custom model data.

Here's what I found: When the minecraft:item_model component is passed through the datafixer, it gets mapped under the minecraft:custom_data component instead of being left alone. This breaks the display of the item since the minecraft:item_model component needs to be at the root level similar to CustomModelData. Since this component did not exist in 1.20.4, the data fixer treats it as custom data. That's why I parse the NBT data back into JSon and manually add the component after running the data fixer.

I can make a modification to see how this would work without the data fixer.


/*
* Since item_model is new, it gets mapped under 'custom_data' in the item.
* We need to manually re-apply it.
*/
if (nbtData.getItemModel() != null) {
modifiableNBT.getOrCreateCompound("components").setString("minecraft:item_model", nbtData.getItemModel());
}

return NBT.itemStackFromNBT(modifiableNBT);
} catch (Exception ex) {
Log.warn("Failed to apply NBT Data to Item: %s", ex, nbt);
return itemStack;
}
}

public static ItemNbtData readData(ItemStack itemStack) {
if (!loadedSuccessfully) {
return ItemNbtData.EMPTY;
}

ReadWriteNBT nbt = NBT.itemStackToNBT(itemStack);
return new ItemNbtData(extractItemModel(nbt), extractCustomModelData(nbt));
}

public static String extractItemModel(ItemStack itemStack) {
Copy link
Member

Choose a reason for hiding this comment

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

This method is unused, remove it.

if (!loadedSuccessfully) {
return null;
}

return extractItemModel(NBT.itemStackToNBT(itemStack));
}

public static String extractItemModel(ReadWriteNBT nbtItem) {
Copy link
Member

Choose a reason for hiding this comment

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

This method can be private

if (!loadedSuccessfully) {
return null;
}

String itemModel;
if (Version.afterOrEqual(Version.v1_21_R2)) {
itemModel = nbtItem.getOrCreateCompound("components")
.getString("minecraft:item_model");
} else {
itemModel = null;
}
return itemModel;
}

public static float extractCustomModelData(ItemStack itemStack) {
Copy link
Member

Choose a reason for hiding this comment

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

This method is unused, so remove it.

if (!loadedSuccessfully) {
return 0f;
}

ReadWriteNBT nbtItem = NBT.itemStackToNBT(itemStack);
return extractCustomModelData(NBT.itemStackToNBT(itemStack));
}

public static float extractCustomModelData(ReadWriteNBT nbtItem) {
Copy link
Member

Choose a reason for hiding this comment

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

This method can be private

if (!loadedSuccessfully) {
return 0f;
}

float customModelData;
if (Version.afterOrEqual(Version.v1_21_R3)) {
// New structure components:{custom_model_data={floats[...]}} since 1.21.4
Expand All @@ -91,4 +145,51 @@ public static float extractCustomModelData(ItemStack itemStack) {
}
return customModelData;
}

/**
* Represents the result of reading data from an item stack.
* This is used to prevent redundancy with NBTAPI.
*/
@Data
public static class ItemNbtData {
public static final ItemNbtData EMPTY = new ItemNbtData(null, 0f);

private final String itemModel;
private final float customModelData;
private String json;

public ItemNbtData(String itemModel, float customModelData) {
this.itemModel = itemModel;
this.customModelData = customModelData;
this.json = toJson();
}

/**
* Converts this result to json.
*
* @return the json.
*/
private String toJson() {
if ((this.itemModel == null || this.itemModel.isEmpty()) && this.customModelData == 0f)
return "";

// Use Gson to allow for easier expansion in the future.
JsonObject object = new JsonObject();
if (this.itemModel != null && !this.itemModel.isEmpty())
object.addProperty("minecraft:item_model", this.itemModel);
if (this.customModelData != 0f)
object.addProperty("CustomModelData", this.customModelData);
return object.toString();
}

public static ItemNbtData fromJson(String json) {
if (json == null || json.isEmpty())
return EMPTY;

JsonObject object = new JsonParser().parse(json).getAsJsonObject();
String itemModel = object.has("minecraft:item_model") ? object.get("minecraft:item_model").getAsString() : null;
float customModelData = object.has("CustomModelData") ? object.get("CustomModelData").getAsFloat() : 0f;
return new ItemNbtData(itemModel, customModelData);
}
}
}