Refactor vector_macros to consistently generate all vector types #721

joriskleiber · 2024-05-22T18:49:50Z

I have reworked vector_macros so that all functions and constants that are available in godot are also available in rust for all vector types.

I have grouped functions and constants depending on which vector types they can be found on.

For all functions for which an implementation was already available, I added benchmarks and checked if both return the same result. I then named the new functions new_*.

I haven't implemented all the functions yet because I wanted to get some feedback first, but when I'm done glam-rs should be able to be removed.

GodotRust · 2024-05-22T18:56:18Z

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-721

Bromeon

Thank you for the PR!

May I ask what the motivation behind this refactoring is? As mentioned in the Contribution guidelines, it would be good if larger changes were briefly discussed beforehand, to make sure our ideas are aligned and no work is wasted.

I haven't implemented all the functions yet because I wanted to get some feedback first, but when I'm done glam-rs should be able to be removed.

I'm not sure if this should be a goal. glam is relatively lightweight and battle-tested. Implementing all its functionality on our side not only means reinventing the wheel, but it comes with some increased maintenance and bug risk (albeit little, but our testing is lacking too). We may also lose out on potential optimizations that glam is doing -- either now, or in case we decide to use Vec3A or similar APIs.

Your benchmark indicates that the new implementation is faster in many cases. Did you run it in release mode? If yes, what do you think is the bottleneck of the existing implementation -- maybe indirections through glam2 etc? Do you see any change if you #[inline(always)] those?

I'm not sure if moving the associated constants to macros is a good idea -- from user/API perspective, this is an objective downgrade, as the documentation comments are now very generic (basically not expressing more than the name already does). I don't think there's a big problem with duplicating them 🙂

On a positive side, I think reducing duplication between the vector types is nice if it doesn't come with worse docs or significant complexity increase (thinking of things like strip_leading_plus, which can be OK in isolated instances but not too much 🙂 )

Bromeon · 2024-05-22T19:12:54Z

godot-core/src/builtin/vectors/vector_macros.rs

+            /// Returns the length (magnitude) of this vector.
+            #[inline]
+            pub fn new_length(self) -> real {
+                (strip_leading_plus!($( + (self.$comp * self.$comp) )*) as real).sqrt()


I guess $( (self.$comp * self.$comp) )+* doesn't work?

Also, as real should be replaced with RealConv functions.

Yes, the + gets interpretet as any number of repetitions but at least one and you can't escape it.

I don't quite understand how I should be replacing as real with RealConv, since the return type is also real.

why do you need as real here actually? does rust just not understand what the type is?

The impl_vector_fns macro implements functions for floating-point and integer vectors. Because the result of strip_leading_plus!($( + (self.$comp * self.$comp) )*) for integer vectors is i32 and sqrt doesn't exists on i32 it needs to be casted. Since there is no performance impact for floating-point vectors i didn't see the benefit to splitting this, as the only difference would be the type casting.

Bromeon · 2024-05-22T19:52:52Z

Since this has the potential of having a huge diff, it would also be nice if you could separate it into multiple PRs, to accelerate the review process:

one with the refactoring (code deduplication and change to glam)
- this is the one to be benchmarked
API additions (like the MODEL_* constants)
any other semantic changes/bugfixes (if applicable)

(But maybe let's finish the discussion on the overarching idea first 🙂 )

joriskleiber · 2024-05-22T21:57:49Z

May I ask what the motivation behind this refactoring is? As mentioned in the Contribution guidelines, it would be good if larger changes were briefly discussed beforehand, to make sure our ideas are aligned and no work is wasted.

Sorry, I was browsing the project and saw #310, experimented a bit and then got carried away. Next time I will definitely do it.

My main goal in moving everything into macros is to make it easier to add new methods and maintain existing ones. Since they ussaly only exist once.

I don't feel strongly about removing glam but I don't necessarily see the benefit of using it's API as it sometimes diverges from godots.

Your benchmark indicates that the new implementation is faster in many cases. Did you run it in release mode? If yes, what do you think is the bottleneck of the existing implementation -- maybe indirections through glam2 etc? Do you see any change if you #[inline(always)] those?

Small improvements come from eliminating the overhead of converting to glam and back, larger ones come from faster implementations based on godots implementations (limit_length vs. clamp_length_max) or making use of the procedural aspect of macros. ~~In my testing, release mode and #[inline(always)] didn't really change anything.~~

Since I based everything on godots API, I would propose that I add integration tests to all functions.

I'm not sure if moving the associated constants to macros is a good idea -- from user/API perspective, this is an objective downgrade, as the documentation comments are now very generic (basically not expressing more than the name already does). I don't think there's a big problem with duplicating them 🙂

Only ZERO and ONE are shared between all the Vectors and I think most of the time I copied godots documentation word for word. Can you maybe point me to an example where they regressed?

lilizoey · 2024-05-23T14:58:32Z

Your benchmark indicates that the new implementation is faster in many cases. Did you run it in release mode? If yes, what do you think is the bottleneck of the existing implementation -- maybe indirections through glam2 etc? Do you see any change if you #[inline(always)] those?

Small improvements come from eliminating the overhead of converting to glam and back, larger ones come from faster implementations based on godots implementations (limit_length vs. clamp_length_max) or making use of the procedural aspect of macros. In my testing, release mode and #[inline(always)] didn't really change anything.

It'd be strange if inline has no impact, like just checking the first method on your list abs, to me it looks like the new implementation and glam's is identical. They both just call abs on each component, and yet the new one is faster?

Would you be able to clean up the data a bit more? Maybe remove any results that have no measurable difference, make sure functions are inline properly, and include how big the performance improvement is for each? As is it's not all that easy to read all the data.

joriskleiber · 2024-05-23T16:45:46Z

I forgot to tell godot to use the release build and I misinterpreted what functions to inline. Sorry about that.

So yes, most of the improvements disappear when inlining the existing function and running in release mode. Except for cases where the godot implementation is faster.

Bromeon · 2024-05-24T19:50:35Z

Only ZERO and ONE are shared between all the Vectors and I think most of the time I copied godots documentation word for word. Can you maybe point me to an example where they regressed?

You're right, I think they're good -- I remember Godot's docs being confusing for some constants, but it was probably somewhere else. Thanks for the deduplication! 👍

So yes, most of the improvements disappear when inlining the existing function and running in release mode. Except for cases where the godot implementation is faster.

So that brings us back to the motivation of this PR.

Performance can make sense in the cases you mention. But maybe we should look at those individually, based on benchmark results? If inlining improves perf over current master, let's do it too.

Removing glam is where I tend to be against. After thinking a bit more about it, there are several reasons why I'm not convinced it's a very good idea:

It shifts extra maintenance to us. While I agree that the cost for most vector operations is probably small, the benefit is, too.

We cannot look at vectors isolated; we use many glam types in matrix and quaternion contexts:

gdext/godot-core/src/builtin/real.rs

Lines 144 to 167 in fd5128b

    
               /// A 2-dimensional vector from [`glam`]. Using a floating-point format compatible with [`real`]. 
        
               pub type RVec2 = glam::DVec2; 
        
               /// A 3-dimensional vector from [`glam`]. Using a floating-point format compatible with [`real`]. 
        
               pub type RVec3 = glam::DVec3; 
        
               /// A 4-dimensional vector from [`glam`]. Using a floating-point format compatible with [`real`]. 
        
               pub type RVec4 = glam::DVec4; 
        
               /// A 2x2 column-major matrix from [`glam`]. Using a floating-point format compatible with [`real`]. 
        
               pub type RMat2 = glam::DMat2; 
        
               /// A 3x3 column-major matrix from [`glam`]. Using a floating-point format compatible with [`real`]. 
        
               pub type RMat3 = glam::DMat3; 
        
               /// A 4x4 column-major matrix from [`glam`]. Using a floating-point format compatible with [`real`]. 
        
               pub type RMat4 = glam::DMat4; 
        
               /// A matrix from [`glam`] quaternion representing an orientation. Using a floating-point format 
        
               /// compatible with [`real`]. 
        
               pub type RQuat = glam::DQuat; 
        
               /// A 2D affine transform from [`glam`], which can represent translation, rotation, scaling and 
        
               /// shear. Using a floating-point format compatible with [`real`]. 
        
               pub type RAffine2 = glam::DAffine2; 
        
               /// A 3D affine transform from [`glam`], which can represent translation, rotation, scaling and 
        
               /// shear. Using a floating-point format compatible with [`real`]. 
        
               pub type RAffine3 = glam::DAffine3;

Reinventing those comes with significant development, maintenance and support effort.

I don't want to establish a general Not-Invented-Here practice 😉 We do re-implement quite a bit of Godot and Rust stuff, but it's usually because we get significant performance gains, can solve something more idiomatically or avoid a heavy crate dependency. In case of glam, the library is quite lightweight and very tailored to our needs.

Of course, general improvements such as adding MODEL_ constants, or more tests are very appreciated. Don't feel obliged to widen the scope too much, it's perfectly fine to do some of those in separate PRs or later 🙂

Summary: to move this forward, I would suggest:

see if there are changes to the existing implementation (e.g. inlining) that have significant impact in Release mode
once that's done, isolate the functions where a manual implementation still improves over the current code
based on the outcome of that, we can make a decision how many/which functions we want to rewrite

Does that sound good?

joriskleiber · 2024-05-26T00:16:31Z

I went through all function benchmarks in release mode and inline the functions where I found performance differences. Even though the differences in performance are small, they are the same for every benchmark run.

length
only the Vector4i implementation results divigate from the rest, i have no idea why but they are consistent

   -- vec4i_old_length           ...     1ns     1ns
   -- vec4i_new_length           ...     0ns     0ns

normalized
my guess would be that Godot's implementation is simply faster then glam's

   -- vec2_old_normalized        ...     2ns     2ns
   -- vec2_new_normalized        ...     1ns     1ns

   -- vec3_old_normalized        ...     2ns     2ns
   -- vec3_new_normalized        ...     1ns     1ns

since direction_to uses normalized these results are expected, when using new_normalized these differences disappear

   -- vec2_old_direction_to      ...     2ns     2ns
   -- vec2_new_direction_to      ...     1ns     1ns

   -- vec3_old_direction_to      ...     3ns     3ns
   -- vec3_new_direction_to      ...     1ns     1ns

Based on these findings I would suggest to only rewrite normalized.

I would continue as follow and create multiple commits for all these steps if given the ok:

cleaning up vector_macros and use the glam implementations
deduplicate vec2 and vec2i with the new macros
deduplicate vec3 and vec3i with the new macros
deduplicate vec4 and vec4i with the new macros
~~move even more impl to the macros for deduplication (GodotFfi, GlamConv, GlamType)~~
~~add from and into impl to the macros~~
add tests (maybe also with macros?)

Some functions currently only exist on some vector types and not in Godot (e.g. coords), should i remove these?

Bromeon · 2024-05-26T07:16:58Z

Based on these findings I would suggest to only rewrite normalized.

I think that makes sense. glam does this:

    pub fn normalize_or(self, fallback: Self) -> Self {
        let rcp = self.length_recip();
        if rcp.is_finite() && rcp > 0.0 {
            self * rcp
        } else {
            fallback
        }
    }

I'm not sure how rcp can be negative, it would mean 1 / length < 0, i.e. length < 0. Maybe it is to counter numerical inaccuracies? We probably don't need that check...

Btw, we should likely change how we do our benchmark, because these numbers are subject to huge error.

   -- vec4i_old_length           ...     1ns     1ns
   -- vec4i_new_length           ...     0ns     0ns

This has almost no expression power. It's unclear if it's the difference between 0.49 and 0.51 ns, or between 0.001 and 1.4...

Anyway, this is out of scope for this PR.

Some functions currently only exist on some vector types and not in Godot (e.g. coords), should i remove these?

They seemed like a good idea at the time, but yes, feel free to remove coords. I will probably re-add something like them in one way or another, but it should be more thought out.

If there are other functions you want to remove, please mention them explicitly. It's easy to miss something in the diff otherwise.

Bromeon · 2024-05-26T07:26:45Z

I would continue as follow and create multiple commits for all these steps if given the ok:

cleaning up vector_macros and use the glam implementations

deduplicate vec2 and vec2i with the new macros

deduplicate vec3 and vec3i with the new macros

deduplicate vec4 and vec4i with the new macros

move even more impl to the macros for deduplication (GodotFfi, GlamConv, GlamType)

add from and into impl to the macros

add tests (maybe also with macros?)

Are there any additions/removals, or other semantic changes compared to the master branch? If yes, I would prefer if refactoring + semantic changes could be separate PRs, it's near-impossible to track potential regressions otherwise, and it also makes review longer.

I'm against adding additional From impls.

Regarding move-to-macro, I'm not sure if we benefit much from doing this for everything. For methods/constants/operators it can help keep things consistent, but I would not do it for the 3 traits you mentioned. Otherwise it becomes really annoying if we want to customize one of them. In general, code duplication is only a problem where it actively hurts maintenance -- we shouldn't go on a crusade based on principle 😉

But yes, other than that your suggestion sounds good 🙂 feel free to use separate commits if it helps clarity. Thanks a lot!

joriskleiber · 2024-05-26T19:19:43Z

This is the vector_macros refactor commit. I know its a huge commit and I hope it isn't too much of a hassle to review it.

Based on this refactor i would gradually refactor the vectors.

Bromeon · 2024-05-26T19:24:02Z

Thanks! And sorry about the conflicts, I was also refactoring a small part around the Vector*Axis and swizzle! parts.

Bromeon

Thanks a lot! Also for the doc improvements all over the place 👍

I made some comments.

godot-core/src/builtin/vectors/vector_macros.rs

Bromeon · 2024-05-26T19:39:06Z

godot-core/src/builtin/vectors/vector_macros.rs

+            /// Returns the axis of the vector's highest value. See [`Vector2Axis`] enum. If all components are equal, this method returns [`Vector2Axis::X`].
+            #[inline]
+            #[doc(alias = "max_axis_index")]
+            pub fn max_axis(self) -> $crate::builtin::Vector2Axis {
+                use $crate::builtin::Vector2Axis;
+
+                if self.x < self.y {
+                    Vector2Axis::Y
+                } else {
+                    Vector2Axis::X
+                }


This changes semantics and signature. Current implementation is:

pub fn max_axis(self) -> Option<Vector2Axis> { match self.x.cmp(&self.y) { Ordering::Less => Some(Vector2Axis::Y), Ordering::Equal => None, Ordering::Greater => Some(Vector2Axis::X), } }

It's relatively easy to do .unwrap_or() on user side, but gives more control about the fallback than the Godot impl.

Handling equality may be especially relevant for the integer vectors.

Since f32 doesn't implement cmp should the implementations be split or would this be sufficent:

pub fn max_axis(self) -> std::option::Option<$crate::builtin::Vector2Axis> { use core::cmp::Ordering; use $crate::builtin::Vector2Axis; match self.x.partial_cmp(&self.y) { Some(Ordering::Less) => Some(Vector2Axis::Y), Some(Ordering::Equal) => None, Some(Ordering::Greater) => Some(Vector2Axis::X), _ => None, } }

I think that should be good enough. The caveat is of course that with float imprecision, exact equality is unlikely as soon as arithmetic is involved, but I think that's fine.

Btw, since the macros are not user-facing but instantiated locally, you don't need to fully qualify all the symbols. Just import them at the beginning of the file -- that makes things quite a bit easier.

What I was wondering also is, should we document how we can get the Godot behavior of max_axis_index using this function? With unwrap_or?

How about?

/// To mimic Godot's behavior unwrap this function with`unwrap_or(Vector2Axis::Y)`.

Sounds great, thanks!

Tiny adjustment (also space/comma):

/// To mimic Godot's behavior, unwrap this function's result with `unwrap_or(Vector2Axis::Y)`.

Btw, maybe wait with resolving conversations that are still active 🙂

godot-core/src/builtin/vectors/vector_macros.rs

Bromeon · 2024-05-26T19:51:34Z

godot-core/src/builtin/vectors/vector_macros.rs

+            pub fn snapped(self, step: Self) -> Self {
+                Self::new(
+                    $(
+                        (self.$comp as real).snapped(step.$comp as real) as $Scalar
+                    ),*
+                )
+            }


Why does this need an as real now?

Previous implementation:

gdext/godot-core/src/builtin/vectors/vector_macros.rs

Lines 425 to 429 in 4f9f47f

pub fn snapped(self, step: Self) -> Self {

Self::new(

$( self.$comp.snapped(step.$comp) ),*

)

}

The previous implementation was only for float vectors. Since self.$comp can now also be a i32 and snapped only exists on real it needs to be converted.

I don't think code deduplication should come at the cost of introducing floating-point operations for integer vectors. This is a potential issue for precision and performance.

Maybe it can be implemented separately? Otherwise omit snapped for integer vectors, for now...

I don't think its possible to implement it any other way since snapped requires division and Godot's implementation is the same. But I can omit it for now.

Btw. while looking at Godot's snapped implementation I noticed that it produces different results to the real implementation

((self / step + 0.5) * step).floor() vs (self / step + 0.5).floor() * step)

We can gladly correct our float impl to match Godot's.

Regarding integer impl, something along this lines? Didn't test much, so almost certainly bugged, but it should be possible without float operations. Division doesn't imply floating point. We could of course look at existing impls as well...

I don't think its possible to implement it any other way since

Or I'm simply not smart enough😅

Didn't test much, so almost certainly bugged

Only found that it breaks when steps are negative

Ah, are negative steps supported in Godot? 🤔

As far as I can tell, yes e.g. if you snap 7 to 2, you get the higher possible result (8 and not 6) but with -2 you get the lower on (6 and not 8).

The Godot documentation doesn't mention negative values, but they are definitely supported by design and not by accident. For a long time: I found godotengine/godot#6399 from a time where issue numbers were still 4 digits... 🏹

Maybe we could test for a few values that the inner function (generated) returns the same result...

Bromeon · 2024-05-27T09:30:09Z

Could you maybe rebase this onto master and fix the merge conflicts?

joriskleiber · 2024-05-29T15:58:09Z

Should I push the other vector refactors or would you like to review the commits individually?

Bromeon · 2024-05-29T18:15:51Z

You can gladly push more commits on top. Would be nice however if you could fix the merge conflicts (should be a small one this time), and if CI is green -- that way, the docs can be generated and we can inspect the results in the link from the GodotRust bot 🙂

Bromeon

The PR looks mostly good, a few small comments remaining.

Are things ready from your side, or you plan to do more changes? As mentioned, unrelated improvements can also be applied in separate PRs. Would be cool to merge this soon 😎

Bromeon · 2024-06-02T15:47:23Z

godot-core/src/builtin/vectors/vector_macros.rs

+            /// Performs a cubic interpolation between this vector and `b` using `pre_a` and `post_b` as handles,
+            /// and returns the result at position `weight`. `weight` is on the range of 0.0 to 1.0, representing the amount of interpolation.


Maybe this could also get a 1-line-brief description 🤔

godot-core/src/builtin/vectors/vector_macros.rs

joriskleiber · 2024-06-03T23:58:22Z

I expected sign() to fail on all float vectors since Godot treats both +0 and -0 as 0 while glam treats them as +1 and -1.
But I'm a little surprised that so many functions only fail on macOS 🤔

joriskleiber · 2024-06-04T00:35:07Z

Replacing assert_eq with assert_eq_approx seems to have fixed the failing tests.
My remaining question is whether sign() should mimic Godot's or glam's behavior. In my opinion, Godot's is way more useful.

Bromeon · 2024-06-04T08:07:58Z

My remaining question is whether sign() should mimic Godot's or glam's behavior. In my opinion, Godot's is way more useful.

For posterity:

Godot sign: returns 0 for input 0
glam int signum: returns 0 for input 0
glam float signum: returns -1 for -0, and +1 for +0 🤪

100% agree that Godot's behavior is better. See also previous discussion.

Bromeon · 2024-06-05T13:20:27Z

Fixed the failing test for the double conversion (only run in full CI) and hid an accidentally public glam conversion function.

Thank you so much for this PR -- it's a great enhancement to consistency, documentation and correctness! 💪

joriskleiber · 2024-06-05T17:07:46Z

Could #310 be updated? Since, except for snapped for integer vectors, everything should be implemented.

Bromeon · 2024-06-05T20:41:52Z

Done! ✔️

joriskleiber force-pushed the refactor-vectors branch from 8ae92ef to 56e0753 Compare May 22, 2024 18:59

Bromeon added feature Adds functionality to the library c: core Core components labels May 22, 2024

Bromeon reviewed May 22, 2024

View reviewed changes

Bromeon added quality-of-life No new functionality, but improves ergonomics/internals and removed feature Adds functionality to the library labels May 22, 2024

joriskleiber force-pushed the refactor-vectors branch from 56e0753 to fc83bb2 Compare May 26, 2024 19:10

joriskleiber force-pushed the refactor-vectors branch from fc83bb2 to 3ec1fda Compare May 26, 2024 19:53

Bromeon reviewed May 26, 2024

View reviewed changes

joriskleiber force-pushed the refactor-vectors branch from 3ec1fda to f120e7d Compare May 26, 2024 23:07

joriskleiber force-pushed the refactor-vectors branch 3 times, most recently from f61e21e to c00663b Compare May 27, 2024 22:46

joriskleiber force-pushed the refactor-vectors branch 2 times, most recently from 2323e19 to b21f9ee Compare May 29, 2024 19:19

joriskleiber marked this pull request as ready for review May 29, 2024 19:30

joriskleiber force-pushed the refactor-vectors branch 3 times, most recently from ba217ea to 9d93265 Compare June 2, 2024 02:10

Bromeon reviewed Jun 2, 2024

View reviewed changes

joriskleiber force-pushed the refactor-vectors branch from 9d93265 to 7f2e5ca Compare June 3, 2024 23:38

joriskleiber added 5 commits June 4, 2024 01:40

Refactor vector_macros

5ec59f3

Refactor 2D vectors with new macros

ccf7745

Refactor 3D vectors with new macros

f777cfa

Refactor 4D vectors with new macros

2a8d97b

Fix some vector refactor errors

22f6880

joriskleiber force-pushed the refactor-vectors branch from 75ac810 to bba7b35 Compare June 3, 2024 23:40

Add vector tests

064511f

joriskleiber force-pushed the refactor-vectors branch from bba7b35 to 064511f Compare June 4, 2024 00:25

Fix sign for float vectors

90665f8

Bromeon added this pull request to the merge queue Jun 5, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 5, 2024

Fix vector2_test::project, hide to_glam_real()

e9d5b92

Bromeon added this pull request to the merge queue Jun 5, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 5, 2024

Bromeon added this pull request to the merge queue Jun 5, 2024

Merged via the queue into godot-rust:master with commit 6c92a05 Jun 5, 2024
15 checks passed

joriskleiber mentioned this pull request Jun 16, 2024

Add snapped to integer vectors #768

Merged

Bromeon mentioned this pull request Jul 8, 2024

[Feature Request] Consider adding a feature flag for enhanced determinism option of glam #789

Closed

	pub fn snapped(self, step: Self) -> Self {
	Self::new(
	$( self.$comp.snapped(step.$comp) ),*
	)
	}

		/// Performs a cubic interpolation between this vector and `b` using `pre_a` and `post_b` as handles,
		/// and returns the result at position `weight`. `weight` is on the range of 0.0 to 1.0, representing the amount of interpolation.

Refactor vector_macros to consistently generate all vector types #721

Refactor vector_macros to consistently generate all vector types #721

Conversation

joriskleiber commented May 22, 2024 • edited Loading

GodotRust commented May 22, 2024

Bromeon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joriskleiber May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Bromeon commented May 22, 2024 • edited Loading

joriskleiber commented May 22, 2024 • edited Loading

lilizoey commented May 23, 2024 • edited Loading

joriskleiber commented May 23, 2024 • edited Loading

Bromeon commented May 24, 2024

joriskleiber commented May 26, 2024 • edited Loading

Bromeon commented May 26, 2024

Bromeon commented May 26, 2024 • edited Loading

joriskleiber commented May 26, 2024

Bromeon commented May 26, 2024

Bromeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bromeon May 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joriskleiber May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Bromeon May 27, 2024 • edited Loading

Choose a reason for hiding this comment

Bromeon commented May 27, 2024

joriskleiber commented May 29, 2024

Bromeon commented May 29, 2024

Bromeon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joriskleiber commented Jun 3, 2024

joriskleiber commented Jun 4, 2024

Bromeon commented Jun 4, 2024

Bromeon commented Jun 5, 2024

joriskleiber commented Jun 5, 2024

Bromeon commented Jun 5, 2024

joriskleiber commented May 22, 2024 •

edited

Loading

Bromeon left a comment •

edited

Loading

joriskleiber May 23, 2024 •

edited

Loading

Bromeon commented May 22, 2024 •

edited

Loading

joriskleiber commented May 22, 2024 •

edited

Loading

lilizoey commented May 23, 2024 •

edited

Loading

joriskleiber commented May 23, 2024 •

edited

Loading

joriskleiber commented May 26, 2024 •

edited

Loading

Bromeon commented May 26, 2024 •

edited

Loading

Bromeon May 26, 2024 •

edited

Loading

joriskleiber May 27, 2024 •

edited

Loading

Bromeon May 27, 2024 •

edited

Loading