Show transitions above everything else

[MAJigsaw77]

I made this pr mostly because when the FlxG.camera has a zoom like 0.9 the transition looks weird, this is fixing it.

[MAJigsaw77]

Idk why nightly fails, the another ones seems to not have an issue?

[Geokureli]

nightly is failing on all branches - https://github.com/HaxeFoundation/haxe/issues/11353

[MAJigsaw77]

Oh, I see

[Geokureli]

I assume this is so that the transition is always on top of all other cameras? Please add a basic description to all PRs, and always provide simple test cases that highlight the change's need. The fact that the first commit didn't compile makes me think this is untested, and perhaps a theoretical feature rather than a practical one.

Shouldn't the camera be removed and destroyed when the transition is done, particularly for intro transitions

[MAJigsaw77]

I assume this is so that the transition is always on top of all other cameras? Please add a basic description to all PRs, and always provide simple test cases that highlight the change's need. The fact that the first commit didn't compile makes me think this is untested, and perhaps a theoretical feature rather than a practical one.

Shouldn't the camera be removed and destroyed when the transition is done, particularly for intro transitions

I made this pr mostly because when the FlxG.camera has a zoom like 0.9 the transition looks weird, this is fixing it.

[Geokureli]

then can you share an example showing that the old system looks weird and the new one does not?

[MAJigsaw77]

then can you share an example showing that the old system looks weird and the new one does not?

Sure

[MAJigsaw77]

then can you share an example showing that the old system looks weird and the new one does not?

Sure

Before: https://streamable.com/bof6up

After: https://streamable.com/xvn1hk

[Geokureli]

Don't know if you saw this

Shouldn't the camera be removed and destroyed when the transition is done, particularly for intro transitions

Also in the future please provide simple code for test cases, so I can check it out locally and look for potential issues or unintended side effects. if you just provide videos, I'll end up making my own test cases when I have time, but it'll take longer

[MAJigsaw77]

Don't know if you saw this

Shouldn't the camera be removed and destroyed when the transition is done, particularly for intro transitions

Also in the future please provide simple code for test cases, so I can check it out locally and look for potential issues or unintended side effects. if you just provide videos, I'll end up making my own test cases when I have time, but it'll take longer

FlxTransitionableState.defaultTransIn = new TransitionData(FADE, FlxColor.BLACK, 1, FlxPoint.weak(0, -1));
FlxTransitionableState.defaultTransOut = new TransitionData(FADE, FlxColor.BLACK, 0.7, FlxPoint.weak(0, 1));

[MAJigsaw77]

About the camera, I don't think it's necessary to destroy it.

[MAJigsaw77]

But I'll do something about it

[Geokureli]

About the camera, I don't think it's necessary to destroy it.

Tools should clean up after themselves and it this case it's trivially easy to do so, otherwise every state with an intro transition will have an useless camera drawing blank alpha over the entire game every frame.

Also I worked on a project that used these transition effects for cinematic events, without switching to or from another state, so this code could have added dozens of cameras to the state

[Geokureli]

mis-clicked and accidentally closed this before, sorry

Edit: whoops

[Geokureli]

this is the test state I made. It works as desired on outros but not intros (after fixing the above issue)

package states;

import flixel.FlxCamera;
import flixel.FlxG;
import flixel.FlxSprite;
import flixel.math.FlxPoint;
import flixel.math.FlxRect;
import flixel.addons.transition.TransitionData;
import flixel.addons.transition.Transition;
import flixel.addons.transition.FlxTransitionableState;
import flixel.util.FlxColor;

/**
 * https://github.com/HaxeFlixel/flixel-addons/pull/410
 */
class TransitionCameraTestState extends FlxTransitionableState
{
    public function new ()
    {
        FlxTransitionableState.defaultTransIn = new TransitionData(FADE, FlxColor.BLACK, 1, FlxPoint.weak(0, -1));
        FlxTransitionableState.defaultTransOut = new TransitionData(FADE, FlxColor.BLACK, 1, FlxPoint.weak(0, 1));

        super();
    }

    override function create()
    {
        super.create();

        FlxG.camera.bgColor = 0xFFffffff;

        var bg = new FlxSprite(50, 50);
        bg.makeGraphic(FlxG.width - 100, FlxG.height - 100, 0xFF000080);
        add(bg);

        var fg = new FlxSprite(10, 10);
        fg.makeGraphic(FlxG.width - 20, 100, 0xFFa00000);
        fg.camera = new FlxCamera();
        fg.camera.bgColor = 0x0;
        FlxG.cameras.add(fg.camera, false);
        add(fg);
    }

    override function update(elapsed)
    {
        super.update(elapsed);

        if (FlxG.keys.justPressed.SPACE)
            FlxG.switchState(new TransitionCameraTestState());
    }
}

[MAJigsaw77]

this is the test state I made. It works as desired on outros but not intros

Weird, on the videos I send it worked.

[Geokureli]

this is the test state I made. It works as desired on outros but not intros

Weird, on the videos I send it worked.

This is why it's important to make easily sharable small tests rather than a video of a massive game

[MAJigsaw77]

this is the test state I made. It works as desired on outros but not intros

Weird, on the videos I send it worked.

Maybe put super.create(); after add(fg);.

[MAJigsaw77]

this is the test state I made. It works as desired on outros but not intros (after fixing the above issue)

Hmm

Maybe the issue has something to do with persistentDraw.

[Geokureli]

It does have to do with the order of super.create() because the FG camera is created after the intro starts, but I'm wondering if there is a way to account for this, since it's a common way to structure code?

[Geokureli]

I moved the adding of the camera to the transition's start so it happens after create() to prevent ordering issues

[Geokureli]

if (FlxG.cameras.list.contains(_effectCamera))
    FlxG.cameras.remove(_effectCamera, false);

_effectCamera = FlxDestroyUtil.destroy(_effectCamera);

this should go back to what it was before, if the camera was removed from the state switch calling FlxG.cameras.reset(), then it has also been destroyed

[MAJigsaw77]

So?

[MAJigsaw77]

I need to remove that code and replace the FlxG.camera.add to that?

[Geokureli]

sorry, working on something else right now, should get back to this soon, I want to double check a few things before merging

[Geokureli]

Made some changes,

• Added a cameraMode option to TransitionData allowing the following values:
  • TOP: (default value) the current "top" camera is used
  • NEW: a new camera is created and destroyed by the transition, useful if the top camera has some unwanted effect on it
  • DEFAULT: transitions use the default cameras
• moved the camera handling logic from Transition to TransitionEffect so the mode is honored even when using transitions effects without transition substates
• changed the hxFormat of flixel-addons to match flixels
• using lower case args for methods
• some better enum handling

[MAJigsaw77]

Made some changes,

• Added a cameraMode option to TransitionData allowing the following values:

  • TOP: (default value) the current "top" camera is used
  • NEW: a new camera is created and destroyed by the transition, useful if the top camera has some unwanted effect on it
  • DEFAULT: transitions use the default cameras
• moved the camera handling logic from Transition to TransitionEffect so the mode is honored even when using transitions effects without transition substates
• changed the hxFormat of flixel-addons to match flixels
• using lower case args for methods
• some better enum handling

What's left to do in order to get this pushed?

[Geokureli]

gonna go run an errand, then merge when i come back assuming CI passes (cept nightly)

[Geokureli]

Note: I put @since 5.6.0 but that's flixel's next minor version, it should be flixel-addon's nextversion which is going to be 3.3.0

this is why I like to wait at least a day on most PRs, so i can look over it for any mistakes with fresh eyes