From e66ad6f4951bad49fe96de8cde59d75ca3e20d40 Mon Sep 17 00:00:00 2001
From: Ayham Al-Ali <20037329+AyhamAl-Ali@users.noreply.github.com>
Date: Mon, 8 Apr 2024 06:38:50 +0300
Subject: [PATCH] Improvements

- RGB pattern now don't require a space after the commas
- renamed couple variables
- added a fail shortcut for dying blocks with RGB
- Removed color-color comparator as it's not the proper fix, it should be fixed in CondCompare
---
 .../classes/data/DefaultComparators.java      | 13 ----------
 .../njol/skript/expressions/ExprColorOf.java  |  1 +
 .../ch/njol/skript/expressions/ExprDyed.java  | 25 ++++++++++++-------
 .../java/ch/njol/skript/util/ColorRGB.java    |  2 +-
 4 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java b/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java
index 282fa632760..8a476d49a9a 100644
--- a/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java
+++ b/src/main/java/ch/njol/skript/classes/data/DefaultComparators.java
@@ -654,19 +654,6 @@ public boolean supportsOrdering() {
 				return false;
 			}
 		});
-
-		// Color - Color
-		Comparators.registerComparator(Color.class, Color.class, new Comparator<Color, Color>() {
-			@Override
-			public Relation compare(Color color1, Color color2) {
-				return Relation.get(color1.getName().equals(color2.getName()));
-			}
-
-			@Override
-			public boolean supportsOrdering() {
-				return false;
-			}
-		});
 	}
 	
 }
diff --git a/src/main/java/ch/njol/skript/expressions/ExprColorOf.java b/src/main/java/ch/njol/skript/expressions/ExprColorOf.java
index 326aa3bccd1..f9294beab42 100644
--- a/src/main/java/ch/njol/skript/expressions/ExprColorOf.java
+++ b/src/main/java/ch/njol/skript/expressions/ExprColorOf.java
@@ -88,6 +88,7 @@ public boolean init(Expression<?>[] exprs, int matchedPattern, Kleenean isDelaye
 	
 	@Override
 	protected Color[] get(Event event, Object[] source) {
+		// TODO FIX
 		// this approach has couple issues, users can't use multiple types in source like
 		// 'broadcast colors of ((trailing burst firework colored blue and red) and targeted block)' and second
 		// this check doesn't work with variables as it's not converted yet
diff --git a/src/main/java/ch/njol/skript/expressions/ExprDyed.java b/src/main/java/ch/njol/skript/expressions/ExprDyed.java
index 7a2599e92b1..e81a93d35e9 100644
--- a/src/main/java/ch/njol/skript/expressions/ExprDyed.java
+++ b/src/main/java/ch/njol/skript/expressions/ExprDyed.java
@@ -29,6 +29,7 @@
 import ch.njol.skript.lang.SkriptParser.ParseResult;
 import ch.njol.skript.lang.util.SimpleExpression;
 import ch.njol.skript.util.Color;
+import ch.njol.skript.util.ColorRGB;
 import ch.njol.util.Kleenean;
 import org.bukkit.Material;
 import org.bukkit.event.Event;
@@ -42,11 +43,12 @@
 import java.util.regex.Matcher;
 
 @Name("Dyed")
-@Description("An expression to return items/entities with a color.")
+@Description("An expression to return colored items.")
 @Examples({
 		"give player leather chestplate dyed red",
 		"give player potion of invisibility dyed rgb 200, 70, 88",
-		"give player filled map with color rgb(20, 60, 70)"
+		"give player filled map colored rgb(20, 60, 70)",
+		"give player wool painted red"
 })
 @Since("INSERT VERSION")
 public class ExprDyed extends SimpleExpression<ItemType> {
@@ -56,8 +58,10 @@ public class ExprDyed extends SimpleExpression<ItemType> {
 	static {
 		Skript.registerExpression(ExprDyed.class, ItemType.class, ExpressionType.COMBINED, "%itemtypes% (dyed|painted|colo[u]red) %color%");
 	}
-	
+
+	@SuppressWarnings("NotNullFieldNotInitialized")
 	private Expression<ItemType> items;
+	@SuppressWarnings("NotNullFieldNotInitialized")
 	private Expression<Color> color;
 
 	@Override
@@ -75,17 +79,17 @@ protected ItemType[] get(Event event) {
 		if (color == null)
 			return new ItemType[0];
 
-		ItemType[] targets = this.items.getArray(event);
+		ItemType[] items = this.items.getArray(event);
 		org.bukkit.Color bukkitColor;
 		bukkitColor = color.asBukkitColor();
 
-		for (ItemType item : targets) {
+		for (ItemType item : items) {
 			ItemMeta meta = item.getItemMeta();
 
 			if (meta instanceof LeatherArmorMeta) {
-				LeatherArmorMeta m = (LeatherArmorMeta) meta;
-				m.setColor(bukkitColor);
-				item.setItemMeta(m);
+				LeatherArmorMeta leatherArmorMeta = (LeatherArmorMeta) meta;
+				leatherArmorMeta.setColor(bukkitColor);
+				item.setItemMeta(leatherArmorMeta);
 			} else if (meta instanceof MapMeta && MAPS_AND_POTIONS_COLORS) {
 				MapMeta mapMeta = (MapMeta) meta;
 				mapMeta.setColor(bukkitColor);
@@ -95,6 +99,9 @@ protected ItemType[] get(Event event) {
 				potionMeta.setColor(bukkitColor);
 				item.setItemMeta(potionMeta);
 			} else {
+				if (color instanceof ColorRGB) // currently blocks don't support RGB
+					continue;
+
 				Material material = item.getMaterial();
 				Matcher matcher = ExprColorOf.MATERIAL_COLORS_PATTERN.matcher(material.name());
 				if (!matcher.matches())
@@ -105,7 +112,7 @@ protected ItemType[] get(Event event) {
 				} catch (IllegalArgumentException ignored) {}
 			}
 		}
-		return targets;
+		return items;
 	}
 
 	@Override
diff --git a/src/main/java/ch/njol/skript/util/ColorRGB.java b/src/main/java/ch/njol/skript/util/ColorRGB.java
index 0658c41d7b9..978b2f9c643 100644
--- a/src/main/java/ch/njol/skript/util/ColorRGB.java
+++ b/src/main/java/ch/njol/skript/util/ColorRGB.java
@@ -32,7 +32,7 @@
 
 public class ColorRGB implements Color {
 
-	private static final Pattern RGB_PATTERN = Pattern.compile("(?>rgb|RGB) (\\d+), (\\d+), (\\d+)");
+	private static final Pattern RGB_PATTERN = Pattern.compile("(?>rgb|RGB) (\\d+), ?(\\d+), ?(\\d+)");
 
 	private org.bukkit.Color bukkit;
 	@Nullable