HaxeFlixel / flixel

Free, cross-platform 2D game engine powered by Haxe and OpenFL
https://haxeflixel.com/
MIT License
1.98k stars 439 forks source link

drawDebug hitbox dimensions are off #1821

Closed IBwWG closed 8 years ago

IBwWG commented 8 years ago

Per https://github.com/HaxeFlixel/flixel/commit/647795c29b3d14cc34296686d0248787fa6b05ca#commitcomment-17198787

Actually, that particular line is OK. "x + width" if you sub in the simplest example of 0 and 1, makes the equation true. Likewise for other x+width and y+height spots.

Where it might not work is x < FlxG.worldBounds.right, but I have to look into how right is calculated.

BTW now that I think of it, when I first was looking into this issue, I noticed that the bounding boxes were drawn off by 1px by the debugger:

hitbox-vis hitbox-invis

Since my fix worked I didn't really revisit that. (Sorry for the white border, you'll have to view those in an editor to see, but you probably would anyway to zoom in.) I wonder if that also needs fixing somewhere.

IBwWG commented 8 years ago

The .right should also work, in terms of the current formula. If the bounds are 0,0 and 1-wide, then right is 0+1 = 1. However, this might be a flaw in FlxRect, because left represents an actual pixel that is part of the rectangle, whereas right is actually one pixel beyond the right edge of the rectangle. I'm guessing that might contribute to the debugger hitboxes being off--you can see (even with the naked eye) the top of the two graphics above, that the bottom and right edges are beyond the 1-px white border that the sprite has as part of it.

IBwWG commented 8 years ago

Can I fix FlxRect, or is this a breaking change? :)

IBwWG commented 8 years ago

Anyway I checked the rest of FlxObject.hx and AFAICT there are no other changes needed. (Except, of course, if we change FlxRect.hx, in which case we'll want <= ....right and so on.)

IBwWG commented 8 years ago

It seems not just .right and .bottom getters and setters would need to change in FlxRect. intersection() is also flawed. Its return line should read return FlxRect.get(x0, y0, x1 - x0 + 1, y1 - y0 + 1); because if you have two 1px-square rects in the same spot, the current code will return a 0 height, 0 width rectangle, instead of 1x1. I.e. if we change right and bottom then the intersection of two identical 1x1 rectangles in the same spot right now would not equal either of them...good unit test. :)

Its special cases for x1<=x0 and for y would also need to change, I think.

Gama11 commented 8 years ago

.top / .left / .right / .bottom and intersection() are all copied from OpenFL's Rectangle implementation afaik, so that's probably the intended behavior / consistent with Flash.

IBwWG commented 8 years ago

I'd say OpenFL and Flash are flawed too, then.

But I was happy to let sleeping dogs lie after my screen-edge buttons worked. :)

I guess if we can't deviate from them, we can close this, since FlxObject is otherwise OK.

Gama11 commented 8 years ago

I don't think the flash implementation is wrong here. :)

There's still the drawDebug issue.

IBwWG commented 8 years ago

They're not infallible, you know. :) IMO a 1px square graphic has width/height 1, and all of its edges should have the same coordinates, because they are all the same 1px...in their implementation, the top/left edges are but the bottom/right edges aren't. I suppose there are advantages to their approach, but I still find it unintuitive, overall. Only their calculation seems cleaner in one context, because you don't have a +1 or -1 anywhere, but that doesn't actually make it correct.

Good point about the drawDebug, let's check it out.