Allow event-based objects to define a rotation center point #4910
Conversation
|
I wonder if I should add private expressions to access custom object internal bounds as events can no longer rely on camera bounds. |
| _localTransformation: gdjs.AffineTransformation = new gdjs.AffineTransformation(); | ||
| _localInverseTransformation: gdjs.AffineTransformation = new gdjs.AffineTransformation(); | ||
| _isLocalTransformationDirty: boolean = true; |
There was a problem hiding this comment.
It will take more memory but the code is easier to maintain and maybe faster because an affine transformation is 8 arithmetical operations versus something like 25 for the hand crafted transformation.
There was a problem hiding this comment.
I think that's fine and fair to have these. Custom objects are not designed to be super lightweight objects working as particles for example.
| this._pixiContainer.position.x = | ||
| this._object.getDrawableX() + | ||
| this._object.getX() + | ||
| this._pixiContainer.pivot.x * Math.abs(this._object._scaleX); | ||
| this._pixiContainer.position.y = | ||
| this._object.getDrawableY() + | ||
| this._object.getY() + | ||
| this._pixiContainer.pivot.y * Math.abs(this._object._scaleY); |
There was a problem hiding this comment.
This was compensating the side effects of the camera bug.
The internal camera of custom objects was trying to stay at the center of the children. This way the transformation has no effect and children were not shifted. This system has been removed to avoid any side effects as it doesn't really make sense.
| /** @type gdjs.Layer */ | ||
| const theLayer: gdjs.Layer = this._layers.items[name]; | ||
| const theLayer: gdjs.RuntimeLayer = this._layers.items[name]; |
There was a problem hiding this comment.
Is /** @type gdjs.Layer */ useful in some way or is it a remanent comment from a long forgotten JS past?
There was a problem hiding this comment.
Remanent from a long forgotten JS past 👻
| @@ -363,6 +333,9 @@ namespace gdjs { | |||
| * @returns the center X from the local origin (0;0). | |||
| */ | |||
| getUnscaledCenterX(): float { | |||
| if (this._customCenter) { | |||
There was a problem hiding this comment.
It's a feature I forgot to add. Though, we had already done it for the shape painter version of the slider.
| * | ||
| * It adapts the layers camera positions. | ||
| */ | ||
| onObjectUnscaledCenterChanged(oldOriginX: float, oldOriginY: float): void { |
There was a problem hiding this comment.
Nice! Why don't we need this anymore?
There was a problem hiding this comment.
The internal camera of custom objects was trying to stay at the center of the children (a bit like camera stays on the upper left corner of the scene). This way the transformation has no effect and children were not shifted. This system has been removed to avoid any side effects as it doesn't really make sense.
| i18n._( | ||
| 'Change the center of rotation of an object relatively to the object origin.' | ||
| ), | ||
| i18n._('Change the center of rotation of _PARAM0_: _PARAM1_; _PARAM2_'), |
There was a problem hiding this comment.
Set the center of rotation of X to Y;Z (relative to the object origin)
There was a problem hiding this comment.
I don't mind either but "Change" is used in most actions, the ones related to positions for instance. Should it be consistent?
There was a problem hiding this comment.
In this case, it should be Change the center of rotation of X (relative to the object origin): set to Y;Z
There was a problem hiding this comment.
(otherwise this is not a valid sentence, and while we don't see it because we're used to it, any new user will notice it and will see it as a bad quality software)
| @@ -589,8 +589,9 @@ void ExporterHelper::AddLibsInclude(bool pixiRenderers, | |||
| InsertUnique(includesFiles, "runtimescene.js"); | |||
| InsertUnique(includesFiles, "scenestack.js"); | |||
| InsertUnique(includesFiles, "force.js"); | |||
| InsertUnique(includesFiles, "RuntimeLayer.js"); | |||
| InsertUnique(includesFiles, "layer.js"); | |||
There was a problem hiding this comment.
To be sure I understand: ideally, from the beginning, gdjs.Layer would have been gdjs.RuntimeSceneLayer if it was perfectly named, right?
|
Storybook is failing because of a Node upgrade, feel free to ignore. |
Ok, and the failed test is about the firebase extension. |
D8H
changed the title
Changes
Demo