Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mini_roundabout #21007

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add mini_roundabout #21007

wants to merge 3 commits into from

Conversation

ivanPyrohivskyi
Copy link
Contributor

To issue #20417

@@ -1456,6 +1447,34 @@ private TurnType processRoundaboutTurn(List<RouteSegmentResult> result, int i, b
}
return t;
}

private TurnType processMiniRoundaboutTurn(List<RouteSegmentResult> result, int i, boolean leftSide) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time to move new code to separate class > 2000 lines already in this class

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create 2 new classes RoadSplitStructure and RoundaboutTurn

@@ -702,6 +701,47 @@ public boolean roundabout() {
return false;
}

public boolean miniRoundabout() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need these in generic class if they could be implemented as

public String getFirstPointTypeByTag(String tag)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed as redundant

@@ -1280,6 +1268,9 @@ private TurnType getTurnInfo(List<RouteSegmentResult> result, int i, boolean lef
if (rr.getObject().roundabout()) {
return processRoundaboutTurn(result, i, leftSide, prev, rr);
}
if (rr.getObject().miniRoundabout() && prev.getObject().miniRoundabout()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We check full method for roundabout ? Where is iteration point by point to test which point exactly is roundabout. What if there are 2 roundabouts on the way, roundabout in the middle.

Code looks suspiciously wrong

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realised in new class RoundaboutTurn isMiniRoundabout
int[] prevTypes = prev.getObject().getPointTypes(prev.getEndPointIndex());
int[] currentTypes = current.getObject().getPointTypes(current.getStartPointIndex());

int[] prevTypes = prev.getObject().getPointTypes(prev.getEndPointIndex());
int[] currentTypes = current.getObject().getPointTypes(current.getStartPointIndex());
if (prevTypes != null && currentTypes != null) {
Integer miniType = prev.getObject().region.decodingRules.get("highway#mini_roundabout");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RouteRegion.searchRouteEncodingRule

@vshcherb
Copy link
Member

vshcherb commented Nov 26, 2024

@ivanPyrohivskyi ivanPyrohivskyi marked this pull request as draft December 2, 2024 15:48
@ivanPyrohivskyi ivanPyrohivskyi marked this pull request as ready for review December 9, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants