Hitboxes do not take `scale` into consideration

[spydon]

Current bug behaviour

If you scale the parent of a hitbox it doesn't always work, due to that we don't know the absolute transform up to that hitbox location in the tree.

Expected behaviour

That the scaling should work properly with hitboxes

Steps to reproduce

Flip a component that has a hitbox as a child

More environment information

Create a list of more environment information, like:

• Flame version: 1.1.1
• Platform affected: all
• Platform version affected: all

More information

[st-pasha]

A more reliable approach would be to re-think the design of ShapeComponent so that it wouldn't need to maintain its own transform that mirrors the transform of PositionComponent.

[spydon]

A more reliable approach would be to re-think the design of ShapeComponent so that it wouldn't need to maintain its own transform that mirrors the transform of PositionComponent.

And good idea of how to do that without having to duplicate code from Shape?

[st-pasha]

Any good idea of how to do that without having to duplicate code from Shape?

Is it possible to make Shape transform-agnostic?

[spydon]

Is it possible to make Shape transform-agnostic?

Not easily I think, one idea is to treat it as a sub-component to ShapeComponent. So instead of having the transform variables that are initially set in Shape being transferred to the ShapeComponent those variables could be relative to it and we can pass in the local point instead of the global one, the problem with that is that if we add the same Shape as a hitbox in the ShapeComponent it will not work with other hitboxes.

[st-pasha]

What if a HitboxShape was a component that could be added to a PositionComponent as a child? Then the parent's transform would apply to the child automatically.

[spydon]

What if a HitboxShape was a component that could be added to a PositionComponent as a child? Then the parent's transform would apply to the child automatically.

The collision detection system needs to be able to live outside of FCS, but it could maybe HitboxShape could be wrapped somehow, that would most likely not end up much different from how it is now though.

[st-pasha]

The collision detection system needs to be able to live outside of FCS

I'm not sure how it is implemented right now, but if there are two components that live in separate places in a component tree, and they need to be able to collide with each other, then they kinda need to be aware of the sequence of transforms up to their common root. For example:

Level
 +-- BulletsLayer
 |    +-- Bullet
 | 
 +-- EnemiesLayer
      +-- Enemy

Then, in order for a bullet to be able to interact with an enemy, the system must know how to apply the transform from the BulletsLayer and the transform from the EnemiesLayer in order to ground them into a common reference frame.

[spydon]

This is how it looks right now in the HitboxShape (tbh I had forgotten that was how I implemented it), which makes all the values inside of the collision detection system absolute:

mixin HitboxShape on Shape {
  late PositionComponent component;

  @override
  Vector2 get size => component.scaledSize;

  @override
  double get parentAngle => component.angle;

  @override
  Vector2 get position => component.absoluteCenter;

  ...

Interestingly enough it looks like I have taken scaling into consideration there... But this won't work as a hitbox on a component that has a parent that has been scaled or angled, so this definitely needs to be refactored too.

[spydon]

I think if I just add absoluteAngle and absoluteScale to PositionComponent this will be solvable, I'll give it a try.

[oloshe]

I scale my component by scale = Vector2(-1, 1); , but It's hitbox's absolute position seem not effected. The tree is like:

       Player(scaled)
       /           \
attackHitboxes     ...
    / 
Hitbox

Did I not use it as expected?

[spydon]

Are you scaling it in several of the layers? Currently the hitboxes doesn't take all combinations of scales into consideration: https://docs.flame-engine.org/1.1.1/flame/collision_detection.html?highlight=scale I think that it should work with just a simple flip though... We'll most likely be able to solve this properly once #1684 is merged.

Hmm, we'd kind of need that context in update too @st-pasha, or how do you propose that we'd use that context for something like getting the absolute transform in update?

[oloshe]

Oh i see it. Such a big deal. It seem not easy to calculate absolute transform by each CanvasTransform? In my opinion, maybe we can write a NotifyingMatrix4 just like NotifyingVector2?

[spydon]

Oh i see it. Such a big deal. It seem not easy to calculate absolute transform by each CanvasTransform? In my opinion, maybe we can write a NotifyingMatrix4 just like NotifyingVector2?

It's more than just a NotifyingMatrix4 that is needed though, it is the matrix that is an aggregate of all matrixes up to the root. In #1684 all the operations are passed down in renderTree so that it would be possible to get the total transformation. Because for scales for example, you can't just add them together if there are rotation steps involved.

[oloshe]

My humble opinion, It's expensive to calculate combined transform every times in update. Maybe we can storage its absolute transform once one of its ancestors' transform change?

[st-pasha]

Hmm, we'd kind of need that context in update too @st-pasha, or how do you propose that we'd use that context for something like getting the absolute transform in update?

I think a component could just store its total transform matrix during the rendering step, and then use it on the next update. Or, alternatively, run some of the transform-sensitive update logic directly from render(). It kinda feels icky, but it would work.

With notifying matrix4 it would probably be the same: you'd end up storing that matrix in the class so that it can be used during updates.

[spydon]

I think a component could just store its total transform matrix during the rendering step, and then use it on the next update. Or, alternatively, run some of the transform-sensitive update logic directly from render(). It kinda feels icky, but it would work.

That would be the wrong way around though, it should update first and then render, like that it would have the last tick's transform.

[st-pasha]

And what about performing update directly from render?

[spydon]

And what about performing update directly from render?

That sounds very dirty, I don't think we should break the distinction between update and render. And even if we did that update from render, it would still update too late, the hitboxes will collide after their intended tick.