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

Implementation of ClickableWay for solitary ways (piste / dirtbike / mtb) #21714

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

Conversation

RZR-UA
Copy link
Contributor

@RZR-UA RZR-UA commented Jan 13, 2025

No description provided.

@RZR-UA
Copy link
Contributor Author

RZR-UA commented Jan 13, 2025

@RZR-UA RZR-UA force-pushed the rzr-select-way branch 2 times, most recently from 45fe60e to 625b96f Compare January 13, 2025 14:09
@RZR-UA RZR-UA requested a review from Chumva January 16, 2025 11:55
@RZR-UA RZR-UA changed the title [draft] Implementation of ClickableWay for solitary ways (piste / dirtbike / mtb) Implementation of ClickableWay for solitary ways (piste / dirtbike / mtb) Jan 16, 2025
@RZR-UA RZR-UA marked this pull request as ready for review January 16, 2025 11:55
@@ -106,10 +108,13 @@ public class MapSelectionHelper {
private Map<LatLon, BackgroundType> touchedFullMapObjects = new HashMap<>();
private Map<LatLon, BackgroundType> touchedSmallMapObjects = new HashMap<>();

private ClickableWayHelper clickableWayHelper;
Copy link
Member

Choose a reason for hiding this comment

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

final ?

@@ -1365,4 +1365,25 @@ public static String splitAndClearRepeats(String ref, String symbol) {
return res;
}

public static String sanitizeFileName(String fileName) {
return fileName
Copy link
Member

Choose a reason for hiding this comment

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

wont replaceAll be shorter & still readable?

Comment on lines +355 to -351
if (isOsmRoute && !isDerivedGpxSelected) {
NetworkRouteSelectorFilter routeFilter = createRouteFilter();
if (!Algorithms.isEmpty(routeFilter.typeFilter)) {
addOsmRoute(result, tileBox, point, routeFilter);
isRouteGpxSelected = true;
isDerivedGpxSelected = true;
}
} else if (isClickableWay && !isDerivedGpxSelected) {
isDerivedGpxSelected = addClickableWayV2(result, obfMapObject, tags);
} else if (isTravelGpx && !isDerivedGpxSelected) {
isDerivedGpxSelected = addTravelGpx(result, tags.get(ROUTE_ID), null);
}
boolean isTravelGpx = app.getTravelHelper().isTravelGpxTags(tags);
if (isTravelGpx && !isRouteGpxSelected) {
isRouteGpxSelected = addTravelGpx(result, tags.get(ROUTE_ID), null);
}
Copy link
Member

@Chumva Chumva Jan 20, 2025

Choose a reason for hiding this comment

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

review conditions, simplify if possible, and at least try to move check for !isDerivedGpxSelected to upper level

Comment on lines +493 to +510
private boolean addClickableWayV1(@NonNull MapSelectionResult result, @NonNull RenderedObject renderedObject) {
ClickableWay clickableWay = clickableWayHelper.loadClickableWayV1(result.pointLatLon, renderedObject);
if (clickableWay != null && isUniqueClickableWay(result.selectedObjects, clickableWay)) {
result.selectedObjects.put(clickableWay, clickableWayHelper.getContextMenuProvider());
return true;
}
return false;
}

private boolean addClickableWayV2(@NonNull MapSelectionResult result, @NonNull ObfMapObject obfMapObject,
@NonNull Map<String, String> tags) {
ClickableWay clickableWay = clickableWayHelper.loadClickableWayV2(result.pointLatLon, obfMapObject, tags);
if (clickableWay != null && isUniqueClickableWay(result.selectedObjects, clickableWay)) {
result.selectedObjects.put(clickableWay, clickableWayHelper.getContextMenuProvider());
return true;
}
return false;
}
Copy link
Member

@Chumva Chumva Jan 20, 2025

Choose a reason for hiding this comment

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

There is the same part, can we combine similar code? Also, do we always need to specify versions?

return false;
}

private boolean isUniqueGpxFileName(Map<Object, IContextMenuProvider> selectedObjects, String gpxFileName) {
Copy link
Member

Choose a reason for hiding this comment

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

annotations


import java.util.List;

public class ClickableWayMenuActivator implements ContextMenuLayer.IContextMenuProvider {
Copy link
Member

Choose a reason for hiding this comment

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

ClickableWayMenuProvider ?

return bbox;
}

public GpxFile getGpxFile() {
Copy link
Member

Choose a reason for hiding this comment

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

nonnull annotations

if (object instanceof ClickableWay that) {
MapActivity mapActivity = view.getMapActivity();
if (mapActivity != null) {
(new ClickableWayAsyncTask(mapActivity, that, readHeights, openAsGpxFile))
Copy link
Member

@Chumva Chumva Jan 20, 2025

Choose a reason for hiding this comment

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

Better add task cancelling similar to NetworkRouteSelectionLayer, probably routesCache we also can add here similar to NetworkRouteSelectionLayer

Comment on lines +198 to +202
List<WptPt> waypoints = loader.loadHeightDataAsWaypoints(clickableWay.getOsmId(), clickableWay.getBbox());
if (!Algorithms.isEmpty(waypoints)
&& !Algorithms.isEmpty(clickableWay.getGpxFile().getTracks())
&& !Algorithms.isEmpty(clickableWay.getGpxFile().getTracks().get(0).getSegments())) {
clickableWay.getGpxFile().getTracks().get(0).getSegments().get(0).setPoints(waypoints);
Copy link
Member

Choose a reason for hiding this comment

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

If we replace existing points from RenderedObject and ObfMapObject with points from RouteDataObject, will there be any issues if the points count is wrong?

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