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

🚀 Improve toString of ItemData #4707

Closed
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
11b3deb
⚒️ Replace color char in debug statements
AyhamAl-Ali Feb 25, 2022
39d2097
Update src/main/java/ch/njol/skript/util/SkriptColor.java
AyhamAl-Ali Mar 8, 2022
b3a3fa7
Ready
AyhamAl-Ali Mar 8, 2022
743987c
Merge branch 'ench/debug-replace-chat-color' into ench/itemdata-tostring
AyhamAl-Ali Mar 27, 2022
7952529
🚀 Improve toString to ItemData
AyhamAl-Ali Mar 27, 2022
2953706
Remove unused imports
AyhamAl-Ali Mar 27, 2022
a4ca252
Revert "Merge branch 'ench/debug-replace-chat-color' into ench/itemda…
AyhamAl-Ali Mar 27, 2022
230f5c0
Details
AyhamAl-Ali Mar 27, 2022
d915a21
Temporarily add replaceColorChar method until the other pr is merged
AyhamAl-Ali Mar 31, 2022
faac015
Requested Changes - formatting
AyhamAl-Ali Nov 27, 2022
bb80a7c
Merge branch 'master' into ench/itemdata-tostring
AyhamAl-Ali Jan 1, 2023
2d56dc6
Add support for parsing itemtype after named and with lore addition
AyhamAl-Ali Apr 21, 2023
c5adbc4
Add itemtype parsing tests
AyhamAl-Ali Apr 21, 2023
28eec80
Fix tests
AyhamAl-Ali Apr 21, 2023
405e0f3
Fix tests 2
AyhamAl-Ali Apr 21, 2023
d44793b
Update src/main/java/ch/njol/skript/aliases/Aliases.java
AyhamAl-Ali Apr 22, 2023
c4c5ffd
Adding some helpful comments
AyhamAl-Ali Apr 22, 2023
05befaa
Remove file ending empty lines from test script
AyhamAl-Ali Apr 22, 2023
250182b
Merge branch 'dev/feature' into ench/itemdata-tostring
AyhamAl-Ali Oct 5, 2023
c6eb7bc
Merge branch 'dev/feature' into ench/itemdata-tostring
sovdeeth Mar 19, 2024
8ea3fec
Merge branch 'dev/feature' into ench/itemdata-tostring
Moderocky May 8, 2024
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
146 changes: 90 additions & 56 deletions src/main/java/ch/njol/skript/aliases/Aliases.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import ch.njol.skript.config.Node;
import ch.njol.skript.config.SectionNode;
import ch.njol.skript.entity.EntityData;
import org.skriptlang.skript.lang.script.Script;
import ch.njol.skript.lang.parser.ParserInstance;
import ch.njol.skript.localization.ArgsMessage;
import ch.njol.skript.localization.Language;
Expand All @@ -39,7 +38,9 @@
import org.bukkit.ChatColor;
import org.bukkit.Material;
import org.bukkit.NamespacedKey;
import org.bukkit.inventory.meta.ItemMeta;
import org.eclipse.jdt.annotation.Nullable;
import org.skriptlang.skript.lang.script.Script;

import java.io.IOException;
import java.io.UncheckedIOException;
Expand All @@ -50,11 +51,13 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

public abstract class Aliases {

Expand Down Expand Up @@ -229,95 +232,126 @@ public static ItemType parseAlias(final String s) {
private final static RegexMessage p_every = new RegexMessage("aliases.every", "", " (.+)", Pattern.CASE_INSENSITIVE);
private final static RegexMessage p_of_every = new RegexMessage("aliases.of every", "(\\d+) ", " (.+)", Pattern.CASE_INSENSITIVE);
private final static RegexMessage p_of = new RegexMessage("aliases.of", "(\\d+) (?:", " )?(.+)", Pattern.CASE_INSENSITIVE);

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");
Comment on lines +237 to +244
Copy link
Member

Choose a reason for hiding this comment

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

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");


/**
* Parses an ItemType.
* <p>
* Prints errors.
*
* @param s
* @param value
* @return The parsed ItemType or null if the input is invalid.
*/
@Nullable
public static ItemType parseItemType(String s) {
if (s.isEmpty())
public static ItemType parseItemType(String value) {
if (value.isEmpty())
return null;
s = "" + s.trim();
value = "" + value.trim();
Copy link
Member

Choose a reason for hiding this comment

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

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

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


final ItemType t = new ItemType();

Matcher m;
if ((m = p_of_every.matcher(s)).matches()) {
t.setAmount(Utils.parseInt("" + m.group(1)));
t.setAll(true);
s = "" + m.group(m.groupCount());
} else if ((m = p_of.matcher(s)).matches()) {
t.setAmount(Utils.parseInt("" + m.group(1)));
s = "" + m.group(m.groupCount());
} else if ((m = p_every.matcher(s)).matches()) {
t.setAll(true);
s = "" + m.group(m.groupCount());
ItemType itemtype = new ItemType();

// Amount
Matcher matcher;
if ((matcher = p_of_every.matcher(value)).matches()) {
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());
Comment on lines +265 to +273
Copy link
Member

Choose a reason for hiding this comment

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

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());

} else {
final int l = s.length();
s = Noun.stripIndefiniteArticle(s);
if (s.length() != l) // had indefinite article
t.setAmount(1);
int length = value.length();
value = Noun.stripIndefiniteArticle(value);
if (value.length() != length) // had indefinite article
itemtype.setAmount(1);
}

String lc = s.toLowerCase(Locale.ENGLISH);
String of = Language.getSpaced("enchantments.of").toLowerCase();
int c = -1;
outer: while ((c = lc.indexOf(of, c + 1)) != -1) {
ItemType t2 = t.clone();

String lowercase = value.toLowerCase(Locale.ENGLISH);

// Enchantments
String of = Language.getSpaced("enchantments.of").toLowerCase(Locale.ENGLISH);
int character = -1;
outer: while ((character = lowercase.indexOf(of, character + 1)) != -1) {
ItemType clonedItemtype = itemtype.clone();
try (BlockingLogHandler ignored = new BlockingLogHandler().start()) {
if (parseType("" + s.substring(0, c), t2, false) == null)
if (parseType("" + value.substring(0, character), clonedItemtype, false) == null)
Copy link
Member

Choose a reason for hiding this comment

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

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

continue;
}
if (t2.numTypes() == 0)
if (clonedItemtype.numTypes() == 0)
continue;
String[] enchs = lc.substring(c + of.length()).split("\\s*(,|" + Pattern.quote(Language.get("and")) + ")\\s*");
for (final String ench : enchs) {
EnchantmentType e = EnchantmentType.parse("" + ench);
if (e == null)

boolean hasName = lowercase.contains(" " + m_named + " ");
boolean hasLore = lowercase.contains(" " + m_with_lore + " ");

// Name
matcher = p_named.matcher(value.substring(character, hasLore ? lowercase.indexOf(" " + m_with_lore + " ") : lowercase.length()));
Copy link
Member

Choose a reason for hiding this comment

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

what happens if the name contains with lore?

if (matcher.matches()) {
ItemMeta meta = clonedItemtype.getItemMeta();
meta.setDisplayName(matcher.group(2));
clonedItemtype.setItemMeta(meta);
}

// Lore
matcher = p_with_lore.matcher(value);
if (matcher.matches()) {
ItemMeta meta = clonedItemtype.getItemMeta();
meta.setLore(Arrays.stream(MULTIPLE_VALUE_SPLIT_PATTERN.split(matcher.group(2))).map(line -> line.substring(1, line.length() -1)).collect(Collectors.toList())); // strip quotes
clonedItemtype.setItemMeta(meta);
}
int endOfSubstring = hasName ? lowercase.indexOf(" " + m_named + " ") : hasLore ? lowercase.indexOf(" " + m_with_lore + " ") : lowercase.length();
String[] enchantments = ENCHANTMENTS_PATTERN.split(lowercase.substring(character + of.length(), endOfSubstring));
for (String enchantment : enchantments) {
EnchantmentType enchantmentType = EnchantmentType.parse("" + enchantment);
AyhamAl-Ali marked this conversation as resolved.
Show resolved Hide resolved
if (enchantmentType == null)
continue outer;
t2.addEnchantments(e);
clonedItemtype.addEnchantments(enchantmentType);
}
return t2;
return clonedItemtype;
}
if (parseType(s, t, false) == null)

if (parseType(value, itemtype, false) == null)
return null;
if (t.numTypes() == 0)

if (itemtype.numTypes() == 0)
return null;
return t;

return itemtype;
}

/**
* Prints errors.
*
* @param s The string holding the type, can be either a number or an alias, plus an optional data part. Case does not matter.
* @param t The ItemType to add the parsed ItemData(s) to (i.e. this ItemType will be modified)
* @param value The string holding the type, can be either a number or an alias, plus an optional data part. Case does not matter.
* @param itemtype The ItemType to add the parsed ItemData(s) to (i.e. this ItemType will be modified)
* @param isAlias Whether this type is parsed for an alias.
* @return The given item type or null if the input couldn't be parsed.
*/
@Nullable
private static ItemType parseType(final String s, final ItemType t, final boolean isAlias) {
ItemType i;
if (s.isEmpty()) {
t.add(new ItemData(Material.AIR));
return t;
} else if (s.matches("\\d+")) {
private static ItemType parseType(String value, ItemType itemtype, boolean isAlias) {
ItemType itemtypeCompare;
if (value.isEmpty()) {
itemtype.add(new ItemData(Material.AIR));
return itemtype;
} else if (value.matches("\\d+")) {
return null;
} else if ((i = getAlias(s)) != null) {
for (ItemData d : i) {
t.add(d.clone());
} else if ((itemtypeCompare = getAlias(value)) != null) {
for (ItemData itemData : itemtypeCompare) {
itemtype.add(itemData.clone());
}
return t;
return itemtype;
}
if (isAlias)
Skript.error(m_invalid_item_type.toString(s));
Skript.error(m_invalid_item_type.toString(value));
return null;
}

Expand Down
59 changes: 54 additions & 5 deletions src/main/java/ch/njol/skript/aliases/ItemData.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@
import ch.njol.skript.bukkitutil.ItemUtils;
import ch.njol.skript.bukkitutil.block.BlockCompat;
import ch.njol.skript.bukkitutil.block.BlockValues;
import ch.njol.skript.localization.GeneralWords;
import ch.njol.skript.localization.Language;
import ch.njol.skript.localization.Message;
import ch.njol.skript.util.EnchantmentType;
import ch.njol.skript.util.SkriptColor;
import ch.njol.skript.variables.Variables;
import ch.njol.yggdrasil.Fields;
import ch.njol.yggdrasil.YggdrasilSerializable.YggdrasilExtendedSerializable;
Expand All @@ -49,7 +53,9 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;

Expand Down Expand Up @@ -85,7 +91,8 @@ public static class OldItemData {
}

private final static Message m_named = new Message("aliases.named");

private final static Message m_with_lore = new Message("aliases.with lore");

/**
* Before 1.13, data values ("block states") are applicable to items.
*
Expand Down Expand Up @@ -242,13 +249,55 @@ public String toString() {
return toString(false, false);
}

public String toString(final boolean debug, final boolean plural) {
public String toString(boolean debug, boolean plural) {
StringBuilder builder = new StringBuilder(Aliases.getMaterialName(this, plural));
ItemMeta meta = stack.getItemMeta();
if (meta != null && meta.hasDisplayName()) {
builder.append(" ").append(m_named).append(" ");
builder.append(meta.getDisplayName());

Map<Enchantment, Integer> enchantments = stack.getEnchantments();
if (!enchantments.isEmpty()) {
builder.append(Language.getSpaced("enchantments.of").toLowerCase(Locale.ENGLISH));
int i = 0;
for (Entry<Enchantment, Integer> e : enchantments.entrySet()) {
if (i != 0) {
if (i != enchantments.size() - 1) {
builder.append(", ");
} else {
builder.append(" " + GeneralWords.and + " ");
}
}
Enchantment ench = e.getKey();
if (ench == null)
continue;
builder.append(EnchantmentType.toString(ench));
builder.append(" ");
builder.append(e.getValue());
i++;
}
}

if (meta != null) {
if (meta.hasDisplayName()) {
builder.append(" ").append(m_named).append(" ");
builder.append("\"").append(SkriptColor.replaceColorChar(meta.getDisplayName())).append("\"");
Copy link
Member

Choose a reason for hiding this comment

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

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

}
if (meta.hasLore()) {
builder.append(" ").append(m_with_lore).append(" ");
List<String> lore = meta.getLore();
int i = 0;
for (String l : lore) {
if (i != 0) {
if (i != lore.size() - 1) {
builder.append(", ");
} else {
builder.append(" " + GeneralWords.and + " ");
}
}
builder.append("\"").append(SkriptColor.replaceColorChar(l)).append("\"");
i++;
}
}
}

return builder.toString();
}

Expand Down
42 changes: 1 addition & 41 deletions src/main/java/ch/njol/skript/aliases/ItemType.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import java.util.RandomAccess;
import java.util.Set;

import ch.njol.skript.classes.Comparator.Relation;
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.OfflinePlayer;
Expand Down Expand Up @@ -993,16 +992,6 @@ public String toString(final int flags, final @Nullable Adjective a) {

private String toString(final boolean debug, final int flags, final @Nullable Adjective a) {
final StringBuilder b = new StringBuilder();
// if (types.size() == 1 && !types.get(0).hasDataRange()) {
// if (getAmount() != 1)
// b.append(amount + " ");
// if (isAll())
// b.append(getAmount() == 1 ? "every " : "of every ");
// } else {
// if (getAmount() != 1)
// b.append(amount + " of ");
// b.append(isAll() ? "every " : "any ");
// }
final boolean plural = amount != 1 && amount != -1 || (flags & Language.F_PLURAL) != 0;
if (amount != -1 && amount != 1) {
b.append(amount + " ");
Expand All @@ -1020,36 +1009,7 @@ private String toString(final boolean debug, final int flags, final @Nullable Ad
}
b.append(types.get(i).toString(debug, plural));
}
// final Map<Enchantment, Integer> enchs = enchantments;
// if (enchs == null)
// return "" + b.toString();
// b.append(Language.getSpaced("enchantments.of"));
// int i = 0;
// for (final Entry<Enchantment, Integer> e : enchs.entrySet()) {
// if (i != 0) {
// if (i != enchs.size() - 1)
// b.append(", ");
// else
// b.append(" " + GeneralWords.and + " ");
// }
// final Enchantment ench = e.getKey();
// if (ench == null)
// continue;
// b.append(EnchantmentType.toString(ench));
// b.append(" ");
// b.append(e.getValue());
// i++;
// }
// if (meta != null) {
// final ItemMeta m = (ItemMeta) meta;
// if (m.hasDisplayName()) {
// b.append(" " + m_named.toString() + " ");
// b.append("\"" + m.getDisplayName() + "\"");
// }
// if (debug)
// b.append(" meta=[").append(meta).append("]");
// }
return "" + b.toString();
return b.toString();
}

public static String toString(final ItemStack i) {
Expand Down
1 change: 1 addition & 0 deletions src/main/resources/lang/default.lang
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ aliases:
of: of(?: any)?

named: named
with lore: with lore

# -- Enchantments --
enchantments:
Expand Down
Loading