Combining ways: Roles not adjusted when changing direction

[skyper]

What steps will reproduce the problem?

1. Have a way with role forward/backward and a second connected way with opposite direction.
2. Combine the ways and have a look at the roles
3. JOSM combines by changing the direction of one way, depending on the selction order.

What is the expected result?

Ways are combined and if a direction of a way is changed its roles are properly adjusted

What happens instead?

The roles are not adjusted which leads to damaged relations.

Please provide any additional information below. Attach a screenshot if possible.

I did test with one untagged way which direction should always be changed in favour of the one with tags. Adding *=left/right to the tagged way exactly leads to this proper behaviour.

Repository Root: http://josm.openstreetmap.de/svn
Build-Date: 2013-08-13 01:35:36
Last Changed Author: Don-vip
Revision: 6143
Repository UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
URL: http://josm.openstreetmap.de/svn/trunk
Last Changed Date: 2013-08-11 23:47:22 +0200 (Sun, 11 Aug 2013)
Last Changed Rev: 6143

Identification: JOSM/1.5 (6143 en) Linux Debian GNU/Linux 7.1 (wheezy)
Memory Usage: 417 MB / 672 MB (126 MB allocated, but free)
Java version: 1.7.0_25, Oracle Corporation, OpenJDK 64-Bit Server VM
Java package: openjdk-7-jre:amd64-7u25-2.3.10-1~deb7u1
Program arguments: [/media/d95c6885-09a1-48fd-87c5-30922ffbd60f/hbf.osm]
Dataset consistency test: No problems found

Plugin: OpeningHoursEditor (29778)
Plugin: PicLayer (29809)
Plugin: conflation (0.1.6)
Plugin: jts (29613)
Plugin: mirrored_download (29753)
Plugin: notes (0.3)
Plugin: openstreetbugs (29435)
Plugin: reverter (29771)
Plugin: undelete (29555)
Plugin: utilsplugin2 (29771)
Plugin: wikipedia (29778)

[AlfonZ]

Osm file would be nice to have. I've tried following, but I am not sure, if it's what you meant.

Referring to 8968-test1.osm​.
Select BC, then CD, combine. CD is reversed, role of combined way is forward - OK.
Select CD, then BC, combine. CD is reversed, role of combined way is backward - incorrect, should be forward.
Remove sidewalk=right from BC. Select CD, then BC, combine. BC is reversed, role of combined way is backward - OK.

[Don-vip]

skyper: is AlfonZ' test file ok ? :)

[skyper]

Replying to Don-vip:

skyper: is AlfonZ' test file ok ? :)

Yes, only different was that my untagged way had an ID above zero which might have an influence in choosing the final direction.
Plus:

• combining BD and CD the result is always wrong.

[akks]

I tried to investigate this problem.

The problem is incorrect detection of the ways that should be reversed.

 if (!confirmChangeDirectionOfWays()) return null;
       // filter out ways that have no direction-dependent tags
       unreversedWays = ReverseWayTagCorrector.irreversibleWays(unreversedWays);  ////  (!) ONLY TAGS ARE ANALYZED
       reversedWays = ReverseWayTagCorrector.irreversibleWays(reversedWays);      ////  (!) NOT RELATION MEMBERSHIP
       // reverse path if there are more reversed than unreversed ways with direction-dependent tags
       if (reversedWays.size() > unreversedWays.size()) {
           Collections.reverse(path);
           ...
           //SWAP   unreversedWays <-> reversedWays
       }
       //  change tags and memebership info for each 
       for (Way w : reversedWays) {
            ... 
            changePropertyCommands = reverseWayTagCorrector.execute(w, wnew);
       }
       ...

If the way has no "direction-dependent" tag, then it does not appear in reversedWays list and is not even analyzed for relation membership.

Here is the dirty fix - just skipping irreversibleWays calls solve the problem.

[akks]

Another place in the code that looks strange:

for (Way w : reversedWays) {
    Way wnew = new Way(w);
    reversedTagWays.add(wnew);
    changePropertyCommands = reverseWayTagCorrector.execute(w, wnew);
}
if ((changePropertyCommands != null) && !changePropertyCommands.isEmpty()) {
    for (Command c : changePropertyCommands) {
        c.executeCommand();
    }
}
}

It seems that the commands are executed only for the last way.
Most of them are to be deleted, so we do not notice it?

[skyper]

updated test file

[skyper]

The behaviour did change but the bug is more or less still present:

changes:

• sadly, there is not auto solving of the role which always leads to the tag/membership conflict dialog even though all are solvable.

problem:

• the final direction of the way is not easy predictable as direction depending tags influence the behaviour.
• In the tag/membership conflict dialog
  • you have no hint about the final direction.
  • the roles need to be adjusted in prior as one of the two ways did change the direction.

There was way to solve these conflicting membership without looking twice or even three times at the outcome and undoing again.

Did update​ the test file and added the two expected solutions combining BC <-> CD and BD <-> CD as id:0 should always be reverted
BC <-> BD should produce a conflict but with properly adjusted roles in the diolog.

r11428