-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
eliminate getters/setters, use 100% reactivity with signals and effects #279
Labels
Comments
Classes that need attention (basically any element that accepts a selector (like an "idref") to reference another element:
|
trusktr
added a commit
that referenced
this issue
Oct 14, 2023
…re properly if the elements in play were defined before their scene. TODO: Avoid ever using getters/setters, switch to fully-reactive updates with signals+effects. Signals and effects solve so many problems, especially code execution order issues. #279 Also remove no-longer-needed module declarations for modules that now have type definitions.
trusktr
added a commit
that referenced
this issue
Oct 14, 2023
…ts logic runs before .scene is ready This circles back to #279 (eliminate getters/setters, use 100% reactivity with signals and effects): we need to move logic out of getters/setters to make it fully reactive and robust.
trusktr
added a commit
that referenced
this issue
Dec 24, 2023
…e-scene background-blur>` attribute) allows configuring the amount of blur for the scene background texture, accepting values from 0 to 1. Because this property is experimental, the way it works is subject to breaking changes without semver-compatible version bumps before we unmark it as `experimental`. - BREAKING: remove reactivity from `Transformable`'s `position`, `rotation`, `scale`, `origin`, `alignPoint`, and `mountPoint` properties, and from `Sizeable`'s `sizeMode` and `size` properties, which all reference an `XYZValues` object. Effects will no longer update when the `XYZValues` objects referenced by those properties have their sub-properties modified because. The referenced objects of the properties do not change anyway, and making the properties be reactive for the setter shorthand usage caused infinite loop problems in cases like these, ```js // This caused an infinite reactivity loop due to `element.position` being // tracked as a dependency and `element.position.x = ...` causing triggering // effects for `element.position`. createEffect(() => { element.position.y = otherElement.rotation.z }) ``` and required a workaround like this: ```js createEffect(() => { // First get the object without tracking, then set its property: untrack(() => element.position).y = otherElement.rotation.z }) ``` Now the former example will work without issues. If you were previously observing one of those properties in an effect like so, ```js createEffect(() => { doSomethingWith(element.position) }) ``` where `doSomethingWith` may be untracked or reads the values of the passed-in object _later_ outside of the reactive context, you can either read the needed sub-properties directly in your effects, ```js createEffect(() => { // Any time x, y, or z change, element.position.x element.position.y element.position.z // do this. doSomethingWith(element.position) }) ``` or use `XYZValues.asDependency()` like so, ```js createEffect(() => { // Any time x, y, or z change, element.position.asDependency() // do this. doSomethingWith(element.position) }) ``` or like so (it returns the same object), ```js createEffect(() => { // Any time x, y, or z change, do this. doSomethingWith(element.position.asDependency()) }) ``` - BREAKING: refactor: move base camera features to a new `Camera` base class from which `PerspectiveCamera` extends from, and from which we will later extend a new `OrthographicCamera` class from. The `Scene` `.camera` and `.threeCamera` properties are now type Lume `Camera` and Three.js `Camera`, respectively, and if you depended on properties/methods from Lume or Three.js `PerspectiveCamera` you now need to conditionally narrow, or downcast, the types. For example, this code, ```js console.log(scene.camera.fov) console.log(scene.threeCamera.fov) ``` can become something like ```js import {PerspectiveCamera} from 'lume' import {PerspectiveCamera as ThreePerspectiveCamera} from 'three' if (scene.camera instanceof PerspectiveCamera) { console.log(scene.camera.fov) console.log((scene.threeCamera as ThreePerspectiveCamera).fov) } ``` or ```js import type {PerspectiveCamera} from 'lume' import type {PerspectiveCamera as ThreePerspectiveCamera} from 'three' console.log((scene.camera as PerspectiveCamera).fov) console.log((scene.threeCamera as ThreePerspectiveCamera).fov) ``` - BREAKING: `<lume-mesh>` elements no longer have default geometry and material behaviors. If you previously relied on a `<lume-mesh>` rendering using a default box geometry and phong materia, you need to update code to apply those behaviors manually: `<lume-mesh has="box-geometry phong-material">`. Other elements like `<lume-box>` continue to extend from `<lume-mesh>` and continue to provide default behaviors. - BREAKING: Elements that extend from `Mesh` and previously has a default `phong-material` now have a `physical-material` as the default. Physical (PBR) materials are the current standard in graphics, so now all elements will rendering using those features by default. This may change the visuals of some scenes. To keep the same rendering, `phong-material` can be used manually. If you had code like this, ```html <lume-box color="red" size="10 10 10"></lume-box> ``` you can override the new default with a phong material: ```html <lume-box has="phong-material" color="red" size="10 10 10"></lume-box> ``` - BREAKING: rename `DefaultBehaviors` to `InitialBehaviors`, rename the `defaultBehaviors` static property to `initialBehaviors` as an instance property, and simplify how it is used. If you previously extended from a Lume class or use the `DefaultBehaviors` mixin, and defined `static defaultBehaviors` in your subclass like so, ```ts class CoolMesh extends Mesh { static defaultBehaviors = { 'cool-geometry': (initialBehaviors: any) => { return !initialBehaviors.some((b: any) => b.endsWith('-geometry')) }, 'awesome-material': (initialBehaviors: any) => { return !initialBehaviors.some((b: any) => b.endsWith('-material')) }, } } ``` it should be converted to the following format: ```js class CoolMesh extends Mesh { initialBehaviors = {geometry: 'cool', material: 'awesome'} } ``` - fix: ensure that camera is not active when not composed - fix: ensure that when webgl is not enabled the `<lume-camera-rig>` still functions - fix: improve texture handling so a texture is not left behind if the source URL is removed - fix: move `ProjectedMaterialBehavior.projectedTextures` handling from a getter into an effect so that it is reactive to the owner element's `.scene` otherwise the associated set of texture projectors will not update when an element gets composed (f.e. into a `<slot>`). This is part of the work for #279 - deprecate `ProjectedMaterialBehavior.projectedTextures` in favor of a new property names `ProjectedMaterialBehavior.textureProjectors` - docs: update examples in JSDoc comments to work with latest `<live-code>` - refactor: simplify `Scene` background handling by using `onCleanup` instead of calling cleanup ourselves on each change. - refactor: move Element3D-specific styling out of SharedAPI so it does not unnecessarily apply to Scene - refactor: move internals that Motor depends on out of SharedAPI and into Motor - refactor: improve child composed handling to avoid scene effects running too many times by returning early when a child is being given a scene if it already has the scene. - refactor: use `batch` inside of `XYZValues.set()` to reduce the number of effect runs to a third for better performance - TODO fix: ensure that when webgl gets disabled the active camera view does not change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Signals and effects naturaly solve a slew of issues, including issues with code load/execution order. By switching away from getters/setters, we'll be confident code execution order issues will not bite us.
For an example, see the issue fixed in 9bb61f6
The text was updated successfully, but these errors were encountered: