Conversation
- Reactivity - Default values - Correct random an unpredictable behavior
✅ Deploy Preview for cientos-tresjs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
commit: |
alvarosabu
left a comment
alvarosabu
left a comment
There was a problem hiding this comment.
Hi @JaimeTorrealba looks better already, thanks for taking care of it, I left a couple of questions before merging.
Regarding hardcoded values
- (Math.random() - 0.5) * 0.5 - horizontal spread factor (0.5)
- (Math.random() - 0.5) * 0.1 - vertical spread factor (0.1)
- Math.sin((index + 1) / segments.value) - scale calculation method
These could be made configurable as:
- spreadX or horizontalSpread (default: 0.5)
- spreadY or verticalSpread (default: 0.1)
- scaleVariation or similar for controlling the sine-based scaling
- Maybe even distributionPattern to allow different mathematical distributions?
The last one could be discussed outside this scope, but I think adding some spread and scale variation props would make this component lot more controlable, what do you think
| groupRef.value?.children.forEach((child: Object3D, index: number) => { | ||
| child.rotation.z += smoke[index].rotation | ||
| groupRef.value?.children.forEach((child: Object3D) => { | ||
| child.rotation.z += Math.max(0.002, 0.005 * Math.random()) * speed.value |
There was a problem hiding this comment.
question: Is this a known math formula or just to introduce variation/randomness? Do you think it could be interesting for users to be able to tweak the hardcoded values?
There was a problem hiding this comment.
Just introduce randomness/variation
I don't know, we can for instance make the "speed" prop as the whole math formula. But currently, you can control the randomness with the speed prop.
What is your advice? If you ask me I would say is fine (don't put all the weight on the user to create their random, just a simple prop, in my experience most user use the random or don't, set as 0)
| :scale="[scale, scale, scale]" | ||
| :rotation="[0, 0, 0]" | ||
| /> | ||
| <TresPlaneGeometry /> |
There was a problem hiding this comment.
question: What's the reasoning behind removing with and scale?
There was a problem hiding this comment.
The big problem with the width and the scale was that, the randomness was wrong too large, this creates unstable component difficult to use and understand. I think with the previous comment (adding props to controls it) we solve it.
The new implementation, treat it different but without actually removing it
|
I like the idea @alvarosabu I would add: As props. I would suggest add the distribution pattern for another PR |
Update docs example, adding leches. Update documentation
|
Thanks for the suggestion @alvarosabu cientos.new.props.mp4 |
2 participants
To explain more: the smoke component have some time broken. Doesn't have reactivity, the behavior is erratic an unpredictable, which make it really difficult to use. This PR fix those
NOTE: a prop has being eliminated "Width" don't know if is considered a breaking change since doesn't brake anything.
smoke.component.webm