[Suggestion] deprecate all in-place mutating methods on Vector/Matrix/... without "InPlace" suffix

Feature requests
1

Yesterday I was chasing a bug for a few hours which looked like this:


let m : BABYLON.Matrix = ...

// do a few things ...

// show a debug box
let box = createBox(...);
let tf = new BABYLON.TransformNode(...);
tf.setPreTransformMatrix(m.invert());
box.parent = tf;

// actual code continues and does something with the Matrix m

This code worked with the debug box, and stopped working when I removed all my debug visualisations.
It was quite a head scratcher.

Can you see the issue immediatly?

Most InPlace mutating methods have the InPlace suffix, like Vector3 add vs addInPlace.
A few exceptions are Vector3 normalizeToNew vs normalize or Matrix where invert mutates in place and there is no ToNew, only invertToRef.

This is made even worse by the fact that invert mutates in place and then returns itself. If it was void-returning, I’d have noticed my mistake much earlier.

Concrete suggestion:

deprecate all similar in-place mutating methods, create new ones with InPlace and/or ToNew suffixes.

Please note that I’m writing this suggestion in anger after wasting half a day yesterday, but I really doubt I’m the only one stumbling over this inconsistency and it would be an improvement to normalize this.

2

@RaananW This would be a cool addition to your new planed packages ?

3

Yes, but only because I know exactly what you are talking about :wink:

Being done this way to allow chaining, but I totally get what you are saying

That’s a great suggestion. We can deprecate them already! and add new functions. Deprecation is just a @deprecated tag in the code.

I do hope everything is fine now. I just want to note that even though the function name might be misleading, the documentation states exactly what the function does.
I know it sounds like I am saying “RTFM”, but it is really not the case. I am sure that many encounter that, and I thank you very much for writing us.

We will be very happy to get a PR with your suggestion :slight_smile: If you don’t have the time, would be great to have an issue on github (or we will create one later).

4

Hi all,
first I just wanted to clarify that the “in anger” part was a joke, tone is sometimes hard in text ;D.

I’m happy to send a PR a bit later if/when I have time, shouldn’t be too complicated, but might take a few days, probably at the end of the week.

Thanks!

5

Please do!!