FlxMouseEventManager: check objects for camera overlap

[bendmorris]

The problem: when using FlxCameras that don't cover the whole screen, mouse events can be generated by FlxMouseEventManager when the mouse isn't within the camera's visible area.

Example: this scrolling button menu has its own FlxCamera. The top button is highlighted by an onMouseOver callback, even though the overlapping part is above the camera's visible area:

With this change, FlxMouseEventManager first checks that the screen coordinates are within the camera's bounds before checking if the object overlaps the mouse position.

[Gama11]

Would be nice to have a unit test for this.

[bendmorris]

Testing this will be hacky since it's a singleton. I would suggest adding tests once this is merged: #1961 which makes FlxMouseEventManager modular and easier to write self-contained tests for.

[Gama11]

I don't think that really impacts testing too much. Besides, I don't have any plans to merge that anytime soon, it just breaks too much (essentially all usages of FlxMouseEventManager).

[bendmorris]

It's a breaking change, so it wouldn't go in a minor release, I get that. (To be clear you just have to add globalManager to any use of FlxMouseEventManager to get exactly the same behavior, it's not a difficult migration.) There are other breaking changes such as #1940 in the pipeline. What's the plan for landing them?

If it's not clear that it will ever be merged, I'm going to continue using my modular version, so I'm probably not going to write new tests for the singleton version, and this fix will become "take it or leave it."

[Gama11]

Not going in a minor release means that it would have to wait until 5.0.0 - there are no concrete plans for that yet.

So far we've avoided breaking changes since 4.0.0, except for bugfixes where it was inevitable (what's broken there is just relying on broken behavior) and #1934. That PR only breaks usage of a field that is hardly used by anyone, and it didn't even break any of the demos - whereas this PR would break all usages of FlxMouseEventManager.

I'll have to check in more detail what exactly #1940 breaks - so far it's scheduled for a minor release (4.3.0). Either way it's a bit of a special case.

[Gama11]

It seems like you also changed this logic to be inclusive? (>= 0 instead of just > 0). This leads to unexpected behavior in FlxObject#isOnScreen():

var object = new FlxObject(0, 0, 1, 1);
add(object);
trace(object.isOnScreen()); // true
object.x = -1;
trace(object.isOnScreen()); // should be false, but true with >= 0

[bendmorris]

You're right, I'll take that out.

[Gama11]

I can still reproduce the issue you describe if I'm on your branch... Am I missing something?

package;

import flixel.input.mouse.FlxMouseEventManager;
import flixel.FlxG;
import flixel.FlxSprite;
import flixel.FlxState;
import flixel.util.FlxColor;

class PlayState extends FlxState{
    override public function create():Void {
        var sprite = new FlxSprite(0, 0);
        sprite.makeGraphic(20, 20);
        add(sprite);
        sprite.screenCenter();

        FlxG.camera.zoom = 4;
        FlxG.camera.y = FlxG.height / 2;
        FlxG.camera.scroll.y = 60;

        FlxMouseEventManager.add(sprite, null, null,
            function(_) sprite.color = FlxColor.RED,
            function(_) sprite.color = FlxColor.WHITE);
    }
}

[bendmorris]

The difference is zoom - if you remove zoom from your demo and change FlxG.camera.scroll.y to FlxG.height/2, it works. It looks like getScreenPosition doesn't incorporate camera scale. I'm not sure what the proper fix for that would be.

[Beeblerox]

so this line should be changes as well to
_point = touch.getPositionInCameraView(camera, _point);

[Beeblerox]

i've found that if we change this _point caclulation code _point = FlxG.mouse.getScreenPosition(camera, _point); to something like this:

public function getPositionInCameraView(?Camera:FlxCamera, ?point:FlxPoint):FlxPoint
    {
        if (Camera == null)
            Camera = FlxG.camera;

        if (point == null)
            point = FlxPoint.get();

        point.x = (_globalScreenX - Camera.x) / Camera.zoom;
        point.y = (_globalScreenY - Camera.y) / Camera.zoom;

        return point;
    }

then checks working properly.

This new method calculates position of a point relative to camera view rectangle

[MSGhero]

You named the function wrong! The function in FlxPointer is not the same name as in FlxMEM.

[Beeblerox]

@MSGhero already fixed this, sorry

[bendmorris]

Thanks @Beeblerox. You should have commit access to my fork, so feel free to just push your change there, otherwise I'll add them when I have a chance.