Skip to content

Commit 2a2b90b

Browse files
committed
Address review comments
1 parent a3dac55 commit 2a2b90b

File tree

3 files changed

+33
-25
lines changed

3 files changed

+33
-25
lines changed

godot-core/src/builtin/color.rs

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,25 +32,22 @@ impl Color {
3232
// TODO implement all the other color constants using code generation
3333

3434
/// Transparent black.
35-
pub const TRANSPARENT_BLACK: Color = Self::new(0.0, 0.0, 0.0, 0.0);
35+
pub const TRANSPARENT_BLACK: Color = Self::from_rgba(0.0, 0.0, 0.0, 0.0);
3636

37-
/// Transparent white. Equivalent of `Color.TRANSPARENT` in GDScript.
38-
pub const TRANSPARENT_WHITE: Color = Self::new(1.0, 1.0, 1.0, 0.0);
37+
/// Transparent white.
38+
///
39+
/// _Godot equivalent: `Color.TRANSPARENT`_
40+
pub const TRANSPARENT_WHITE: Color = Self::from_rgba(1.0, 1.0, 1.0, 0.0);
3941

4042
/// Opaque black.
41-
pub const BLACK: Color = Self::new(0.0, 0.0, 0.0, 1.0);
43+
pub const BLACK: Color = Self::from_rgba(0.0, 0.0, 0.0, 1.0);
4244

4345
/// Opaque white.
44-
pub const WHITE: Color = Self::new(1.0, 1.0, 1.0, 1.0);
46+
pub const WHITE: Color = Self::from_rgba(1.0, 1.0, 1.0, 1.0);
4547

4648
/// Constructs a new `Color` with the given components.
47-
pub const fn new(r: f32, g: f32, b: f32, a: f32) -> Self {
48-
Self { r, g, b, a }
49-
}
50-
51-
/// Constructs a new `Color` with the given components. More explicit alias of [`Color::new`].
5249
pub const fn from_rgba(r: f32, g: f32, b: f32, a: f32) -> Self {
53-
Self::new(r, g, b, a)
50+
Self { r, g, b, a }
5451
}
5552

5653
/// Constructs a new `Color` with the given color components, and the alpha channel set to 1.
@@ -59,26 +56,30 @@ impl Color {
5956
}
6057

6158
/// Constructs a new `Color` with the given components as bytes. 0 is mapped to 0.0, 255 is
62-
/// mapped to 1.0. Equivalent of the global constructor function `Color8` in GDScript.
59+
/// mapped to 1.0.
60+
///
61+
/// _Godot equivalent: the global `Color8` function_
6362
pub fn from_rgba8(r: u8, g: u8, b: u8, a: u8) -> Self {
6463
Self::from_rgba(from_u8(r), from_u8(g), from_u8(b), from_u8(a))
6564
}
6665

67-
/// Constructs a new `Color` with the given components as bytes. 0 is mapped to 0.0, 65535
68-
/// (`0xffff`) is mapped to 1.0.
66+
/// Constructs a new `Color` with the given components as `u16` words. 0 is mapped to 0.0,
67+
/// 65535 (`0xffff`) is mapped to 1.0.
6968
pub fn from_rgba16(r: u16, g: u16, b: u16, a: u16) -> Self {
7069
Self::from_rgba(from_u16(r), from_u16(g), from_u16(b), from_u16(a))
7170
}
7271

73-
/// Constructs a new `Color` from a 32-bits value with the given channel `order`. Passing
74-
/// `ColorChannelOrder::Rgba` is the equivalent of `Color.hex` in GDScript.
72+
/// Constructs a new `Color` from a 32-bits value with the given channel `order`.
73+
///
74+
/// _Godot equivalent: `Color.hex`, if `ColorChannelOrder::Rgba` is used_
7575
pub fn from_u32_rgba(u: u32, order: ColorChannelOrder) -> Self {
7676
let [r, g, b, a] = order.unpack(u.to_be_bytes());
7777
Color::from_rgba8(r, g, b, a)
7878
}
7979

80-
/// Constructs a new `Color` from a 64-bits value with the given channel `order`. Passing
81-
/// `ColorChannelOrder::Rgba` is the equivalent of `Color.hex64` in GDScript.
80+
/// Constructs a new `Color` from a 64-bits value with the given channel `order`.
81+
///
82+
/// _Godot equivalent: `Color.hex64`, if `ColorChannelOrder::Rgba` is used_
8283
pub fn from_u64_rgba(u: u64, order: ColorChannelOrder) -> Self {
8384
let [r, g, b, a] = order.unpack(to_be_words(u));
8485
Color::from_rgba16(r, g, b, a)
@@ -109,12 +110,14 @@ impl Color {
109110
///
110111
/// Returns `None` if the string is neither a valid HTML color code nor an existing color name.
111112
///
113+
/// Most color constants have an alpha of 1; use [`Color::with_alpha`] to change it.
114+
///
112115
/// [color_constants]: https://docs.godotengine.org/en/latest/classes/class_color.html#constants
113116
/// [cheat_sheet]: https://raw.githubusercontent.com/godotengine/godot-docs/master/img/color_constants.png
114117
pub fn from_string<S: Into<GodotString>>(string: S) -> Option<Self> {
115118
let color = InnerColor::from_string(
116119
string.into(),
117-
Self::new(f32::NAN, f32::NAN, f32::NAN, f32::NAN),
120+
Self::from_rgba(f32::NAN, f32::NAN, f32::NAN, f32::NAN),
118121
);
119122
// Assumption: the implementation of `from_string` in the engine will never return any NaN
120123
// upon success.
@@ -125,12 +128,17 @@ impl Color {
125128
}
126129
}
127130

128-
/// Equivalent to [`Color::from_hsva`] with `a` set to 1.0.
131+
/// Constructs a `Color` from an [HSV profile](https://en.wikipedia.org/wiki/HSL_and_HSV). The
132+
/// hue (`h`), saturation (`s`), and value (`v`) are typically between 0.0 and 1.0. Alpha is
133+
/// set to 1; use [`Color::with_alpha`] to change it.
129134
pub fn from_hsv(h: f64, s: f64, v: f64) -> Self {
130135
InnerColor::from_hsv(h, s, v, 1.0)
131136
}
132137

133-
/// Equivalent to [`Color::from_ok_hsl`] with `a` set to 1.0.
138+
/// Constructs a `Color` from an [OK HSL
139+
/// profile](https://bottosson.github.io/posts/colorpicker/). The hue (`h`), saturation (`s`),
140+
/// and lightness (`l`) are typically between 0.0 and 1.0. Alpha is set to 1; use
141+
/// [`Color::with_alpha`] to change it.
134142
pub fn from_ok_hsl(h: f64, s: f64, l: f64) -> Self {
135143
InnerColor::from_ok_hsl(h, s, l, 1.0)
136144
}
@@ -241,7 +249,7 @@ impl Color {
241249
}
242250

243251
/// Returns the color with its `r`, `g`, and `b` components inverted:
244-
/// `Color::new(1 - r, 1 - g, 1 - b, a)`.
252+
/// `Color::from_rgba(1 - r, 1 - g, 1 - b, a)`.
245253
#[must_use]
246254
pub fn inverted(self) -> Self {
247255
self.as_inner().inverted()
@@ -465,7 +473,7 @@ impl ops::SubAssign<Color> for Color {
465473
impl ops::Neg for Color {
466474
type Output = Self;
467475
fn neg(self) -> Self {
468-
Self::new(-self.r, -self.g, -self.b, -self.a)
476+
Self::from_rgba(-self.r, -self.g, -self.b, -self.a)
469477
}
470478
}
471479

itest/rust/build.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ fn collect_inputs() -> Vec<Input> {
3939
push!(inputs; float, f32, 12.5);
4040
push!(inputs; float, f64, 127.83156478);
4141
push!(inputs; bool, bool, true);
42-
push!(inputs; Color, Color, Color(0.7, 0.5, 0.3, 0.2), Color::new(0.7, 0.5, 0.3, 0.2));
42+
push!(inputs; Color, Color, Color(0.7, 0.5, 0.3, 0.2), Color::from_rgba(0.7, 0.5, 0.3, 0.2));
4343
push!(inputs; String, GodotString, "hello", "hello".into());
4444
push!(inputs; Vector2, Vector2, Vector2(12.5, -3.5), Vector2::new(12.5, -3.5));
4545
push!(inputs; Vector3, Vector3, Vector3(117.5, 100.0, -323.25), Vector3::new(117.5, 100.0, -323.25));

itest/rust/src/codegen_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ fn codegen_static_builtin_method() {
4545
assert_eq!(pi, GodotString::from("3.142"));
4646

4747
let col = InnerColor::html("#663399cc".into());
48-
assert_eq!(col, Color::new(0.4, 0.2, 0.6, 0.8));
48+
assert_eq!(col, Color::from_rgba(0.4, 0.2, 0.6, 0.8));
4949
}
5050

5151
#[itest]

0 commit comments

Comments
 (0)