Add "flow_direction" to directed tags.#12359
Conversation
| 'unknown' : true, | ||
| // instead of BiDirectional, 'unknown' might fit better under a new "UnDirectional" category |
There was a problem hiding this comment.
would it not be better to just omit unknown then? then, no arrows are shown at all, which is more true to the actual meaning of the tag.
https://wiki.openstreetmap.org/wiki/Key%3Aflow_direction does not include the value unknown either, which would also be an argument against including it.
There was a problem hiding this comment.
No, this tag is meant to be used on waterways which inherently have arrows displayed.
I'm not satisfied with using oneway=no, since it describes traffic and doesn't have an "unknown" value.
I don't know what it'd take to make flow_direction able to just remove arrows, but after some testing how oneway and "conveying" work, this seems like the best option.
There was a problem hiding this comment.
https://github.com/openstreetmap/iD/blob/develop/modules/osm/way.js#L144
I've now found out why oneway=no removes arrows entirely. It has been hardcoded into both isOneWayForwards, isOneWayBackwards, isBiDirectional and isOneWay.
I don't find this to be an elegant solution, so I don't want to replicate that.
There was a problem hiding this comment.
Valid point. Implementing a more elegant solution is relatively straightforward (it was just not necessary, as oneway=no was the only tags with this connotation until now). I added this to this branch in 15b2099
That said, I'm still very hesitant to merge this as is: The tag value unknown is neither documented on the current wiki page nor was it mentioned in the original proposal. Such a tag must first be discussed in the general community, then properly documented in the wiki, and only then implemented in editors.
There are two options now:
- start discussion about the new tag
flow_direction=unknown, and after a grace period, merge this PR - amend the PR to omit the
flow_direction=unknowntag for now (then it can be merged sooner), and start the discussion about the new tag in parallel (with a follow-up PR to complete the implementation at a later point)
There was a problem hiding this comment.
Value "unknown" is defined for many other keys with the same meaning, so it shouldn't be a problem. Alternatively, as you suggested in the first comment, "unknown" doesn't need to be defined if unfamiliar values of flow_direction were to default to directionlessness.
I am looking forward to checking out your changes. Will they appear in https://ideditor.netlify.app/ soon?
The tags interact somewhat strangely as is. flow_direction and the like need to be able to overrule the direction inherent to waterway=ditch. Have you somehow given lower priority to the direction of "feature type" keys like waterway and aerialway?
(The reason that I am the one who invented the flow_direction=unknown combination is that I've been placing waterways in forests in Denmark using Terrain Shadow Map, and even with a height map in a separate web pane, many ditches are too flat and/or short to determine direction.)
re: openstreetmap/iD#12359 This also adjusts the `OsmWay.ts` tests a bit. The code had already been rewritten to use the rulesets, but this commit adds a few more conditions worth checking, and updates the test descriptions.
2 participants
Adds flow_direction to directed tags (like 'oneway' and 'conveying').