-
-
Notifications
You must be signed in to change notification settings - Fork 375
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
Colours Are Cool 🌈 #7246
base: dev/feature
Are you sure you want to change the base?
Colours Are Cool 🌈 #7246
Conversation
- Also fixes blending util method using old, wrong scale
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work
src/main/java/org/skriptlang/skript/misc/colours/ColourModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ColourModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ColourModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ColourUtils.java
Outdated
Show resolved
Hide resolved
public static float[] rgbToHsl(Color color) { | ||
float r = color.getRed() / 255f; | ||
float g = color.getGreen() / 255f; | ||
float b = color.getBlue() / 255f; | ||
float max = Math.max(r, Math.max(g, b)); | ||
float min = Math.min(r, Math.min(g, b)); | ||
float h, s, l = (max + min) / 2f; | ||
if (max == min) { | ||
h = s = 0f; | ||
} else { | ||
float delta = max - min; | ||
s = l > 0.5f ? delta / (2f - max - min) : delta / (max + min); | ||
if (max == r) { | ||
h = ((g - b) / delta + (g < b ? 6f : 0f)) / 6f; | ||
} else if (max == g) { | ||
h = ((b - r) / delta + 2f) / 6f; | ||
} else { | ||
h = ((r - g) / delta + 4f) / 6f; | ||
} | ||
} | ||
return new float[]{ h, s, l }; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not return a new Color?
src/main/java/org/skriptlang/skript/misc/colours/ExprBlend.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ExprBlend.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ExprComplement.java
Outdated
Show resolved
Hide resolved
static { | ||
register(ExprHex.class, String.class, "hex [code]", "colors"); | ||
} | ||
|
||
@Override | ||
public @Nullable String convert(Color from) { | ||
return ColourUtils.toHex(from); | ||
} | ||
|
||
@Override | ||
protected String getPropertyName() { | ||
return "hex code"; | ||
} | ||
|
||
@Override | ||
public Class<? extends String> getReturnType() { | ||
return String.class; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this
I think i'd rather see it implemented as taking in a number and returning a string, rather than being restricted to colors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
colour
is used everywhere, and i think skript generally uses color
, as do most people. you should probably replace all colour
mentions with color
src/main/java/org/skriptlang/skript/misc/colours/ColourModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ColourModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ColourModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ColourModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ExprComplement.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colours/ExprComplement.java
Outdated
Show resolved
Hide resolved
Yeah already discussed briefly on Discord - I'll change them all over in my next round of commits 👍 |
- Works for ColorRGBs but not SkriptColors
src/main/java/org/skriptlang/skript/misc/colors/ColorModule.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colors/ColorUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colors/ColorUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colors/ExprComplementaryColor.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/misc/colors/ExprComplementaryColor.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good, great work. Just a few last things. There are a few colour
usages in the tests (and their directory) that we might want to change for consistency.
src/main/java/org/skriptlang/skript/common/colors/ColorModule.java
Outdated
Show resolved
Hide resolved
public class ColorModule { | ||
|
||
public static void load() throws IOException { | ||
Skript.getAddonInstance().loadClasses("org.skriptlang.skript.misc", "colors"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skript.getAddonInstance().loadClasses("org.skriptlang.skript.misc", "colors"); | |
Skript.getAddonInstance().loadClasses("org.skriptlang.skript.misc.colors.elements"); |
I would move the syntax into an elements
subpackage
float max = Math.max(red, Math.max(green, blue)); | ||
float min = Math.min(red, Math.min(green, blue)); | ||
|
||
float hue, saturation, lightness = (max + min) / 2f; // lightness = midpoint of max and min |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might split this onto new lines for clarity
hue = ((red - green) / delta + 4f) / 6f; | ||
} | ||
} | ||
return new float[]{ hue, saturation, lightness }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return new float[]{ hue, saturation, lightness }; | |
return new float[]{hue, saturation, lightness}; |
- Separates the classinfos, functions, and elements registrations - Moves RGB function to ColorModule - Optimizes imports
Description
This PR adds a few colour manipulation expressions and functions. There might be more to come in another PR, because there's plenty left which can still be included, but these seemed (to me) like the important-ish ones. Find below a list of new syntax:
shade
function: shades a colour by a given amount, optionally using HSL adjustmentstint
function: tints a colour by a given amount, optionally using HSL adjustmentscolourBrightness
function: adjusts the brightness of a colour by a given amount. This is similar to, but not the same as, theshade
andtint
functionsgrayscale
function: converts a colour to its grayscale equivalentsepiatone
function: converts a colour to its sepiatone equivalentThis PR also adds
channel
as an option in ExprARGB.I used this script to do some basic testing in the early stages of the PR, so feel free to try it:
Target Minecraft Versions: any
Requirements: none
Related Issues: none