[Shapes] Wrong Polygon Contains behaviour

[Spessi]

Hi, due to the missing comments in the PolygonF class, I'm not sure if the Contains method behaves correctly or incorrectly.

Here's a code example:

// Create 4 vertices, representing a square, starting at (10/10) and ending at (20/20)
List<Vector2> testVertices = new List<Vector2>();
testVertices.Add(new Vector2(10, 10));
testVertices.Add(new Vector2(10, 20));
testVertices.Add(new Vector2(20, 20));
testVertices.Add(new Vector2(20, 10));

// Create a polygon out of the previously created vertices
PolygonF testPoly = new PolygonF(testVertices);

// Let's do some edge tests
bool res;
res = testPoly.Contains(10, 10); // true
res = testPoly.Contains(20, 20); // false

What I'd expected is that both results are true or false (in my case, both true would be better).

I'll now try to understand the algorithm and fix it, so that both test cases will be true. If you confirm it's a bug and not a wanted behaviour, I'd try to submit a PR (it's my first time :-)).

[craftworkgames]

To consistent with #213 we want Contains to behave the same way it does on standard MonoGame shapes like Rectangle. I'm not certain what this behavior actually is so if you want to investigate that'd be great.

Unfortunately, when returning a bool for something that actually has 3 states (contains, not contains, touching) it's always going to be ambiguous. That said, MonoGame is the source of truth here, so we need to match it.

(in my case, both true would be better).

May I ask what you using these for out of curiosity?

[charles-esterbrook]

Unfortunately, when returning a bool for something that actually has 3 states (contains, not contains, touching) it's always going to be ambiguous. That said, MonoGame is the source of truth here, so we need to match it.

Should we have an additional method that returns the relation?

enum Relation { Outside, Inside, Touches };

[craftworkgames]

Should we have an additional method that returns the relation?

Yeah we could do that. I'm pretty sure it's common practice in physics engines. It might be worth taking a look at how Farseer does it or one of the others. I don't think they call it a relation.

[craftworkgames]

Any one want to take a closer look at this bug?

[lithiumtoast]

I'll get around to this with collisions. Though, I will be gutting the PolygonF struct in favour of a Polygon class.

[craftworkgames]

@LithiumToast have you already finished the new shape classes? It might be worth pulling those in sooner rather than later.

[lithiumtoast]

Troubled code is not standard with the orange book. Will need to look at all the surrounding code for the context of what is good and not so good.