Skip to content
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

Feat/data components #514

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Feat/data components #514

wants to merge 2 commits into from

Conversation

Owen1212055
Copy link
Member

Please feel free to make modifications directly to the PR.

This is meant as an initial base.

Copy link

github-actions bot commented Dec 29, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
papermc-docs ✅ Ready (View Log) Visit Preview 12382fb

Copy link
Contributor

@Strokkur424 Strokkur424 left a comment

Choose a reason for hiding this comment

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

Definitely helps a ton with understanding data components.
One thing I would maybe suggest to add is a small line which explains whether you can use ItemMeta and DataComponents concurrently, since I am not quite sure about that, even after reading your docs.

EDIT: Also sorry if some of these reviews are broken. I haven't really used this specific feature of GitHub yet

itemStack.setData(DataComponentTypes.RARITY, ItemRarity.RARE);

itemStack.unsetData(DataComponentTypes.TOOL); // Remove the cool component

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you meant to say tool here?

Suggested change
itemStack.unsetData(DataComponentTypes.TOOL); // Remove the tool component

ItemStack originalSword = new ItemStack(Material.DIAMOND_SWORD);

boolean match = damagedSword.matchesWithoutData(originalSword, Set.of(DataComponentTypes.DAMAGE), false);
System.out.println("Match ignoring damage? " + match);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is the last sout call I would replace with logger

Suggested change
System.out.println("Match ignoring damage? " + match);
logger.info("Match ignoring damage? " + match);

:::

The Data Component API provides a version-specific, programmatic interface for accessing and manipulating item data that is otherwise not representable by the `ItemMeta` API. Through this API, you can read and write properties of an item—called "data components"—in a stable and object-oriented manner.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhabs remove the "programming interface" part, as that is already contained within API. Also add a line wrap for better readability in-editor

Suggested change
The Data Component API provides a version-specific interface for accessing and manipulating item data that is otherwise not representable by the `ItemMeta` API.
Through this API, you can read and write properties, called "data components", of an item in a stable and object-oriented manner.

### Structure
![Component Structure](assets/data-component-api-tree.png)
See implementation [here](#example-cool-sword)

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make it clearer what this link does?

Suggested change
For implementation details, [click here](#example-cool-sword)

allowing us to make modifications to an item.

The patch also allows for removing components that were previously in the prototype, this is shown by
the `minecraft:tool` example in red. We are removing this component, so this sword item will no longer
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
the `minecraft:tool` example in red. We are removing this component, so this sword item will no longer
The patch also allows for removing components that were previously in the prototype. This is shown by


#### No Snapshot
Currently, ItemMeta represents a *Snapshot* of an ItemStack's patched map.
This is expensive as it requires the entire patch to be read, even values that you may not be using.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This is expensive as it requires the entire patch to be read, even values that you may not be using.
Currently, ItemMeta represents a *snapshot* of an ItemStack's patched map.

Comment on lines +106 to +110
if (damageValue != null) {
System.out.println("Current damage: " + damageValue);
} else {
System.out.println("This item doesn't have a damage component set.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here, replace sout call with logging

Suggested change
if (damageValue != null) {
System.out.println("Current damage: " + damageValue);
} else {
System.out.println("This item doesn't have a damage component set.");
}
if (damageValue != null) {
logger.info("Current damage: " + damageValue);
} else {
logger.info("This item doesn't have a damage component set.");
}

Comment on lines +118 to +125
```java
// Set a custom model data value on this item
stack.setData(DataComponentTypes.CUSTOM_MODEL_DATA, CustomModelData.customModelData()
.addFloat(0.5f)
.addFlag(true)
.build()
);
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe smaller indentation to be easier on the reader?

Suggested change
```java
// Set a custom model data value on this item
stack.setData(DataComponentTypes.CUSTOM_MODEL_DATA, CustomModelData.customModelData()
.addFloat(0.5f)
.addFlag(true)
.build()
);
```
```java
// Set a custom model data value on this item
stack.setData(DataComponentTypes.CUSTOM_MODEL_DATA, CustomModelData.customModelData()
.addFloat(0.5f)
.addFlag(true)
.build()
);

Comment on lines +170 to +174
```
This will create a diamond helmet that looks like a netherrite helmet when equipped, but plays a spooky
ghast sound when equipped.

### Example: Written Book
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```
This will create a diamond helmet that looks like a netherrite helmet when equipped, but plays a spooky
ghast sound when equipped.
### Example: Written Book

This will create a diamond helmet that looks like a netherrite helmet and plays a spooky ghast sound when equipped.

Example: Written Book

// Check if this item has a custom name data component
boolean hasCustomName = stack.hasData(DataComponentTypes.CUSTOM_NAME);
System.out.println("Has custom name? " + hasCustomName);
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe replace System.out.println with more robust logging in order to not endorse people to sout in production?

Suggested change
```
logger.info("Has custom name? " + hasCustomName);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants