Skip to content

Commit 58fb2a5

Browse files
committed
Document why Basis columns are a, b, and c
Why Basis doesn't follow Godot's column name convention came up in Discord, and the reasoning was fairly buried in code reviews and commits in other projects, so I'm sharing the knowledge with API users. The explanation here is lifted directly from godot-rust/gdnative@16aa18e
1 parent 4c8ea83 commit 58fb2a5

File tree

3 files changed

+22
-8
lines changed

3 files changed

+22
-8
lines changed

godot-core/src/builtin/basis.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ use std::ops::{Mul, MulAssign};
2323
///
2424
/// The basis vectors are the columns of the matrix, whereas the [`rows`](Self::rows) field represents
2525
/// the row vectors.
26+
///
27+
/// Note that the names of the column vectors here are `a`, `b`, and `c`, which differs from Godot's convention of `x`, `y`, and `z`. This is
28+
/// because columns are the basis vectors of the transform, while rows represent the X/Y/Z coordinates of each vector. Although basis vectors are
29+
/// the _transformed_ unit vectors of X/Y/Z axes, they have no direct relation to those axes in the _transformed_ coordinate system. Thus, an
30+
/// independent notion of a, b, c does not suggest such a relation. Furthermore, there are sometimes expressions such as `x.x`, `x.y`, `y.x`
31+
/// etc. They are typically hard to read and error-prone to write. Having `a.x`, `a.y`, `b.x` makes things more understandable.
2632
#[derive(Copy, Clone, PartialEq, Debug)]
2733
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
2834
#[repr(C)]
@@ -457,7 +463,7 @@ impl Basis {
457463

458464
/// Returns the first column of the matrix,
459465
///
460-
/// _Godot equivalent: `Basis.x`_
466+
/// _Godot equivalent: `Basis.x`_, see [`Basis`] for why it's changed
461467
#[doc(alias = "x")]
462468
#[must_use]
463469
pub fn col_a(&self) -> Vector3 {
@@ -473,7 +479,7 @@ impl Basis {
473479

474480
/// Returns the second column of the matrix,
475481
///
476-
/// _Godot equivalent: `Basis.y`_
482+
/// _Godot equivalent: `Basis.y`_, see [`Basis`] for why it's changed
477483
#[doc(alias = "y")]
478484
#[must_use]
479485
pub fn col_b(&self) -> Vector3 {
@@ -489,7 +495,7 @@ impl Basis {
489495

490496
/// Returns the third column of the matrix,
491497
///
492-
/// _Godot equivalent: `Basis.z`_
498+
/// _Godot equivalent: `Basis.z`_, see [`Basis`] for why it's changed
493499
#[doc(alias = "z")]
494500
#[must_use]
495501
pub fn col_c(&self) -> Vector3 {

godot-core/src/builtin/transform2d.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ use std::ops::{Mul, MulAssign};
3131
pub struct Transform2D {
3232
/// The first basis vector.
3333
///
34-
/// This is equivalent to the `x` field from godot.
34+
/// _Godot equivalent: `Transform2D.x`_, see [`Basis`][crate::builtin::Basis] for why it's changed
3535
pub a: Vector2,
3636

3737
/// The second basis vector.
3838
///
39-
/// This is equivalent to the `y` field from godot.
39+
/// _Godot equivalent: `Transform2D.y`_, see [`Basis`][crate::builtin::Basis] for why it's changed
4040
pub b: Vector2,
4141

4242
/// The origin of the transform. The coordinate space defined by this transform
@@ -71,7 +71,8 @@ impl Transform2D {
7171

7272
/// Create a new `Transform2D` with the given column vectors.
7373
///
74-
/// _Godot equivalent: `Transform2D(Vector2 x_axis, Vector2 y_axis, Vector2 origin)`_
74+
/// _Godot equivalent: `Transform2D(Vector2 x_axis, Vector2 y_axis, Vector2 origin)`_, see [`Basis`][crate::builtin::Basis] for why it's
75+
/// changed
7576
pub const fn from_cols(a: Vector2, b: Vector2, origin: Vector2) -> Self {
7677
Self { a, b, origin }
7778
}
@@ -291,6 +292,9 @@ impl Transform2D {
291292
}
292293

293294
impl Display for Transform2D {
295+
/// Formats the value with the given formatter. [Read more](https://doc.rust-lang.org/1.79.0/core/fmt/trait.Display.html#tymethod.fmt)
296+
///
297+
/// The output is similar to Godot's, but calls the columns a/b instead of X/Y. See [`Basis`][crate::builtin::Basis] for why.
294298
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
295299
// Godot output:
296300
// [X: (1, 2), Y: (3, 4), O: (5, 6)]

godot-core/src/builtin/transform3d.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ impl Transform3D {
6969

7070
/// Create a new transform from 4 matrix-columns.
7171
///
72-
/// _Godot equivalent: `Transform3D(Vector3 x_axis, Vector3 y_axis, Vector3 z_axis, Vector3 origin)`_
72+
/// _Godot equivalent: `Transform3D(Vector3 x_axis, Vector3 y_axis, Vector3 z_axis, Vector3 origin)`_, see [`Basis`][crate::builtin::Basis]
73+
/// for why it's changed
7374
pub const fn from_cols(a: Vector3, b: Vector3, c: Vector3, origin: Vector3) -> Self {
7475
Self {
7576
basis: Basis::from_cols(a, b, c),
@@ -265,10 +266,13 @@ impl Transform3D {
265266
}
266267

267268
impl Display for Transform3D {
269+
/// Formats the value with the given formatter. [Read more](https://doc.rust-lang.org/1.79.0/core/fmt/trait.Display.html#tymethod.fmt)
270+
///
271+
/// The output is similar to Godot's, but calls the columns a/b/c instead of X/Y/Z. See [`Basis`][crate::builtin::Basis] for why.
268272
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
269273
// Godot output:
270274
// [X: (1, 2, 3), Y: (4, 5, 6), Z: (7, 8, 9), O: (10, 11, 12)]
271-
// Where X,Y,O are the columns
275+
// Where X,Y,Z,O are the columns
272276
let [a, b, c] = self.basis.to_cols();
273277
let o = self.origin;
274278

0 commit comments

Comments
 (0)