ObservableVec3 in lite

Questions
1

I wasn’t sure what category this is: Question? Feature request? It’s more like a “musing”. :sweat_smile:

I noticed that the current API for SceneNode.position, scaling, etc have transitioned from assignment of tuples to interaction with ObservableVec3. I understand why the dirty implementation was added. However, I do miss the simplicity of tuple assignment (and consistency with colors, et al). My own code happens to use tuples before it hands them off to Babylonjs (full or lite), so the short-lived tuple API was welcome.

Have you considered using a getter/setter approach? Here’s an example that would allow both idioms to coexist.

Alternately, you could return to tuple-only approach and still maintain dirty semantics through accessors. If you went that route, I would consider making the setter copy and the getter return a ReadonlyArray, so that lite users don’t fall victim to one of the classic Babylon foot-guns: accidentally sharing ownership of a Vector3. It also prevents non-working assignments like position[0] = 1.

As an aside, with the current implementation it may make sense to declare the ObservableVec3’s readonly, so that users don’t accidentally disconnect the onDirty callback by assigning to the position property.

interface SceneNode {
readonly position: ObservableVec3;
}

2

Yup !

In Lite, the ObservableVec3 instance is part of the node’s dirty tracking, so position and scaling are meant to be mutated, not replaced:

box.position.set(x, y, z);
box.scaling.set(x, y, z);

I agree the writable property type is misleading, so these handles should be marked readonly. Do you want to do a PR? I’ll merge it

Copy-on-assignment is reasonable ergonomically, and the compat layer already follows that pattern, but I’d keep it separate rather than switching Lite back to tuple-valued transforms. Again we aim for size not convenience

3

Thanks!

4

(Note: This isn’t me arguing, I just want to understand what’s being optimized for)

When you say that size is prioritized over convenience, aren’t accessors of arrays the most efficient in terms of both code size and speed? I thought that javascript optimized this.position = [...position] for number[].

5

Size as in bundle size. I was not able to get the bundle smaller with accessors :frowning:

6

and Copilot found two problems with your Pr that are worth checking

7

Note: I updated the comment threads with my current thoughts.

I’m pretty rusty on PR workflows… since, er, before automated pipelines. :sweat_smile: Pardon my newbieness, as I ask some questions: :folded_hands:

  1. After addressing prior feedback, is there anything additional to do to the PR to indicate that I’m ready for another look? No rush, I just don’t want to leave thing languishing because I missed a step.

  2. Once all PR threads are resolved, there may be conflicts from new CLs. Is the workflow: rebase, resolve conflicts, push --force-with-lease? Once automation runs again, will it auto-merge if there are no new issues?

3. Some of the render tests seem to consistently fail on my machine. Is that unusual? If the failures seem unrelated to my change, I’m pushing and assuming that the PR automation will run the tests in the correct environment. Should I instead be digging into these failures?

8
  1. nope it is on me now to relaunch the automatic CI :slight_smile:
  2. Correct
  3. Nope it could happen. the guardail is the stable CI.

I’ll let you know by pinging you on the issue if there is more for you else I’ll drive it to the finish line :wink: