Fix freeze when actor width or height is 0 and tilemap is shown

[DaVince]

Closes #1418

TileMapImpl.collides() got stuck infinitely on the trace points for overlap loop when the actor's size is zero.

Changes:

• Check size of actor before deciding whether to run the collision logic
• Return null when actor width or height is 0

[DaVince]

This is my second merge request on an open source project ever, please let me know if I did it right! 🙂

[DaVince]

In the failing test, an object with a non-zero size is being tested, but it fails anyway. My implementation might have been naive. Will look into this further.

[DaVince]

Test Collision Shape a Circle should collide with other polygons when touching fails, but I have no clue why, because on further inspection it does nothing with TileMaps. Is it even related? Additionally, it seems to have succeeded on subsequent unrelated commits?

Testing the code tells me collisions work; they don't seem to have broken. Except for zero-size actors, which now are not checked at all. Is this behavior acceptable?

On another note: can actor widths and heights ever be a negative value?

[eonarheim]

I think this is a flakey test that was discovered, not related to TileMaps. I think negative widths/heights are unexpected and should be an error. Zero width and heights do have a use.

…

On Tue, Apr 14, 2020 at 8:50 AM Vincent Beers ***@***.***> wrote: Test 'should collide with other polygons when touching' fails, but I have no clue why, because on further inspection it does nothing with TileMaps. Is it even related? On another note: can actor widths and heights ever be a negative value? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1491 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEVNZYFXJGU25UKVZDJPQ3RMRS2FANCNFSM4MHUXCBA> .

[eonarheim]

@kamranayub ^ Any more info on this test

[eonarheim]

@DaVince This looks great, is it possible to add a test for this fix? (ignore the flakey test)

[eonarheim]

If it is too onerous to add a test for this let me know.

[kamranayub]

I wouldn't worry about that test, it has failed in master before and will pass on a rebuild. Definitely flaky.

[DaVince]

Thanks for all the great feedback!

@DaVince This looks great, is it possible to add a test for this fix? (ignore the flakey test)

Sure, I'm on it.

[eonarheim]

@DaVince Thanks again for the contribution!