clipRect in overlap functions

[MSGhero]

clipRect isn't really used in any of the FlxObject overlap functions (overridden in FlxSprite), but should it be?

Repro:

var sprite = new FlxSprite();
sprite.makeGraphic(800, 600, 0xff00ff00);
sprite.clipRect = new FlxRect(0, 0, 1, 1);
FlxMouseEventManager.add(sprite, openfl.Log.trace, null, null, null, true, true, false); // pixelPerfect = false
add(sprite);

Click anywhere except pixel 0,0, and the mouse event will fire, even though visually you're not clicking on anything. Easily solved by setting pp to true, but it seems like overkill. Plus, you might still want transparent areas within the clip to be mouse-able.

Issue because it affects overlaps, separate, overlapsAt, and overlapsPoint, plus potentially isOnScreen, getGraphicMidpoint, etc.

[JoeCreates]

I'd have expected clipRect to only apply to the graphic. You might want to clip a sprite but still have a hitbox larger than the sprite, e.g. for a touch button.

ClipRect is for the graphic and hitbox is for collisions. Am I missing something?

On 9 Dec 2017 03:28, "Nick" notifications@github.com wrote:

clipRect isn't really used in any of the FlxObject overlap functions (overridden in FlxSprite), but should it be?

Repro:

var sprite = new FlxSprite(); sprite.makeGraphic(800, 600, 0xff00ff00); sprite.clipRect = new FlxRect(0, 0, 1, 1);FlxMouseEventManager.add(sprite, openfl.Log.trace, null, null, null, true, true, false); // pixelPerfect = false add(sprite);

Click anywhere except pixel 0,0, and the mouse event will fire, even though visually you're not clicking on anything. Easily solved by setting pp to true, but it seems like overkill. Plus, you might still want transparent areas within the clip to be mouse-able.

Issue because it affects overlaps, separate, overlapsAt, and overlapsPoint, plus potentially isOnScreen, getGraphicMidpoint, etc.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/HaxeFlixel/flixel/issues/2121, or mute the thread https://github.com/notifications/unsubscribe-auth/ACbEvOFG3Jy3Jg-b_R6Z1VzEhb7_Q3Bpks5s-f5rgaJpZM4Q74m9 .

[Gama11]

@JoeCreates That was my thinking as well.

[MSGhero]

Ohh k. How about a short addition to clipRect’s docs: “Clipping rectangle for this sprite’s graphic” and maybe “(not its hitbox!)” because I didn’t know that.

It’s sorta in the last line I guess, but I was expecting something like scrollRect and thought no one used the feature enough to implement hitbox clipping.

[MSGhero]

Does this belong anywhere in flixel, using a Boolean or whatever? https://github.com/haxeui/haxeui-flixel/blob/master/haxe/ui/backend/ComponentBase.hx#L57-L115

This is the required functionality for something like a scrolling pane, without using pixel perfect.

[JoeCreates]

That code looks really horrible. I wouldn't recommend doing anything like that. Is there any reason you can't just set the width and height to the clip rect's if that's what you require?

On 17 Dec 2017 15:37, "Nick" notifications@github.com wrote:

Does this belong anywhere in flixel, using a Boolean or whatever? https://github.com/haxeui/haxeui-flixel/blob/master/ haxe/ui/backend/ComponentBase.hx#L57-L115

This is the required functionality for something like a scrolling pane, without using pixel perfect.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/HaxeFlixel/flixel/issues/2121#issuecomment-352264185, or mute the thread https://github.com/notifications/unsubscribe-auth/ACbEvC0IyI-1VHOWkTqA8Yx58s9f726Wks5tBTU4gaJpZM4Q74m9 .

[MSGhero]

Because HaxeUI also uses width and height, so setting the width would change the total size of the object instead of just the hitbox size. I don't recommend this code either, except it works.

To be fair, it's only adding like 3 lines of additional code to the function implementation in FSG and FlxSprite.

[JoeCreates]

That seems to be bad design in flxui. I don't think we should let it leak into flixel. The solution is to set the hitbox to the graphic size if that is the required behavior, not to combine the two and redefine how things interact with them.

In flixel (not flxui), can you get the required behavior by setting the hitbox to the graphic size?

As far as I can tell it's a simple solution that requires minimal code and no hacky interface redefinition or code duplication like the flxui way. Furthermore, the flxui behavior is undesirable for mobile touch controls, where it is often beneficial to have hitboxes larger than the graphics.

On 18 Dec 2017 00:18, "Nick" notifications@github.com wrote:

Because HaxeUI also uses width and height, so setting the width would change the total size of the object instead of just the hitbox size. I don't recommend this code either, except it works.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/HaxeFlixel/flixel/issues/2121#issuecomment-352298057, or mute the thread https://github.com/notifications/unsubscribe-auth/ACbEvHoCcdJw1PWTnhELZFEUy5tLmF11ks5tBa9LgaJpZM4Q74m9 .

[MSGhero]

HaxeUI, not flixel-ui. HaxeUI is trying to be compatible with a dozen different libraries, and I’m telling you that you can’t set the hitbox since it’s not separate from the graphics. Plus you can easily add padding or a margin to get touch-friendly buttons.

It’s fine if it doesn’t belong in flixel, I was just asking since it’s a requirement for me and might be helpful to others...

[Gama11]

you that you can’t set the hitbox since it’s not separate from the graphics.

What do you mean by that.. in the HaxeUI API?

[JoeCreates]

Sorry, I misread the lib, but I think my point still stands.

Do I understand this correctly? HaxeUI has concepts of width, height and clipRect, and so does Flixel, but they are not exactly the same, because in HaxeUI the width and height are tied to the graphic and the clipRect affects the "hitbox"?

If so, then my recommendation remains the same. Flixel's and HaxeUI's properies should not be forcedly mashed together to make flixel's behave the same as HaxeUI or vice versa. In a flixel-haxeUI integration you should be able to access both flixel's interface using flixel's definitions and HaxeUI's with its own.

If you want to manipulate the clipRect and width/height of a HaxeUI component in a HaxeUI way, you would not do this through Flixel's clipRect, but rather via an adapter such as myButton.ui.clipRect. The ui component in this example would provide the interface as defined by HaxeUI, and manipulating these properties would in turn modify flixel's properties in a meaningful way. For example, setting the haxeUI clipRect would set both the flixel clipRect and hitbox to have the equivalent behaviour to HaxeUI's definition.

You would not redefine the flixel interface nor reimplement stuff like overlapsPoint. You would have two concepts of width and height per sprite, but you would typically modify the haxeUI properties which would in turn modify the flixel properties.

Repurposing flixel's interface appears to be simpler, but it is confusing to have the same interface have different meanings, things have to be reimplemented with the possibility of duplicating code (such as in overlapsPoint), and it is less flexible as it removes the option of using the original flixel properties.

To give an example of how this can create confusion and further problems, consider what happens when you turn on the debug view in flixel to see the hitboxes. With a clipped HaxeUI component, would you like to see a box around the clickable area, or around the full width and height of the original graphic even though the clipped out part can't be clicked? To get a consistent behaviour in the debug view, it turns out it's not only overlapsPoint that needs reimplementing, but also the debug renderer. Writing an adapter to map the HaxeUI interface to flixel's avoids missing cases like this.

[MSGhero]

My understanding of flixel is that width and height from FlxObject are the hitbox, with x and y. HaxeUI-core defines or overrides the width and height of every backend to be the total size of its components. So by setting width to the clipped width, you’re resizing the component and triggering a layout update.

My janky code ignores width and height and uses the clip, if one exists, for mouse events. If there’s no clip, then the hitbox is just the size of the component.

[MSGhero]

@JoeCreates the point of HaxeUI is to not make UI a property of sprite group or vice versa. “You should just be able to addChild or add or whatever a UI component directly. That part is essential for making HaxeUI easy to use, since it integrates into your normal workflow.”

It would be really easy if I didn’t have to extend FSG, but that’s really more of a restriction.

[JoeCreates]

HaxeUI overriding its backends is a mistake in my opinion, if that means repurposing their interfaces based on name commonalities with HaxeUI, rather than their definitions.

It is not easier to use HaxeUI if you have properties like width, height and clipRect which don't behave the same as they do elsewhere in flixel. As I said, the interface /looks/ simpler, but it will result in unexpected behaviour as it is difficult to account for all the ways that the existing interface had been used, such as my debug renderer example. The fact that their definitions have changed is a problem for anything that uses these properties and expects them to represent a particular thing.

The adapter component method doesn't particularly make things harder. You just remember to go via the adapter if you wish to do things in a HaxeUI way. It avoids the confusion, avoids the reimplementation (thus is more maintainable), and avoids the possibility of missed use cases that also need reimplementing such as the debug renderer example.

The point of using HaxeUI is surely to gain the advantages of using its components and layout management, and different HaxeUI backends will require different solutions based on how much they have already implemented. Flixel has definitions of width, height and clipRect which do not directly map to HaxeUIs definitions. The fact that they are /similarly/ defined and have the same names could mislead into trying to map them directly to each other, but I'm arguing that this would be a mistake. They have the same names but they are not the same interface. They need an adapter.

[MSGhero]

Ok well this isn’t going to change anything, and I got my answer a while ago. I agree that a UI property is preferable from a backend’s perspective, but I’m not going to fork the core library and make one change that breaks every backend just so flixel’s doesn’t need to add 3 lines.

The bounding box still shows up like normal in the debug window as I’ve been using that to debug, due to the way things are structured. And with the work on a mobile-specific backend, touch friendly buttons will be a thing without needing flixel input.