New Circle and Polygon masks + minor changes/fixes to other masks

[azrafe7]

Trying to sum up:

• ported and improved Circle and Polygon masks from HaxePunk
• console is updated after World/Entities (so now should not be a frame behind)
• added a project() function to some masks to make working with Polygon easier
• added some more params to Image.createRect/createCircle() (fill, thick, etc.)
• added Image.createPolygon() to create an Image from a Polygon mask
• changed private collide___() functions to override protected collide___() (this was a weird surprise o.O: don't know why they were simply declared as private and not overriding from the super class)
• added debugDraw() for Hitboxes in Masklist
• changes in Masklist.update() to handle Hitboxes/Polygons correctly

There shouldn't be any breaking changes, but surely needs some more testing.

Link to related topic on the forums: http://developers.useflashpunk.net/t/polygon-and-circle-masks-for-flashpunk-round-2

[zachwlewis]

I want @Draknek to get eyes on this.

[Draknek]

I also want to get eyes on this! But am currently exhausted and have lots going on. Can someone send me a reminder to look at this if I've not commented again by Thurs 17th April?

[erikyuzwa]

(Pickup Boomerang for gmail. You can toss away mail and have it "return" when you need to)

[zachwlewis]

I'll remind you, bro.

On Sun, Mar 30, 2014 at 4:37 PM, Erik Yuzwa [email protected]
wrote:

(Pickup Boomerang for gmail. You can toss away mail and have it "return" when you need to)

Reply to this email directly or view it on GitHub:
#133 (comment)

[azrafe7]

@zachwlewis @Draknek have you found the time to take a proper look at this yet?

[Draknek]

I have not! New self-imposed deadline: 1st May.

Thanks for the reminder :)

[azrafe7]

:D

[Draknek]

So I've not tested the actual code, just skimmed over it looking for potential problems. Hopefully it's been well tested! Ideally @zachwlewis would also skim over the code and set up some tests to check weird edge-cases.

This doesn't appear to fix a lot of the weirdness that makes it kinda awkward to make new masks (code/properties split weirdly between Mask/Entity? Can't even remember the details now) but for backwards compatibility that's probably good.

I'm happy for this to be merged in after the following issues are resolved:

• Setting Polygon.angle multiple times will lead to loss of precision. Should save the initial points and always rotate relative to those rather than rotating the current points repeatedly.
• If you change Polygon.originX/Y, does it get moved? What if the polygon has already been rotated?
• Polygon.createRegular() should not have default arguments.
• Image.createPolygon() should take Vector.<Point> not Polygon.
• I don't think Image.syncWithPolygon() is a thing that needs to exist.
• I'm not convinced that the Polygon constructor needs to have x/y arguments. I'd argue for removing them. If they really are important, then the Polygon.createFromFlatVector() method should also have them.
• Polygon.createFromFlatVector() should also have an angle argument.

Minor things that aren't as important:

• Mask.project() and Hitbox.project(): isn't there a better way of doing this than four if statements? Should be simple trigonometry
• Add the utility methods in Draw back in! No harm in having more utility methods.

[zachwlewis]

We should probably create PolygonError and throw those instead. That seems more elegant (and easier to catch polygon errors).

[zachwlewis]

Added a couple notes to Polygon. Also, the merge looks conflicted currently. That'll need to get fixed, too.

[Draknek]

Oh, also: this code only supports convex polygons, but that's not documented anywhere.

It should either throw an error if you provide a concave polygon, or generate the convex hull of the points passed in.

[zachwlewis]

Should it generate a convex hull, or should it divide the polygon into multiple convex polygons?

On Fri, May 2, 2014 at 4:16 AM, Alan Hazelden [email protected]
wrote:

Oh, also: this code only supports convex polygons, but that's not documented anywhere.

It should either throw an error if you provide a concave polygon, or generate the convex hull of the points passed in.

Reply to this email directly or view it on GitHub:
#133 (comment)

[azrafe7]

I agree with most of your points:

1. Polygon.angle: yes, I noticed that too, but never took the time to fix it
2. Polygon.origin: not tested yet, but it should move
3. Polygon.createRegular: agree
4. Polygon.createPolygon(Vector<Point>): agree
5. Image.syncWithPolygon(): already removed in last commit
6. x/y in constructor: other masks have them so Polygon should too; and yes, in that case they should probably also be added to create___ methods
7. Polygon.createFromFlatVector(): ok
8. project(): this needs some research
9. extra Draw methods: I removed them because they were in another PR (probably better to leave out enqueueCall though)
10. PolygonError: don't have a position on this one (I'd also be fine with removing the exception altogether)
11. only convex polygons: yes, this should be added and clearly stated in the code
12. concave polygons: not really sure we should add support for this (the user could rely on 3rd party tools/libs), but if we do then I'd go with generating the decomposing convex polygons and not just the hull

Above all you're right we need more tests/people testing the whole thing.

[Draknek]

Other masks have x/y arguments because they need them: if you need a centred PixelMask then there's no other way to do it. With Polygon, however, you're passing in coordinates so you can offset those.

I'm happy either way though.

Supporting multiple polygons means either refactoring a lot of code or having a createConvexPolygon method that actually returns a list of Polygons. Simplest solution is just to not support convex shapes.

Buuut I'm not the one who's doing the work, so do as you like :)

[azrafe7]

About modifying polygon origin: I was wrong, it doesn't move, but the rotation pivot is updated, so if you change the angle afterwards the polygon rotates around the new pivot.

[azrafe7]

About Image.createPolygon... relooking at the code I can see why I used a Polygon instead of a Vector.<Point>. That's because it also syncs with the Polygon mask (angle, etc.) and to do that needs access to some of its properties.

[azrafe7]

So... I think I've addressed almost all of the issues.

Only things left are:

• project(): but it's not that important
• PolygonError: not sure what road to follow here, need help/clarifications
• Draw methods: I'll fix the other pull request (Additional Draw methods to facilitate debugging #122)
• concave polygons: although I already have a lib that does this kind of stuff, I'm not sure we should add the code into FP. Maybe just a simple triangulator could do. But I can foresee a user requesting a way to turn their images into polygons which will lead to add a marching square implementation, and after that a way to simplify polygons, etc and well things will get out of hands pretty soon. Still open to ideas though!

[zachwlewis]

Don't worry about a special error type. I can add those when I'm bored. 

For concave polys, as a user, I want to make a shape and have it collide. I don't care what's going on. I'll be very confused if I give it a valid polygon and it doesn't work the way I expect. I'd say a simple triangulation would work just fine for concave polys.

On Sat, May 3, 2014 at 11:04 AM, Giuseppe Di Mauro
[email protected] wrote:

So... I think I've addressed almost all of the issues.
Only things left are:

• project(): but it's not that important
• PolygonError: not sure what road to follow here, need help/clarifications
• Draw methods: I'll fix the other pull request (Additional Draw methods to facilitate debugging #122)

- concave polygons: although I already have a lib that does this kind of stuff, I'm not sure we should add the code into FP. Maybe just a simple triangulator could do. But I can foresee a user requesting a way to turn their images into polygons which will lead to add a marching square implementation, and after that a way to simplify polygons, etc and well things will get out of hands pretty soon. Still open to ideas though!

Reply to this email directly or view it on GitHub:
#133 (comment)

[azrafe7]

The main problem with using just a triangulator is that it is not efficient, it would probably decompose the original concave polygon into a high number of triangles (f.e. see https://raw.github.com/azrafe7/as3GeomAlgo/master/screenshot.png).

Another thing to note: since the result of the triangulation is a list of triangles that would eventually be turned into a Masklist of Polygons in our case, building it right into the Polygon class doesn't sound right.

[AbelToy]

I'd throw an error when creating a concave polygon. Then later we can add some functions to create the Masklist of Polygons from a concave one.

[azrafe7]

I agree with @Rolpege, that's a reasonable compromise for the time being.

[azrafe7]

Testing polygons for simplicity (non self-intersection) and convexity (which are the costraints under which the mask should properly work).

Please test this further and report back if you encounter some bugs. If none we could drop this in the Polygon class.

code: https://github.com/azrafe7/FPPolygonProps
swf: https://dl.dropboxusercontent.com/u/32864004/dev/FPDemo/FPPolygonProps.swf

[AbelToy]

Assuming this code is similar to the one that was pushed to HaxePunk, an additional issue has arisen.

The new masks could have problems with hitboxes set using setHitbox instead of mask = new Hitbox. They won't take into account origins. This is because setHitbox sets width, height, originX and originY of an Entity already.

I think this should be changed, and all Masks should be using the mask properties and not the Entity's own properties (because entities can have multiple masks using Masklist, and other issues). Maybe this should be a separate issue...