Opened 5 years ago
Last modified 8 months ago
#15751 new enhancement
[PATCH] oneway=reversible|alternating for highways
Reported by: | naoliv | Owned by: | team |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | Internal preset | Version: | |
Keywords: | oneway reversible alternating | Cc: | Klumbumbus |
Description
With https://twitter.com/bhousel/status/948995008404729856 I guess that the usage of oneway=reversible|alternating
¹ might increase.
¹ https://wiki.openstreetmap.org/wiki/Key:oneway#List_of_values
I am unsure if JOSM should also include them in the presets, but at least it should ignore the values when validating (so we won't see messages like Value 'alternating" for key 'oneway' not in presets
)
JOSM:
Relative:URL: ^/trunk Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b Last:Changed Date: 2018-01-08 02:45:59 +0100 (Mon, 08 Jan 2018) Revision:13300 Build-Date:2018-01-08 01:49:06 URL:http://josm.openstreetmap.de/svn/trunk Identification: JOSM/1.5 (13300 pt_BR) Linux Debian GNU/Linux testing (buster) Memory Usage: 1349 MB / 7168 MB (769 MB allocated, but free) Java version: 9.0.1+11-Debian-1, Oracle Corporation, OpenJDK 64-Bit Server VM Screen: :0.0 1600x900, :0.1 1280x1024 Maximum Screen Size: 1600x1024 Java package: openjdk-9-jre:amd64-9.0.1+11-1 Java ATK Wrapper package: libatk-wrapper-java:all-0.33.3-13 VM arguments: [--add-modules=java.activation,java.se.ee, -Dawt.useSystemAAFontSettings=on] Dataset consistency test: No problems found
Attachments (7)
Change History (44)
comment:1 Changed 5 years ago by
Cc: | Klumbumbus added |
---|
comment:2 Changed 5 years ago by
OK, but I think we need two way arrows too. Simply using our current oneway arrows would be a bit confusing.
comment:3 follow-up: 4 Changed 5 years ago by
That's what I meant: is JOSM already capable of displaying two arrows, or do we need to implement it first?
comment:4 Changed 5 years ago by
Replying to Don-vip:
is JOSM already capable of displaying two arrows?
I don't think so.
If you implement this maybe consider if it makes sense (and not too much work) to make them adjustable, so we could display oneway:bycycle=no arrows in a different style, similar to opencyclemap.
comment:5 Changed 5 years ago by
Which tag can I change to have vehicles drive on the left hand side?
comment:6 Changed 12 months ago by
looks like the alternating|reversible modeling is not supported still by josm. While that is true I am not seeing the warning Value 'alternating" for key 'oneway' not in presets
as stated above when I use oneway=reversible|alternating? Has this been resolved?
Changed 12 months ago by
Attachment: | josm_15751.patch added |
---|
patch silencing warning about both values for now
comment:7 Changed 12 months ago by
Nothing has changed but the common warning about values not in presets is an informal (other) warning and still present once these warnings are enabled in preferences.
I've attached a patch silencing validator for now. Should be removed again once these values enter defaultpresets.
In my eyes, the most interesting part are more flexible arrows like more than one and different/multiple colors. This is probably worth a new ticket.
comment:8 Changed 12 months ago by
Keywords: | oneway reversible alternating added |
---|---|
Summary: | oneway=reversible|alternating for highways → [Patch] oneway=reversible|alternating for highways |
comment:9 Changed 12 months ago by
Milestone: | → 21.08 |
---|
comment:10 Changed 12 months ago by
I think we should add the two values to the defaultpresets instead. Or is there something blocking it (except that we need to transform the checks into combos)?
comment:11 Changed 12 months ago by
Component: | Core validator → Internal preset |
---|---|
Milestone: | 21.08 |
Summary: | [Patch] oneway=reversible|alternating for highways → oneway=reversible|alternating for highways |
Besides medium to low numbers of use and only in use
, e.g. no proposal, there is nothing wrong or disputing. Changing the checks to combos has some impact on the presets layout, though, especially, in places where grouped checks are used. So little work to do but shouldn't be that big problem
comment:12 Changed 12 months ago by
I want to make sure I am understanding the defaultpresets.xml file properly:
for id="highway_base"
is it not as simple as adding -> <combo key="oneway" text="Oneway" values="yes,no,reversible,alternating" values_sort="true" />
What is id="highway_base"
referring to, a way with a highway tag present on it?
IS there any documentation on how the xml file works for presets?
comment:13 follow-up: 14 Changed 12 months ago by
id=""
is the name of the `<chunk>` which is used in several <item>
or <chunk>
with <reference ref="" />
<check />
can be grouped together with <checkgroup>
so simply changing the <check />
into a <combo />
does not work in all places. Additionally, the layout and order of tags in the presets should be looked at.
comment:14 Changed 12 months ago by
Replying to skyper:
id=""
is the name of the `<chunk>` which is used in several<item>
or<chunk>
with<reference ref="" />
<check />
can be grouped together with<checkgroup>
so simply changing the<check />
into a<combo />
does not work in all places. Additionally, the layout and order of tags in the presets should be looked at.
Thank you for the information! I will get on this!
comment:15 Changed 12 months ago by
Am I correct that the oneway presets we are adding should be on anything with a highway tag?
answered this on my own. 99.14% of features with the oneway
tag on it are on features with highway
tag.
Changed 12 months ago by
Attachment: | 15751_v1.patch added |
---|
move oneway checks to combos that include reversbiel|alternating
comment:16 Changed 12 months ago by
15751_v1.patch includes all of the oneway check boxes converted to combos which include the desired values reversible|alternating
.
comment:17 Changed 12 months ago by
Summary: | oneway=reversible|alternating for highways → [PATCH] oneway=reversible|alternating for highways |
---|
comment:19 follow-ups: 20 21 Changed 12 months ago by
Oh, forgot about the last enhancement of placement of <checkgroup>
. So only some columns need adjustment but I need to take a closer look and fear that some more rearranging is useful to get a nice layout of consecutive checkgroups. Especially 'highway_yesno_incline_oneway_lit_width
' does not look nice anymore.
comment:20 Changed 12 months ago by
Replying to skyper:
Oh, forgot about the last enhancement of placement of
<checkgroup>
. So only some columns need adjustment but I need to take a closer look and fear that some more rearranging is useful to get a nice layout of consecutive checkgroups. Especially 'highway_yesno_incline_oneway_lit_width
' does not look nice anymore.
I will check these and fix them!
Changed 12 months ago by
Attachment: | 15751_v2.patch added |
---|
only problem I can't solve is having evenly spaced checkboxes on Motorway, Motorway_Link, Trunk, and Trunk_Link. There are 2 rows of checkboxes and the 2nd row is not spaced out the same as the 1st row.
comment:21 Changed 12 months ago by
Replying to skyper:
Oh, forgot about the last enhancement of placement of
<checkgroup>
. So only some columns need adjustment but I need to take a closer look and fear that some more rearranging is useful to get a nice layout of consecutive checkgroups. Especially 'highway_yesno_incline_oneway_lit_width
' does not look nice anymore.
Patch 15751_v2 looks pretty good now with 5 columns for 'highway_yesno_incline_oneway_lit_width
', just need to figure out the spacing as mentioned with the patch attachment:
only problem I can't solve is having evenly spaced checkboxes on Motorway, Motorway_Link, Trunk, and Trunk_Link. There are 2 rows of checkboxes and the 2nd row is not spaced out the same as the 1st row.
comment:22 Changed 12 months ago by
The spacing problem can only be solved by using one checkgroup instead of several (or core needs to be enhanced).
I took a closer look and found a minor issue regarding oneway:bicyle
which should be right below oneway
.
In general, I would rework the chunks and checkgroups.
- I believe
lit
could be added to "highway_yes/no" as even rails are sometimes lit and all highways can be for sure. incline
is also useful for all.- Regarding the spacing all checks should be in one checkgroup which means some more chunks are needed for e.g.
motorroad
andtoll
- Maybe, having chunks for only the checks and defining the checkgroups in another chunk together with the other tags or even in the item itself works better.
- Another option might be moving all tags besides the checks into own chunks and either group them in another chunk or reference both chunks in the item.
Changed 11 months ago by
Attachment: | 15751_v3.patch added |
---|
Bandaid for now. New ticket should be made for revamp of the defaultpresets.xml file.
comment:23 Changed 11 months ago by
So, patch 15751_v3 moved lit
, toll
, and motorroad
to the "highway_yesno" chunk. This puts them in a lot of presets where they won't be used but for now it seems fine since toll and lit are useful in most scenarios. Motorroad can be moved around in the new ticket for a defaultpresets revamp?
I can create that ticket for a defaultpresets.xml
file revamp. (#21308)
comment:24 Changed 11 months ago by
toll
is in "Road Restrictions" which is linked to withpreset_link
and might not be needed at all.motorroad
should only be present with useful combinations, e.g. own chunk.- there is no need for a checkgroup with only one check (
oneway:bicycle
). - Seven checks in a row might be to much, I'd rather use
columns=4
and have the checks spread over two rows.- Five checks in a row should work, so this might be only interesting for groups with
motorroad
and/ortoll
.
- Five checks in a row should work, so this might be only interesting for groups with
comment:25 Changed 11 months ago by
- So remove
toll
fromRoad Restrictions
?
- Should
motorroad
have its own individual chunk with onecheck
in it? Reference to achunk
with checks is not allowed withincheckgroups
so it would be a single check on one its own line.
- removed checkgroups with only one check!
- Easily can the columns!
comment:26 Changed 11 months ago by
Sorry, my last comment was a bit miss-understandable.
- I thought about removing
toll
from the primary presets and leave "Road Restriction" untouched. After a second look, I see, thatoneway
is present in both, too, but I did not check the opposite, ifoneway
is present in every primary preset linking to "Road Restrictions".- Atm, the same tag in a primary preset and a linked presets does not work, see #18992.
- Only one checkgroup for all.
motorroad
should be in the chunk (checkgroup) fortrunk[_link]
andprimary[_link]
.primary[_link]
has the check below in a second group which is irritating.
Checkgroup should work pretty well after the changes some month ago, even with multiple columns like nine checks with columns=4
.
Some of the cleanup, like the same tag in the primary and the linked to preset, could be made in #21308 or #18992 might get fixed. In the end, I can life with little duplication as "Road Restriction" and "Access Restrictions" are useful with the links to presets from property/membership panel.
I am sorry for the amount of critics and some problems like the spacing and a strange vertical space (#21319) are present atm, too. These checkgroup chunks and the highway presets are quite complicated and not the easiest task for the first time modifying defaultpresets.xml.
Changed 11 months ago by
Attachment: | 15751_v4.patch added |
---|
moved toll
and oneway
to Road Restrictions
item and moved motorroad to Access Restrictions
.
comment:27 follow-up: 28 Changed 11 months ago by
No worries at all, our discussions are enjoyable and productive!
patch 15751_v4 moved toll
/oneway
to Road Restriction
item from their original chunks. Also moved motorroad
to Access Rstrictions
since it historically has been used on Motorway
through Secondary
and not just the links. duplicates in linked preset should be eliminated.
Max amount of columns used is 4 because it looks good so far.
comment:28 follow-up: 29 Changed 11 months ago by
Replying to reichg:
No worries at all, our discussions are enjoyable and productive!
Nice, that you take it this way. I sometimes fear that I miss the point and that I am not clear enough.
patch 15751_v4 moved
toll
/oneway
toRoad Restriction
item from their original chunks. Also movedmotorroad
toAccess Restrictions
since it historically has been used onMotorway
throughSecondary
and not just the links. duplicates in linked preset should be eliminated.
+1 for adding motorroad
to "Access Restrictions" but in my eyes, I would not remove that many tags from the primary presets, especially tags with high numbers as combination, which leads us to #21308.
I really hope that #18992 will get fixed soon as little duplication does not harm and makes it more flexible. Looking at external presets which extend other presets like Lane Attributes or Public Transport GTFS duplication exist as I do not want to rip some keys out of context once they enter defaultpresets.
comment:29 Changed 11 months ago by
Replying to skyper:
Replying to reichg:
No worries at all, our discussions are enjoyable and productive!
Nice, that you take it this way. I sometimes fear that I miss the point and that I am not clear enough.
patch 15751_v4 moved
toll
/oneway
toRoad Restriction
item from their original chunks. Also movedmotorroad
toAccess Restrictions
since it historically has been used onMotorway
throughSecondary
and not just the links. duplicates in linked preset should be eliminated.
+1 for adding
motorroad
to "Access Restrictions" but in my eyes, I would not remove that many tags from the primary presets, especially tags with high numbers as combination, which leads us to #21308.
I really hope that #18992 will get fixed soon as little duplication does not harm and makes it more flexible. Looking at external presets which extend other presets like Lane Attributes or Public Transport GTFS duplication exist as I do not want to rip some keys out of context once they enter defaultpresets.
so can I just add the one off motorroad
to the Motorway - Secondary items for now so we can close this ticket and move on to #21308? Wouldn't it be best to have all duplicates removed between primary and linked presets in the future for simple organization sake? I can work on the #21308.
comment:30 Changed 11 months ago by
I am fine with that.
I do not think that we get rid of all duplicates. We want to emphasis on certain tags like motorroad=yes
together with highway=trunk[_link]/primary[_link]
but still want to have it for general use in "Access Restrictions". Similar with oneway=*
which is important for highway=motorway_link
and useful together with every highway link but not that important for path.
Changed 11 months ago by
Attachment: | 15751_v5.patch added |
---|
moves motorroad back into motorway
- primary_link
comment:31 Changed 11 months ago by
Just added motorroad
where it probably looks best, added a space under the name/ref checkboxes.
comment:33 Changed 11 months ago by
- Please, be careful with
<space />
at the top or bottom of chunks. E.g. in many presets there is two much empty space belowsmoothness=*
, now. - I still do not like that
oneway=*
is only present in the "Road Restriction" preset, now, but less important keys are in the main preset . Plus themaxspeed=*
entry are still in both and somehow depend ononeway=*
as backward/forward make seldomly sense withoneway=yes
.- additionally
oneway:bicycle
is now lonely, again, butoneway=*
should be right above. No objections to addoneway:bicyle=*
to "Road Restrictions" even though it adds another duplication.
- additionally
- Can we stick to this ticket and add the values for
oneway=*
plus adjust the individual chunks but leave out the moving or removing of keys in the presets for another ticket?
Changed 11 months ago by
Attachment: | 15751_v6.patch added |
---|
removed spaces at the beginning/end of chunks, added oneway combo in the highway_yesno
chunk.
comment:34 Changed 11 months ago by
Let me know if anything else needs to be moved around or if I need to revert some of the other changes not applicable to this ticket.
comment:35 Changed 11 months ago by
- Oh, I just noticed that all
default=""
values are dropped. Regardingmotorroad
there should be no problem to keep it. Regardingoneway
only one chunk for all highway classes does not work if we want to keep it. - I am not sure about
motorroad
for secondary highways and below. According to taginfo the combination has only low numbers ([1], [2]). I'd rather would add it to the Access Restriction preset and would life with another duplicate for primary highways and above. - Do we need the
<space />
abovemotorroad
? At least it should be consistent. Right now it is a mix.
comment:36 Changed 11 months ago by
Oh, one more: I would not change the order of the checks on top of "Road Restriction" preset. Categorical order is better than alphabetical one, especially, if you count all translation languages.
comment:37 Changed 8 months ago by
@reichg:
Are you still working on this?
In #21396 oneway:moped=no
is mentioned as next oneway tag which needs to be added, but how?
Definition looks clear enough for me. Can we render these ways as iD does?