Fix over-zealous refactor

src/main/java/org/skriptlang/skript/bukkit/tags/SkriptTag.java

@@ -11,14 +11,19 @@
 import java.util.HashSet;
 import java.util.Set;
 
+/**
+ * Represents a custom tag created by the Skript user.
+ * Implementation of {@link Tag} with a custom set of contents.
+ * @param <T> The type of the contents.
+ */
 public class SkriptTag<T extends Keyed> implements Tag<T> {
 
 	private final Set<T> contents;
 	private final NamespacedKey key;
 
 	public SkriptTag(NamespacedKey key, Collection<T> contents) {
-		this.contents = new HashSet<>(contents);
 		this.key = key;
+		this.contents = new HashSet<>(contents);
 	}
 
 	@Override

src/main/java/org/skriptlang/skript/bukkit/tags/TagModule.java

@@ -19,8 +19,6 @@
 import org.jetbrains.annotations.Nullable;
 import org.skriptlang.skript.lang.comparator.Comparators;
 import org.skriptlang.skript.lang.comparator.Relation;
-import org.skriptlang.skript.lang.converter.Converter;
-import org.skriptlang.skript.lang.converter.Converters;
 
 import java.io.IOException;
 
@@ -30,15 +28,13 @@ public class TagModule {
 	public static final boolean PAPER_TAGS_EXIST = Skript.classExists("com.destroystokyo.paper.MaterialTags");
 
 	// tag object
-	public static Tags TAGS;
+	public static TagRegistry tagRegistry;
 
 	public static void load() throws IOException {
 		// abort if no class exists
 		if (!Skript.classExists("org.bukkit.Tag"))
 			return;
 
-		// load classes (todo: replace with registering methods after regitration api
-		Skript.getAddonInstance().loadClasses("org.skriptlang.skript.bukkit", "tags");
 
 		// Classes
 		Classes.registerClass(new ClassInfo<>(Tag.class, "minecrafttag")
@@ -63,14 +59,14 @@ public String toVariableNameString(Tag<?> tag) {
 				}
 			}));
 
+		// load classes (todo: replace with registering methods after registration api
+		Skript.getAddonInstance().loadClasses("org.skriptlang.skript.bukkit", "tags");
+
 		// compare tags by keys, not by object instance.
 		Comparators.registerComparator(Tag.class, Tag.class, (a, b) -> Relation.get(a.getKey().equals(b.getKey())));
 
-		// converter to String
-		Converters.registerConverter(Tag.class, String.class, tag -> tag.getKey().toString(), Converter.NO_LEFT_CHAINING);
-
 		// init tags
-		TAGS = new Tags();
+		tagRegistry = new TagRegistry();
 	}
 
 	/**

src/main/java/org/skriptlang/skript/bukkit/tags/Tags.java → src/main/java/org/skriptlang/skript/bukkit/tags/TagRegistry.java

@@ -27,14 +27,14 @@
 /**
  * A class in charge of storing and handling all the tags Skript can access.
  */
-public class Tags {
+public class TagRegistry {
 
 	private final TagSourceMap tagSourceMap = new TagSourceMap();
 
 	/**
 	 * Each new instance will create a new set of tag sources, in an effort to be reload safe.
 	 */
-	public Tags() {
+	TagRegistry() {
 		tagSourceMap.put(TagType.ITEMS, new BukkitTagSource<>("items", TagType.ITEMS));
 		tagSourceMap.put(TagType.BLOCKS, new BukkitTagSource<>("blocks", TagType.BLOCKS));
 		tagSourceMap.put(TagType.ENTITIES, new BukkitTagSource<>("entity_types", TagType.ENTITIES));
@@ -72,15 +72,15 @@ public Tags() {
 	 * @param origin The origin to filter by.
 	 * @param typeClass The class the tags should be applicable to.
 	 * @param types Tag types to check with. Leaving this empty will check all tag types.
-	 * @return Tags from the given origin and types that apply to the given class.
+	 * @return TagRegistry from the given origin and types that apply to the given class.
 	 * @param <T> see typeClass.
 	 */
 	public <T extends Keyed> Iterable<Tag<T>> getTags(TagOrigin origin, Class<T> typeClass, TagType<?>... types) {
 		List<Iterator<Tag<T>>> tagIterators = new ArrayList<>();
 		if (types == null)
 			types = tagSourceMap.map.keys().toArray(new TagType[0]);
 		for (TagType<?> type : types) {
-			if (type.type() == typeClass) {
+			if (typeClass.isAssignableFrom(type.type())) {
 				//noinspection unchecked
 				Iterator<Tag<T>> iterator = getTags(origin, (TagType<T>) type).iterator();
 				if (iterator.hasNext())
@@ -99,7 +99,7 @@ public <T extends Keyed> Iterable<Tag<T>> getTags(TagOrigin origin, Class<T> typ
 	 * Gets all the tags of a specific origin that are of a specific type.
 	 * @param origin The origin to filter by.
 	 * @param type The type of tags to get.
-	 * @return Tags from the given origin that are of the given type.
+	 * @return TagRegistry from the given origin that are of the given type.
 	 * @param <T> The class these tags apply to.
 	 */
 	public <T extends Keyed> Iterable<Tag<T>> getTags(TagOrigin origin, TagType<T> type) {
@@ -114,21 +114,16 @@ public <T extends Keyed> Iterable<Tag<T>> getTags(TagOrigin origin, TagType<T> t
 				return new Iterator<>() {
 					//<editor-fold desc="iterator over tagSources, returning each individual tag">
 					private Iterator<Tag<T>> currentTagIter = tagSources.next().getAllTags().iterator();
-					private Iterator<Tag<T>> nextTagIter;
 
 					@Override
 					public boolean hasNext() {
 						// does the current source have more tags
 						if (currentTagIter.hasNext())
 							return true;
-						// is there another source in the pipeline? if so, check it.
-						if (nextTagIter != null)
-							return nextTagIter.hasNext();
-						// if there's no known next source, check if have one.
-						// if we do, mark it as the next source.
+						// if we have another tag source, mark it as the next source.
 						if (tagSources.hasNext()) {
-							nextTagIter = tagSources.next().getAllTags().iterator();
-							return nextTagIter.hasNext();
+							currentTagIter = tagSources.next().getAllTags().iterator();
+							return currentTagIter.hasNext();
 						}
 						return false;
 					}
@@ -138,12 +133,6 @@ public Tag<T> next() {
 						// if current source has more, get more.
 						if (currentTagIter.hasNext())
 							return currentTagIter.next();
-						// if current source is dry, switch to using the next source
-						if (nextTagIter != null && nextTagIter.hasNext()) {
-							currentTagIter = nextTagIter;
-							nextTagIter = null;
-							return currentTagIter.next();
-						}
 						throw new IllegalStateException("Called next without calling hasNext to set the next tag iterator.");
 					}
 					//</editor-fold>
@@ -158,7 +147,7 @@ public Tag<T> next() {
 	 * @param origin The origin to filter by.
 	 * @param type The type of tags to get.
 	 * @param predicate A predicate to filter the tags with.
-	 * @return Tags from the given origin that are of the given type and that pass the filter.
+	 * @return TagRegistry from the given origin that are of the given type and that pass the filter.
 	 * @param <T> The class these tags apply to.
 	 */
 	public <T extends Keyed> Iterable<Tag<T>> getMatchingTags(TagOrigin origin, TagType<T> type, Predicate<Tag<T>> predicate) {

src/main/java/org/skriptlang/skript/bukkit/tags/TagType.java

@@ -44,9 +44,7 @@ public class TagType<T extends Keyed> {
 	 * @param type The class this tag type applies to.
 	 */
 	public TagType(String pattern, Class<T> type) {
-		this.pattern = pattern;
-		this.type = type;
-		this.toString = pattern;
+		this(pattern, pattern, type);
 	}
 
 	/**
@@ -77,7 +75,7 @@ public String toString() {
 	 * Adds types to the registered tag types.
 	 * @param type The types to add.
 	 */
-	private static void addType(TagType<?>... type) {
+	public static void addType(TagType<?>... type) {
 		REGISTERED_TAG_TYPES.addAll(List.of(type));
 	}
 
@@ -120,6 +118,7 @@ private static void addType(TagType<?>... type) {
 	public static @NotNull String getFullPattern() {
 		return getFullPattern(false);
 	}
+
 	/**
 	 * @param required whether the choice should be optional or required.
 	 * @return Returns a choice pattern for use in Skript patterns. Contains parse marks.

src/main/java/org/skriptlang/skript/bukkit/tags/elements/CondIsTagged.java

@@ -59,8 +59,8 @@ public boolean check(Event event) {
  		return elements.check(event, element -> {
 			boolean isAny = (element instanceof ItemType itemType && !itemType.isAll());
 			Keyed[] values = TagModule.getKeyed(element);
-			 if (values == null)
-				 return false;
+			if (values == null)
+				return false;
 
 			for (Tag<Keyed> tag : tags) {
 				 if (isTagged(tag, values, !isAny)) {

src/main/java/org/skriptlang/skript/bukkit/tags/elements/EffRegisterTag.java

@@ -61,7 +61,7 @@ public class EffRegisterTag extends Effect {
 
 	static {
 		Skript.registerEffect(EffRegisterTag.class,
-				"register [a[n]] [new] [custom] " + TagType.getFullPattern(true) +
+				"register [a[n]] [custom] " + TagType.getFullPattern(true) +
 					" tag named %string% (containing|using) %entitydatas/itemtypes%");
 	}
 
@@ -74,9 +74,7 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean is
 		//noinspection unchecked
 		name = (Expression<String>) expressions[0];
 		if (name instanceof Literal<String> literal) {
-			String key = literal.getSingle();
-			if (key.startsWith("skript:"))
-				key = key.substring(7);
+			String key = removeSkriptNamespace(literal.getSingle());
 			if (!KEY_PATTERN.matcher(key).matches()) {
 				Skript.error("Tag names can only contain the following characters: 'a-z', '0-9', '/', '.', '_', and '-'.");
 				return false;
@@ -94,8 +92,7 @@ protected void execute(Event event) {
 		if (name == null)
 			return;
 
-		if (name.startsWith("skript:"))
-			name = name.substring(7);
+		name = removeSkriptNamespace(name);
 
 		if (!KEY_PATTERN.matcher(name).matches())
 			return;
@@ -121,6 +118,12 @@ protected void execute(Event event) {
 		}
 	}
 
+	private static @NotNull String removeSkriptNamespace(@NotNull String key) {
+		if (key.startsWith("skript:"))
+			key = key.substring(7);
+		return key;
+	}
+
 	@Contract("_, _ -> new")
 	private @NotNull Tag<Material> getMaterialTag(NamespacedKey key, Object @NotNull [] contents) {
 		ThreadLocalRandom random = ThreadLocalRandom.current();
@@ -132,7 +135,8 @@ protected void execute(Event event) {
 				tagContents.add((Material) values[random.nextInt(0, values.length)]);
 			} else {
 				for (Keyed value : values) {
-					tagContents.add((Material) value);
+					if (value instanceof Material material)
+						tagContents.add(material);
 				}
 			}
 		}
@@ -144,7 +148,8 @@ protected void execute(Event event) {
 		List<EntityType> tagContents = new ArrayList<>();
 		for (Object object : contents) {
 			for (Keyed value : TagModule.getKeyed(object)) {
-				tagContents.add((EntityType) value);
+				if (value instanceof EntityType entityType)
+					tagContents.add(entityType);
 			}
 		}
 		return new SkriptTag<>(key, tagContents);
@@ -155,4 +160,5 @@ public String toString(@Nullable Event event, boolean debug) {
 		return "register a new " + type.toString() + " tag named " + name.toString(event, debug) + " containing " +
 				contents.toString(event, debug);
 	}
+
 }

src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTag.java

@@ -97,7 +97,7 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean is
 
 			Tag<?> tag;
 			for (TagType<?> type : types) {
-				tag = TagModule.TAGS.getTag(origin, type, key);
+				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")))

src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTagsOf.java

@@ -50,7 +50,7 @@ public class ExprTagsOf extends PropertyExpression<Object, Tag> {
 
 	static {
 		Skript.registerExpression(ExprTagsOf.class, Tag.class, ExpressionType.PROPERTY,
-				"[all [of]] [the] " + TagOrigin.getFullPattern() + " " + TagType.getFullPattern() + " tags of %itemtype/entity/entitydata%",
+				"[all [[of] the]|the] " + TagOrigin.getFullPattern() + " " + TagType.getFullPattern() + " tags of %itemtype/entity/entitydata%",
 				"%itemtype/entity/entitydata%'[s] " + TagOrigin.getFullPattern() + " " + TagType.getFullPattern() + " tags");
 	}
 
@@ -103,7 +103,7 @@ public <T extends Keyed> Collection<Tag<T>> getTags(@NotNull T value) {
 		List<Tag<T>> tags = new ArrayList<>();
 		//noinspection unchecked
 		Class<T> clazz = (Class<T>) value.getClass();
-		for (Tag<T> tag : TagModule.TAGS.getTags(origin, clazz, types)) {
+		for (Tag<T> tag : TagModule.tagRegistry.getTags(origin, clazz, types)) {
 			if (tag.isTagged(value))
 				tags.add(tag);
 		}
@@ -126,4 +126,5 @@ public String toString(@Nullable Event event, boolean debug) {
 		//noinspection DataFlowIssue
 		return  origin.toString(datapackOnly) + registry + "tags of " + getExpr().toString(event, debug);
 	}
+
 }

src/main/java/org/skriptlang/skript/bukkit/tags/elements/ExprTagsOfType.java

@@ -44,7 +44,7 @@ public class ExprTagsOfType extends SimpleExpression<Tag> {
 
 	static {
 		Skript.registerExpression(ExprTagsOfType.class, Tag.class, ExpressionType.SIMPLE,
-				"[all [[of] the]] " + TagOrigin.getFullPattern() + " " + TagType.getFullPattern() + " tags");
+				"[all [[of] the]|the] " + TagOrigin.getFullPattern() + " " + TagType.getFullPattern() + " tags");
 	}
 
 	TagType<?>[] types;
@@ -63,7 +63,7 @@ public boolean init(Expression<?>[] expressions, int matchedPattern, Kleenean is
 	protected Tag<?> @Nullable [] get(Event event) {
 		Set<Tag<?>> tags = new TreeSet<>(Comparator.comparing(Keyed::key));
 		for (TagType<?> type : types) {
-			for (Tag<?> tag : TagModule.TAGS.getMatchingTags(origin, type,
+			for (Tag<?> tag : TagModule.tagRegistry.getMatchingTags(origin, type,
 				tag -> (origin != TagOrigin.BUKKIT || (datapackOnly ^ tag.getKey().getNamespace().equals("minecraft"))))
 			) {
 				tags.add(tag);
@@ -87,4 +87,5 @@ public String toString(@Nullable Event event, boolean debug) {
 		String registry = types.length > 1 ? "" : types[0].toString();
 		return "all of the " + origin.toString(datapackOnly) + registry + "tags";
 	}
+
 }

src/main/java/org/skriptlang/skript/bukkit/tags/sources/BukkitTagSource.java

@@ -34,4 +34,5 @@ public BukkitTagSource(String registry, TagType<T> type) {
 	public @Nullable Tag<T> getTag(NamespacedKey key) {
 		return Bukkit.getTag(registry, key, getTypes()[0].type());
 	}
+
 }

src/main/java/org/skriptlang/skript/bukkit/tags/sources/PaperTagSource.java

@@ -23,11 +23,11 @@ public class PaperTagSource<T extends Keyed> extends CustomTagSource<T> {
 	 * @param <T> The class of the tags.
 	 */
 	private static <T extends Keyed> @NotNull Iterable<Tag<T>> getPaperTags(@NotNull Iterable<Tag<T>> tags) {
-		List<Tag<T>> modified_tags = new ArrayList<>();
+		List<Tag<T>> modifiedTags = new ArrayList<>();
 		for (Tag<T> tag : tags) {
-			modified_tags.add(new PaperTag<>(tag));
+			modifiedTags.add(new PaperTag<>(tag));
 		}
-		return modified_tags;
+		return modifiedTags;
 	}
 
 	/**

src/main/java/org/skriptlang/skript/bukkit/tags/sources/SkriptTagSource.java

@@ -14,7 +14,7 @@ public class SkriptTagSource<T extends Keyed> extends CustomTagSource<T> {
 	public static final SkriptTagSource<EntityType> ENTITIES = new SkriptTagSource<>(TagType.ENTITIES);
 
 	/**
-	 * @param types  The tag types this source will represent.
+	 * @param types The tag types this source will represent.
 	 */
 	@SafeVarargs
 	public SkriptTagSource(TagType<T>... types) {
@@ -24,4 +24,5 @@ public SkriptTagSource(TagType<T>... types) {
 	public void addTag(Tag<T> tag) {
 		tags.put(tag.getKey(), tag);
 	}
+
 }

src/main/java/org/skriptlang/skript/bukkit/tags/sources/TagOrigin.java

@@ -7,6 +7,10 @@
 
 import java.util.List;
 
+/**
+ * The origin of a tag, eg. from Bukkit, from Paper, from a custom Skript tag, or from anywhere.
+ * Used for classification and filtering tags.
+ */
 public enum TagOrigin {
 	/**
 	 * Bukkit supplies both native minecraft tags and datapack tags.
@@ -84,4 +88,5 @@ public boolean matches(TagOrigin other) {
 			case ANY -> "";
 		};
 	}
+
 }