Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more barrier presets #506

Merged
merged 12 commits into from Jun 23, 2022
Merged

Add more barrier presets #506

merged 12 commits into from Jun 23, 2022

Conversation

arch0345
Copy link
Contributor

This PR adds presets for the following barriers listed on the wiki:

I used the default barrier icon for most of these, but ideally those should get their own icons. For example, barrier=jersey_barrier could have an icon similar to this image depicting the general outline:
Jersey barrier 2
The preset for full-height_turnstile could have an icon similar to the one used in OSM carto:
Jersey barrier 2

Since the wiki article for barrier=bus_trap recommends also using maxwidth, I created a field for it using the same field type used for maxheight since their values are similar.

I also added barrier=ditch, barrier=handrail, and barrier=log as unsearchable presets. For barrier=ditch, waterway=ditch has a lot more uses and having making both of these searchable might be confusing for some users. The wiki article for barrier=handrail mentions that there's an "Unclear difference from fence". Although barrier=log is an approved tag, I made it unsearchable to prevent mappers from adding fallen trees that don't obstruct a passage.

Closes #505

@danieldegroot2
Copy link
Contributor

barrier=planter should not be confused with man_made=planter;

See also man_made=planter One object can be tagged both with man_made=planter and barrier=planter, planters that are not barriers should be tagged with just man_made=planter.

@arch0345 arch0345 marked this pull request as draft June 20, 2022 07:11
data/presets/barrier/hampshire_gate.json Show resolved Hide resolved
data/presets/barrier/motorcycle_barrier.json Show resolved Hide resolved
data/presets/barrier/rope.json Outdated Show resolved Hide resolved
data/presets/barrier/wicket_gate.json Outdated Show resolved Hide resolved
arch0345 and others added 4 commits June 21, 2022 08:46
Co-authored-by: Martin Raifer <martin@raifer.tech>
* Remove `barrier=planter` preset
* Add `planter_barrier` field which adds `barrier=planter`
* Add `man_made=planter` preset with `planter_barrier` field
@arch0345
Copy link
Contributor Author

arch0345 commented Jun 21, 2022

One object can be tagged both with man_made=planter and barrier=planter, planters that are not barriers should be tagged with just man_made=planter.

Because of this, I decided to change the preset to use man_made=planter and added a field (planter_barrier) to add barrier=planter, similar to how the crossing_raised field adds traffic_calming=table to the crossing presets.

@tyrasd
Copy link
Member

tyrasd commented Jun 21, 2022

Because of this, I decided to change the preset to use man_made=planter and added a field (planter_barrier) to add barrier=planter, similar to how the crossing_raised field adds traffic_calming=table to the crossing presets.

That sounds like a good approach 👍

But could you please rename the field to data/fields/barrier_planter.json (the first part of the field's file name should be the tag key it operates on)?

@arch0345 arch0345 marked this pull request as ready for review June 21, 2022 23:33
This allows to show the "access" field if a man_made=planter vertex has also the barrier=planter tag.
@tyrasd
Copy link
Member

tyrasd commented Jun 22, 2022

planter preset

I just noticed that, unfortunately, this approach doesn't allow us to show the access field for planter objects when they the barrier=planter tag is set. In 87e795f I've now added a non-searchable preset called Planter (Barrier) (for vertex nodes only) which affords this. Do you think that solution is acceptable?

@tyrasd tyrasd force-pushed the main branch 2 times, most recently from 9d3204d to 49f529e Compare June 22, 2022 16:19
@arch0345
Copy link
Contributor Author

Good thinking, looks good to me

data/presets/barrier/hampshire_gate.json Show resolved Hide resolved
"entrance"
],
"tags": {
"barrier": "wicket_gate"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this tag refer to the way the gate is constructed (small), its purpose (pedestrian access), or its location (secondary), or some combination? I’m finding the preset difficult to translate due to this ambiguity.

Copy link
Member

@tyrasd tyrasd Jul 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, the term refers only to a secondary gate for pedestrians near (or in1) a larger gate.

Footnotes

  1. I would say that in OSM the barrier=wicket_gate probably only applies to the situation where the wicket is actually a separate entrance (as in this example), and not for wicket doors which are built directly into a large gate (example). The latter should probably be modeled as an attribute of the main gate (e.g. barrier=gate + wicket=yes), I assume.

@arch0345 arch0345 mentioned this pull request Jul 3, 2022
@RudyTheDev
Copy link

Although barrier=log is an approved tag, I made it unsearchable to prevent mappers from adding fallen trees that don't obstruct a passage.

Perhaps it is better to rename it to something obvious like "Barrier (Fallen Tree)" rather than hide it from the search? As someone who surveys a lot of forests, this is a common item to add on paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support barrier=jersey_barrier
5 participants