Lusito / box2d.ts

Full blown Box2D Ecosystem for the web, written in TypeScript
https://lusito.github.io/box2d.ts
60 stars 6 forks source link

Evaluate and implement a consistent and safe style for temporary variables. #15

Open Lusito opened 3 years ago

Lusito commented 3 years ago

JavaScript has a garbage collector.. we gotta live with it.

In the original code, temporary vectors and similar where created on the stack without problems. If we use new objects everytime, we'll just push work for the garbage collector.

Flyover has 2 solutions for this:

The former is fine.

The latter, however, is a but ugly to work with and compare code. This also allows to access these static variables from other classes, which flyover does a lot with the b2Vec2 static attributes. This requires you to have a good eye on these to avoid accidental double use.

In order to make the code easier to compare to upstream, I used top-level constants instead in some places, but that approach isn't perfect either:

const temp = {
    J1: new b2Vec2(),
    J2: new b2Vec2(),
    J3: new b2Vec2(),
    r: new b2Vec2(),
    e1: new b2Vec2(),
    e2: new b2Vec2(),
    Jd1: new b2Vec2(),
    Jd2: new b2Vec2(),
    d: new b2Vec2(),
    u: new b2Vec2(),
    dp1: new b2Vec2(),
    dp2: new b2Vec2(),
    dp3: new b2Vec2(),
    d1: new b2Vec2(),
    d2: new b2Vec2(),
    dHat: new b2Vec2(),
};

When I'm talking about double use, I mean something like:

function doStuff(m: b2Vec2, n: b2Vec2, out: b2Vec2) {
    const t = b2Vec2.s_t0.Set(0, 1);
    // ...
    out.x = m.x + n.x + t.x;
    out.y = m.y + n.y + t.y;
    return out;
}

function accidentalDoubleUse() {
    const a = new b2Vec2(1, 2);
    const b = new b2Vec2(2, 3);
    const c = doStuff(a, b2Vec2.Add(a, b, b2Vec2.s_t0), b2Vec2.s_t1);
    return c.y;
}

Both functions use b2Vec2.s_t0, causing loss of data.

The goal of this task would be to evaluate the best approach to encapsulate the temp variables, so they can't easily get used in two places at once.

Then when implementing these changes, ensure, that no public temporary variables get shared over multiple modules.