-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add interpolation to bitECS network transform. #6024
Conversation
LinearScale.duration[eid] = millisecondsBetweenTicks; | ||
LinearScale.targetX[eid] = tmpVec.x; | ||
LinearScale.targetY[eid] = tmpVec.y; | ||
LinearScale.targetZ[eid] = tmpVec.z; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably remove the TODO at the end of this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, updated. Thanks
src/systems/networked-transform.js
Outdated
LinearTranslate.destinationX[eid] = tmpVec.x; | ||
LinearTranslate.destinationY[eid] = tmpVec.y; | ||
LinearTranslate.destinationZ[eid] = tmpVec.z; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mayeb it would be more consistent to name this targetX/Y/Z?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong preference about it. Updated, thanks.
|
||
if (deltaTime >= duration) { | ||
obj.position.set(destX, destY, destZ); | ||
removeComponent(world, LinearTranslate, eid); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels a bit strange that we are removing the component inside it's own query. Wouldn't it make more sense to remove it in networked-transform.ts
?
tmpVec.fromArray(NetworkedTransform.position[eid]);
if (!tmpVec.near(obj.position)) {
addComponent(world, LinearTranslate, eid);
LinearTranslate.duration[eid] = millisecondsBetweenTicks;
LinearTranslate.destinationX[eid] = tmpVec.x;
LinearTranslate.destinationY[eid] = tmpVec.y;
LinearTranslate.destinationZ[eid] = tmpVec.z;
} else {
removeComponent(world, LinearTranslate, eid);
}
and let this system do just what is responsible for, interpolating:
linearTranslateQuery(world).forEach(eid => {
const duration = LinearTranslate.duration[eid];
const destX = LinearTranslate.destinationX[eid];
const destY = LinearTranslate.destinationY[eid];
const destZ = LinearTranslate.destinationZ[eid];
const deltaTime = world.time.delta;
const obj = world.eid2obj.get(eid)!;
obj.position.lerp(_vec3.set(destX, destY, destZ), deltaTime / duration);
LinearTranslate.duration[eid] = duration - deltaTime;
obj.matrixNeedsUpdate = true;
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it is so strange to remove a component inside a query especially if it does operations across ticks and finishes at specified time or event. MediaLoader does it, too.
And LinearTranslate/Rotate/Scale and their system are designed as common components and systems that linearly (against time) update object's transform to target values using the specified duration time. I think removing component in that system is natural and makes them decoupled from other components and systems.
But if removing a component in its query can cause a bug in certain scenarios, we may need to revisit and write it in the internal design document. Would you please share if you have?
c3280f8
to
af605eb
Compare
This commit adds easy linear interpolation to bitECS network transform. Changes * Add LinearTranslate/Rotate/Scale components * Add linearTransform system to handle the added components * network-transform sets the component when receiving data from remote * Move millisecondsBetweenTicks from network-send-system to networking because it is referred from two places
af605eb
to
3a6e295
Compare
This PR adds easy linear interpolation to bitECS network transform.
Changes