🚀 Improve toString of ItemData

[AyhamAl-Ali]

Description

This PR should be merged at least after #4630
Enhance toString of items to include enchantments, lore, fix colors and add quotation marks around the name
After PR

It's no longer a breaking change as of commit however, I am keeping the label to note in the release changelog that the output of itemtypes has been updated so if someone is depending on that it will be a breaking change.

---

Target Minecraft Versions: Any
Requirements: None
Related Issues:

• enchance read tool #4706

[pawelkuwa]

where can i download skript nightly with it?

[AyhamAl-Ali]

where can i download skript nightly with it?

You can locate it now in the checks tab > artifacts or from this link

[pawelkuwa]

doesn't give item :(

[APickledWalrus]

doesn't give item :(

It's because the ItemType parser does not support parsing to this extent. We can expand the output here, but it won't be useful for parsing the string version of an item type.

[pawelkuwa]

why it won't be usefull? in some scripts i save items into string for store items in yaml but i must use my special function for read and for other people it is terrible

[APickledWalrus]

The issue is you won't be able to parse the string as an item (as you encountered above when you stated that it doesn't give you the item)

Perhaps we could change this in the future, but you should probably use something like skript-yaml which IIRC will serialize items for you (meaning you don't need to store them as strings)

[AyhamAl-Ali]

If you don't want to use the suggestion above you better split its attributes when saving then parse it when needed something like this

itemtype: stone
name: "&aCustom Name"
lore: ["lore 1", "lore 2", "lore 3"]

script would be

# set {_itemtype}, {_name} and {_lore} to the yml values
set {_item} to {_itemtype} parsed as itemtype named {_name} with lore {_lore}

[TheLimeGlass]

@AyhamAl-Ali conflicts

[TheLimeGlass]

I think Aliases#parseItemType should support parsing of this addition (Just name and lore need to be added from the looks of it), that way parsing will work. I don't think we should completely remove the ability to parse what ItemType returns.

[APickledWalrus]

Suggested change

	private final static RegexMessage p_named = new RegexMessage("aliases.named", "(.+) ", " \"(.+)\"", Pattern.CASE_INSENSITIVE);
	private final static RegexMessage p_with_lore = new RegexMessage("aliases.with lore", "(.+) ", " (\".+\")", Pattern.CASE_INSENSITIVE);
	private final static Pattern ENCHANTMENTS_PATTERN = Pattern.compile("\\s*(,|" + Pattern.quote(Language.get("and")) + ")\\s*");
	private final static Pattern MULTIPLE_VALUE_SPLIT_PATTERN = Pattern.compile("(, ?| " + Pattern.quote(Language.get("and")) + " )");
	private final static Message m_named = new Message("aliases.named");
	private final static Message m_with_lore = new Message("aliases.with lore");
	private static final RegexMessage p_named = new RegexMessage("aliases.named", "(.+) ", " \"(.+)\"", Pattern.CASE_INSENSITIVE);
	private static final RegexMessage p_with_lore = new RegexMessage("aliases.with lore", "(.+) ", " (\".+\")", Pattern.CASE_INSENSITIVE);
	private static final Pattern ENCHANTMENTS_PATTERN = Pattern.compile("\\s*(,|" + Pattern.quote(Language.get("and")) + ")\\s*");
	private static final Pattern MULTIPLE_VALUE_SPLIT_PATTERN = Pattern.compile("(, ?| " + Pattern.quote(Language.get("and")) + " )");
	private static final Message m_named = new Message("aliases.named");
	private static final Message m_with_lore = new Message("aliases.with lore");

[APickledWalrus]

Suggested change

	value = "" + value.trim();
	value = value.trim();

These should be removable. IIRC they are leftover from old null analysis

[APickledWalrus]

Suggested change

	itemtype.setAmount(Utils.parseInt("" + matcher.group(1)));
	itemtype.setAll(true);
	value = "" + matcher.group(matcher.groupCount());
	} else if ((matcher = p_of.matcher(value)).matches()) {
	itemtype.setAmount(Utils.parseInt("" + matcher.group(1)));
	value = "" + matcher.group(matcher.groupCount());
	} else if ((matcher = p_every.matcher(value)).matches()) {
	itemtype.setAll(true);
	value = "" + matcher.group(matcher.groupCount());
	itemtype.setAmount(Utils.parseInt(matcher.group(1)));
	itemtype.setAll(true);
	value = matcher.group(matcher.groupCount());
	} else if ((matcher = p_of.matcher(value)).matches()) {
	itemtype.setAmount(Utils.parseInt(matcher.group(1)));
	value = matcher.group(matcher.groupCount());
	} else if ((matcher = p_every.matcher(value)).matches()) {
	itemtype.setAll(true);
	value = matcher.group(matcher.groupCount());

[APickledWalrus]

Suggested change

	if (parseType("" + value.substring(0, character), clonedItemtype, false) == null)
	if (parseType(value.substring(0, character), clonedItemtype, false) == null)

[APickledWalrus]

what happens if the name contains with lore?

[APickledWalrus]

Suggested change

	for (Entry<Enchantment, Integer> entry : enchantments.entrySet()) {
	if (i != 0) {
	if (i != enchantments.size() - 1) {
	int size = enchantments.size();
	for (Entry<Enchantment, Integer> entry : enchantments.entrySet()) {
	if (i != 0) {
	if (i != size - 1) {

[APickledWalrus]

I'm not sure we should color this by default. IMO it should be up to the user to color the string.

[APickledWalrus]

This PR also likely needs to account for the fact that some ItemDatas do not represent ItemStacks anymore

[sovdeeth]

Closing due to inactivity