Add a Facing enum to make facing of topology blocking segments easier to reason about #4484
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instead of an ambiguous boolean to indicate "front side", or elsewhere "facing away", we now indicate facing using the `Facing` enum. This enum is based on the fact that each line segment will have an island on one side and an ocean on the other side, thus allowing names that indicate the position of the origin point as being either on the island side or the ocean side.
- Fix RPTools#4437 more directly by not excluding the current island while iterating over siblings. - Fix facing of current ocean, while removing redundant null check against the parent.
- Fix RPTools#4469 by not excluding the parent ocean from blocking - Remove `excludeChildOcean` parameter as no one uses it anymore
kwvanderlinde
force-pushed
the
bugfix/4469-hill-vbl-grandparent-peninsula
branch
from
19e7baa
to
ee9df5c
Compare
cwisniew
approved these changes
Nov 23, 2023
cwisniew
added
the
code-maintenance
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Identify the Bug or Feature request
Fixes #4469
Also catches a backwards case in Cover VBL, which resulted in surrounding Cover VBL to have no effect.
Description of the Change
We used to use a boolean to indicate whether segments on the front side of a container should be returned. This was
ambiguous for several reasons:
faceAwayinAreaMetawhich was unclear for other reasons. For one, we aren't talking about whether the line segment itself points toward or away from the origin, but rather whether an imaginary orientation normal vector would point toward or away from the origin. And more importantly, the programmer had to remember the arbitrary choice of which direction (clockwise, counterclockwise) would give the expected orientation.Now we have a
Facingenum that can be used by all components involved, and which has decsriptive names. Since each line segment will have an island on one side and an ocean on the other, the names reflect that the origin might be on the island side of the line, or the ocean side of the line, without any need to remember orientation conventions.It's a lot easier now to see how each type of VBL works:
As for the Hill VBL bug fix, the new phrasing made it clear that that we should not be skipping the parent ocean when finding blocking segments for the grandparent island. As this was the only case where we did that, the logic for skipping oceans like that has been removed.
Possible Drawbacks
Should be none.
Documentation Notes
N/A
Release Notes
This change is