Replace val() with getters/setters

[clabe45]

util.val is used to sample a property, using keyframes, functions, or just returning a single value.

Since the syntax is gross, let's replace it with setters and getters, which use a similar util.val function (to prevent repeated code). Besides that, and the fact that each value should be cached per frame (I will make this a new issue), the details are left open for now.

As a side benefit, properties like layer.width and layer.height can default to the movie's width/height right in the getter, or perform similar behavior (instead of doing it manually with every query). More importantly, caching eliminates the bug of multiple different values from function properties per frame.

---

• Refactor movie.js
• Refactor layer.js
• Refactor effect.js

[alejolale]

Hi !
I'm interested in this work ! could you explain more clear this work ?

Thanks for your answer

[clabe45]

Absolutely! Because properties can be keyframes, functions, or just simple values; we need a way to get the current value of a property for a frame. Right now, the util function val(property, element, time) does this, but I think there is a way to make the syntax nicer (and more friendly to usage from the user).

The task is to write a getter and setter for each public property (that's intended to be used externally). I'm thinking something like:

// for layers
set propName(value) {
    this._propName = value;
}

get propName() {
    return val(this._propName, this, this._movie.currentTime - this.startTime);
}

Doing this will solve another problem: There are some properties that require custom logic when accessing their values, like layer dimensions default to the movie's dimensions when they are undefined. Right now, this is handled by resizing the canvas to the current frame's values each frame. Then, the canvas's dimensions are read for the rest of the frame. It would be a lot better to put this logic in a getter, like:

// in layer.Base
get width() {
    return val(this._width, this, this._movie.currentTime - this.startTime);
}

I just created #20 which ideally should be done first, so you can use this.currentTime for movies and layers and this._target.currentTime for effects when you pass the time to val.

[clabe45]

I updated the OP with a checklist. Just pick one and push it to a new feature branch for this issue, unless you're feeling ambitious. Or, you can start with one of the smaller good first issues, if you'd like.

[thefishgineer]

Any progress on this? I can help with this.

[clabe45]

@alejolale Are you currently working on this issue?

[clabe45]

@rc41186 I forget about this, yes you can work on it if you're still interested

[clabe45]

I decided not to do this, because it would be very error-prone to have to implement setters and getters (that call val) for every public property in every class. Instead, I implemented property filters (0fca188) that process the results of val. You only need to implement property filters when custom logic needs to be applied to the instant value.