The current code is broken for Square goals and can be ambiguous in certain situations, as well as the different functions returning incompatible results in rare edge (literally) cases.
This revision removes those incoherences.
Differential D1829
Fix Pathgoal::RectContainsGoal family of function for SQUARE goals, reduce ambiguities, improve coherence among the different functions. wraitii on Apr 19 2019, 5:51 PM. Authored by
Details
The current code is broken for Square goals and can be ambiguous in certain situations, as well as the different functions returning incompatible results in rare edge (literally) cases. This revision removes those incoherences. Review.
Diff Detail
Event TimelineComment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/1229/display/redirect
Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/1246/display/redirect
Comment Actions Mhpf, you're right. Fix other whitespace in this file while I'm at it. @bb Do you agree with the intent of this patch (beyond comment clarity issues)? Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/1249/display/redirect Comment Actions I've realised there is a much more fundamental problem in RectContainsGoal (and related functions): our case for square goals is not sufficient! We were checking if the point on the rect closest to the center of the goal was inside the goal, but that's not enough since the goal can be rotated arbitrarily. Indeed we have to used the Separating Axis Theorem in full force, unless I've missed something. @Itms @echotangoecho (and anyone else really) I think you're both much better at geometry than I am, so I would appreciate if you could give this a look. It passes the tests and fixes the one empirical edge case I noticed, so I'm relatively confident, but still. Incidentally, I'm also changing the rules for 'strictly' to be coherent across all functions, and I am also removing the (insane, frankly) code duplication in this file. Will affect performance (possibly slower, possibly faster, hard to say for sure), and will change hashes since the previous code was incorrect. With this I can should be able to make at least the reachable case from D1834 (the 'fast' case) not change svn hashes, which will give me high confidence in committing that performance improvement. Comment Actions Successful build - Chance fights ever on the side of the prudent. Linter detected issues: Executing section Source... source/simulation2/helpers/PathGoal.h | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" source/simulation2/helpers/PathGoal.cpp | 1| /*·Copyright·(C)·2018·Wildfire·Games. | | [NORMAL] LicenseYearBear: | | License should have "2019" year instead of "2018" Executing section JS... Executing section cli... Link to build: https://jenkins.wildfiregames.com/job/differential/1321/display/redirect Comment Actions Rereading, this seems somewhat incorrect & RectContainsGoal is actually unused except in that test, so I'm abandoning this for now. |