[PATCH] Fix relation ordering after split-way

[JeroenHoek]

In some cases the split-way command failed to detect the direction of ways part of a relation by breaking too soon from the loop that inspects relation members. This commit fixes this.

The types of relations affected are route, multipolygon, and boundary relations; e.g., relations containing ordered sets of ways.

Additionally, a warning is shown to the user if by splitting the way a relation may have been damaged. This warning is raised when all of these requirements are met:

• the relation concerned is a route, multipolygon, or boundary relation,
• has more than one member,
• does not have all of its members downloaded, and
• the direction could not be determined from the available members.

Background

This bug causes routes for public transport, hiking, and cycling to break when edits are performed on small parts of the map — e.g., when a mapper is working on details in a neighbourhood. It is common for such relations to have only a few members downloaded.

The split way command tries to determine the order of the new parts by looking at the available members, but due to a bug in the detection loop this can fail even when at least two connected members are available. This specific bug is hit when a way is split that is at one end of the downloaded members: in that case there is a 50% chance (depending on the position of the member in the relation) that the split will place the new parts in the wrong order. This commit solves this by not breaking from the detection loop too soon.

[JeroenHoek]

The warning is shown when only one member of a relation is downloaded (warning.png).

[skyper]

Thanks.
Please, add type boundary relations.

You do not need to download all members only the neighbours, so, the message is wrong.
Did you test it with ways which are multiple times part of a relation ?
Did you have a look at #18018 ?
Did you test it with unordered relations ?

Replying to JeroenHoek:

The warning is shown when only one member of a relation is downloaded (warning.png).

I would prefer to have JOSM deal this case by downloading one (or two) more members to solve it automatically. At least offer to download with the warning or do it silently.

[anonymous]

Please, add type boundary relations.

I can do that.

You do not need to download all members only the neighbours, so, the message is wrong.

JOSM doesn't have an option to download just the neighbours (although users can do this manually). The warning intends to point the user to the 'download all members' option in the context menu of a relation.

Did you test it with ways which are multiple times part of a relation ?

Yes. Looks fine.

Did you test it with unordered relations ?

What kind of behaviour do you expect? This patch does not appear to break them. The warning is only shown for ordered relations (routes, multipolygons).

Did you have a look at #18018 ?

I have tested it, and it looks to be a different kind of bug. I may look into it after this patch has landed, but it is not related to the issue I am fixing here.

I would prefer to have JOSM deal this case by downloading one (or two) more members to solve it automatically. At least offer to download with the warning or do it silently.

That is a good suggestion, but I would prefer to tackle that in a future patch in order to keep the scope of this patch limited.

[skyper]

Replying to anonymous:

You do not need to download all members only the neighbours, so, the message is wrong.

JOSM doesn't have an option to download just the neighbours (although users can do this manually). The warning intends to point the user to the 'download all members' option in the context menu of a relation.

I you want the user to download hundreds of objects instead of two nodes with their parent ways, fine. Still the question between "incomplete" members or "all" members needs to be find.

Did you test it with unordered relations ?

What kind of behaviour do you expect? This patch does not appear to break them. The warning is only shown for ordered relations (routes, multipolygons).

Thought about relations which are broken already.

[JeroenHoek]

Updated patch now also applies same strategy to boundary relations.