flame-engine / flame

A Flutter based game engine.
https://flame-engine.org
MIT License
9.19k stars 899 forks source link

fix: ViewportAwareBounds component and lifecycle issues #3276

Closed TheMaverickProgrammer closed 2 weeks ago

TheMaverickProgrammer commented 1 month ago

Description

The behavior for CameraComponent.setBounds was not behaving correctly. The viewport-aware behavior was not triggering until a window resize event. All supported bound variants were not respecting the fixed resolution viewport, or in some cases, behaving unpredictably.

The issue was with the fact the viewport-aware behavior component depends on the bounded position component to be in its parent, but was returning null during onMount. The viewport math was using the logical size instead of the virtual size. I made some math changes to be accurate.

This PR adds new a getter CameraComponent.considerViewport. ViewportAwareBoundsBehavior is now added as a side effect of mounting BoundedPositionBehavior by waiting for the mounted future. This guarantees that Flames life cycle is respected and the components behave as expected.

Tests pass on my own local project.

Videos are linked on the discord thread here.

Melos dry run completed successfully.

Checklist

Breaking Change?

Migration instructions

No work to be done.

Related Issues

https://github.com/flame-engine/flame/pull/2769

QUESTIONS FOR THE DEVS

So I'm not satisfied with the fixes for Circle bounds. The bounds uses the maxima of the viewport, and yes, creates a circle that stays within the size of the bounds circle, but I'd expect that Circle bounds should keep my viewport inside the circle as it is documented to stay inside the desired bounds shape. Therefore, I think Circle case should have 4 incident points to fit the viewport. That is to say, the viewport is not the maxima of the newly calculated Circle, but will be fully contained by this new Circle. This way, you'll never see outside of the bounds as it, seems to me anyway, implies. However I do not know if this is intuitive for others and expected behavior. Thoughts?

spydon commented 2 weeks ago

@TheMaverickProgrammer any update on this? :)

TheMaverickProgrammer commented 2 weeks ago

Need to test something I forgot before pushing. I will re-open after verifying the mount operation happens correctly.