streetcomplete / StreetComplete

Easy to use OpenStreetMap editor for Android
https://streetcomplete.app
GNU General Public License v3.0
3.9k stars 357 forks source link

Street parking overlay creates duplicated tags #4534

Closed tapetis closed 2 years ago

tapetis commented 2 years ago

The street parking overlay seems to tag ways with duplicated information for :left or :right and :both. From what I have seen so far, either the :right or :left key appears to duplicate the value of the :both key. I did not find any inconsistent information between these keys.

I do not know how to reproduce this issue, but I found many examples of this type of tagging on osmose, which reports this issue as "Too many parking:lane:[side]". Listed below are some ways that now have duplicated parking:condition and/or parking:lane tags after they were edited using StreetComplete:

Most of the changesets that introduced this type of tagging were created using v47.1 and v47.2 of StreetComplete.

tapetis commented 2 years ago

What all erroneous changesets linked above have in common is that beforehand the street parking tags were specified for only one side of the way. Afterwards, both sides have the same parking tags.

This bug seems to be related to the following code, which only removes the previous parking tags when they are found to be different from the parking tags chosen by the user (which is not the case):

https://github.com/streetcomplete/StreetComplete/blob/4cc76155b6da18a75e2cb4473291ca8c6224eaee/app/src/main/java/de/westnordost/streetcomplete/osm/street_parking/StreetParking.kt#L84-L86

The following code in this function then additionally specifies the :both tags.

A good question would also be why many users edit the two sides one after another and not simultaneously. Specifying just one side is only possible since 5bce8c52b5af7462d4e8fa2d606fe802f6664420, which makes this bug in the implementation more visible now.

westnordost commented 2 years ago

Nice, so you found the culprit?

A good question would also be why many users edit the two sides one after another and not simultaneously

This was actually a requested feature and I am now using it too, it is very handy. Sometimes you can only see one side of the street from where you are.

tapetis commented 2 years ago

Actually yes, but I don't think it's a good idea to just remove all :left or :right tags completely and just add new :both tags. When there are no changes to the already specified side, the previous parking:condition:side value should not be lost.

westnordost commented 2 years ago

Hmm, sounds reasonable. But is this implementable with reasonable complexity?

tapetis commented 2 years ago

But would a complicated implementation be a valid justification for potentially deleting correctly entered data? Wouldn't this also lead to problems if in the future this parking:condition:side data could be entered by StreetComplete itself?

westnordost commented 2 years ago

No, it wouldn't be a valid justification. To be honest, I was just trying to sneakily nudge you into looking into the code more in detail so that maybe you would create a PR to fix it :-P Sorry for that. But now that I confessed, would you? Otherwise of course, I'd come around to it of course, as this is a bug that must be fixed.

tapetis commented 2 years ago

I can try. So if for one side new parking tags are added while no existing tags of the other, already specified side are changed, no old parking tags should be lost. But once any existing parking tag is changed, all previous parking tags should be deleted for both sides as discussed in #4501?

westnordost commented 2 years ago

I guess if either one side is specified with null or the parking situation did not change for that side, the subtags for that side should not be touched.