From d4f649620b47bf4d111bfc90843664a5fd83d209 Mon Sep 17 00:00:00 2001 From: Fusezion Date: Thu, 16 Jan 2025 00:34:23 -0500 Subject: [PATCH 1/9] Fix tag lookup order - Add a regression test --- .../skript/bukkit/tags/elements/ExprTag.java | 45 ++++++++++--------- .../7449-tag lookup only does minecraft.sk | 5 +++ 2 files changed, 29 insertions(+), 21 deletions(-) create mode 100644 src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk diff --git a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java index 8c990e6b85e..65ab05ae9ce 100644 --- a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java +++ b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java @@ -78,33 +78,36 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is String[] names = this.names.getArray(event); List> tags = new ArrayList<>(); - String namespace = switch (origin) { - case ANY, BUKKIT -> "minecraft"; - case PAPER -> "paper"; - case SKRIPT -> "skript"; + String[] namespaces = switch (origin) { + case ANY -> new String[]{"minecraft", "paper", "skript"}; + case BUKKIT -> new String[]{"minecraft"}; + case PAPER -> new String[]{"paper"}; + case SKRIPT -> new String[]{"skript"}; }; nextName: for (String name : names) { // get key NamespacedKey key; - if (name.contains(":")) { - key = NamespacedKey.fromString(name); - } else { - // populate namespace if not provided - key = new NamespacedKey(namespace, name); - } - if (key == null) - continue; + for (String namespace : namespaces) { + if (name.contains(":")) { + key = NamespacedKey.fromString(name); + } else { + // populate namespace if not provided + key = new NamespacedKey(namespace, name); + } + if (key == null) + continue; - Tag tag; - for (TagType type : types) { - tag = TagModule.tagRegistry.getTag(origin, type, key); - if (tag != null - // ensures that only datapack/minecraft tags are sent when specifically requested - && (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals("minecraft"))) - ) { - tags.add(tag); - continue nextName; // ensure 1:1 + Tag tag; + for (TagType type : types) { + tag = TagModule.tagRegistry.getTag(origin, type, key); + if (tag != null + // ensures that only datapack/minecraft tags are sent when specifically requested + && (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals("minecraft"))) + ) { + tags.add(tag); + continue nextName; // ensure 1:1 + } } } } diff --git a/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk b/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk new file mode 100644 index 00000000000..6bc08894019 --- /dev/null +++ b/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk @@ -0,0 +1,5 @@ +test "77449 - Tag Lookup Only does Minecraft": + register an item tag named "my_favorite_blocks" using oak log, stone, and podzol + assert tag "my_favorite_blocks" is tag "skript:my_favorite_blocks" with "Tag lookup didn't find a skript tag ""helmets"" namespace" + assert tag "helmets" is tag "paper:helmets" with "Tag lookup didn't find a paper tag ""helmets"" namespace" + assert tag "enchantable/sword" is tag "minecraft:enchantable/sword" with "Tag lookup didn't find a minecraft tag ""enchantable/sword"" namespace" From 7ba2435eb08ef9693f3f08d32fa040ac909ab9a1 Mon Sep 17 00:00:00 2001 From: Fusezion Date: Thu, 16 Jan 2025 00:53:04 -0500 Subject: [PATCH 2/9] Fix java 17 test (stupid mc) --- .../tests/regressions/7449-tag lookup only does minecraft.sk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk b/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk index 6bc08894019..143e89eadc3 100644 --- a/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk +++ b/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk @@ -2,4 +2,4 @@ test "77449 - Tag Lookup Only does Minecraft": register an item tag named "my_favorite_blocks" using oak log, stone, and podzol assert tag "my_favorite_blocks" is tag "skript:my_favorite_blocks" with "Tag lookup didn't find a skript tag ""helmets"" namespace" assert tag "helmets" is tag "paper:helmets" with "Tag lookup didn't find a paper tag ""helmets"" namespace" - assert tag "enchantable/sword" is tag "minecraft:enchantable/sword" with "Tag lookup didn't find a minecraft tag ""enchantable/sword"" namespace" + assert tag "dirt" is tag "minecraft:dirt" with "Tag lookup didn't find a minecraft tag ""dirt"" namespace" From cae6cba5a97642f6f61955f6f1931f8005143d22 Mon Sep 17 00:00:00 2001 From: Fusezion Date: Thu, 16 Jan 2025 16:51:46 -0500 Subject: [PATCH 3/9] Update src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk Co-authored-by: Efnilite <35348263+Efnilite@users.noreply.github.com> --- .../tests/regressions/7449-tag lookup only does minecraft.sk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk b/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk index 143e89eadc3..ad830f21fa0 100644 --- a/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk +++ b/src/test/skript/tests/regressions/7449-tag lookup only does minecraft.sk @@ -1,4 +1,4 @@ -test "77449 - Tag Lookup Only does Minecraft": +test "7449 - tag lookup only uses minecraft namespace": register an item tag named "my_favorite_blocks" using oak log, stone, and podzol assert tag "my_favorite_blocks" is tag "skript:my_favorite_blocks" with "Tag lookup didn't find a skript tag ""helmets"" namespace" assert tag "helmets" is tag "paper:helmets" with "Tag lookup didn't find a paper tag ""helmets"" namespace" From dd1ab21e5ba995b5fc3105f6bf5e6f8ac5223282 Mon Sep 17 00:00:00 2001 From: Fusezion Date: Thu, 16 Jan 2025 18:10:59 -0500 Subject: [PATCH 4/9] Update ExprTag.java - Remove lookup if namespace is provided --- .../skript/bukkit/tags/elements/ExprTag.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java index 65ab05ae9ce..321582a772d 100644 --- a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java +++ b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java @@ -59,7 +59,7 @@ public class ExprTag extends SimpleExpression { } private Expression names; - TagType[] types; + public TagType[] types; private TagOrigin origin; private boolean datapackOnly; @@ -75,7 +75,6 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is @Override protected Tag @Nullable [] get(Event event) { - String[] names = this.names.getArray(event); List> tags = new ArrayList<>(); String[] namespaces = switch (origin) { @@ -85,22 +84,22 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is case SKRIPT -> new String[]{"skript"}; }; - nextName: for (String name : names) { + nextName: for (String name : this.names.getArray(event)) { // get key NamespacedKey key; + boolean providedNamespace = name.contains(":"); for (String namespace : namespaces) { - if (name.contains(":")) { + if (providedNamespace) { key = NamespacedKey.fromString(name); + if (key == null) + continue nextName; } else { // populate namespace if not provided key = new NamespacedKey(namespace, name); } - if (key == null) - continue; - Tag tag; for (TagType type : types) { - tag = TagModule.tagRegistry.getTag(origin, type, key); + Tag tag = TagModule.tagRegistry.getTag(origin, type, key); if (tag != null // ensures that only datapack/minecraft tags are sent when specifically requested && (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals("minecraft"))) @@ -109,6 +108,8 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is continue nextName; // ensure 1:1 } } + if (providedNamespace) + continue nextName; } } return tags.toArray(new Tag[0]); From 770909f728e85edc626a4e0e3a881f708f398791 Mon Sep 17 00:00:00 2001 From: Fusezion Date: Thu, 16 Jan 2025 18:50:46 -0500 Subject: [PATCH 5/9] Update ExprTag.java - Cleanup lookup code - thanks sovde for explaining in further detail/provide literal code. --- .../skript/bukkit/tags/elements/ExprTag.java | 44 +++++++++---------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java index 321582a772d..030085048aa 100644 --- a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java +++ b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java @@ -85,34 +85,32 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is }; nextName: for (String name : this.names.getArray(event)) { - // get key - NamespacedKey key; - boolean providedNamespace = name.contains(":"); - for (String namespace : namespaces) { - if (providedNamespace) { - key = NamespacedKey.fromString(name); - if (key == null) - continue nextName; - } else { - // populate namespace if not provided - key = new NamespacedKey(namespace, name); - } - - for (TagType type : types) { - Tag tag = TagModule.tagRegistry.getTag(origin, type, key); - if (tag != null - // ensures that only datapack/minecraft tags are sent when specifically requested - && (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals("minecraft"))) - ) { + if (name.contains(":")) { + tags.add(findTag(NamespacedKey.fromString(name))); + } else { + for (String namespace : namespaces) { + Tag tag = findTag(new NamespacedKey(namespace, name)); + if (tag != null) { tags.add(tag); - continue nextName; // ensure 1:1 + continue nextName; } } - if (providedNamespace) - continue nextName; } } - return tags.toArray(new Tag[0]); + return tags.toArray(Tag[]::new); + } + + private Tag findTag(NamespacedKey key) { + for (TagType type : types) { + Tag tag = TagModule.tagRegistry.getTag(origin, type, key); + if (tag != null + // ensures that only datapack/minecraft tags are sent when specifically requested + && (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals("minecraft"))) + ) { + return tag; + } + } + return null; } @Override From 8fb063e3df7e9be5382fa369703ddf6b8f28d6cb Mon Sep 17 00:00:00 2001 From: Fusezion Date: Wed, 29 Jan 2025 08:06:41 -0500 Subject: [PATCH 6/9] Update ExprTag.java - Revert back to package-private --- .../org/skriptlang/skript/bukkit/tags/elements/ExprTag.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java index 030085048aa..ff2da4d413a 100644 --- a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java +++ b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java @@ -59,7 +59,7 @@ public class ExprTag extends SimpleExpression { } private Expression names; - public TagType[] types; + TagType[] types; private TagOrigin origin; private boolean datapackOnly; From a209a103da948e970284a0ecbc6ee78a8bf0554d Mon Sep 17 00:00:00 2001 From: sovdee <10354869+sovdeeth@users.noreply.github.com> Date: Sat, 1 Feb 2025 13:36:20 -0500 Subject: [PATCH 7/9] Update src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java Co-authored-by: Patrick Miller --- .../org/skriptlang/skript/bukkit/tags/elements/ExprTag.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java index ff2da4d413a..79211374358 100644 --- a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java +++ b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java @@ -105,7 +105,7 @@ private Tag findTag(NamespacedKey key) { Tag tag = TagModule.tagRegistry.getTag(origin, type, key); if (tag != null // ensures that only datapack/minecraft tags are sent when specifically requested - && (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals("minecraft"))) + && (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals(NamespacedKey.MINECRAFT))) ) { return tag; } From da384ca6a8683da77c0fb5dd9bcc6c1bb2342e9b Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Sat, 1 Feb 2025 14:30:18 -0500 Subject: [PATCH 8/9] Fix indentation mistakes from merge --- .../org/skriptlang/skript/bukkit/tags/elements/ExprTag.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java index 7e1318c3865..759d839045a 100644 --- a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java +++ b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java @@ -90,11 +90,11 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is }; nextName: for (String name : this.names.getArray(event)) { - boolean invalidKey = false; + boolean invalidKey = false; try { if (name.contains(":")) { NamespacedKey key = NamespacedKey.fromString(name); - invalidKey = key == null; + invalidKey = key == null; if (!invalidKey) { tags.add(findTag(key)); } From ed8716eea0f3458d7f9f2cae22a0f1a26a63e46e Mon Sep 17 00:00:00 2001 From: Patrick Miller Date: Sat, 1 Feb 2025 14:31:34 -0500 Subject: [PATCH 9/9] Mark findTag method as Nullable --- .../org/skriptlang/skript/bukkit/tags/elements/ExprTag.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java index 759d839045a..c812f564dbd 100644 --- a/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java +++ b/src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java @@ -118,7 +118,7 @@ public boolean init(Expression[] expressions, int matchedPattern, Kleenean is return tags.toArray(Tag[]::new); } - private Tag findTag(NamespacedKey key) { + private @Nullable Tag findTag(NamespacedKey key) { for (TagType type : types) { Tag tag = TagModule.tagRegistry.getTag(origin, type, key); if (tag != null