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

Incline quest where mtb:scale:uphill=* is used #4385

Merged
merged 13 commits into from
Sep 28, 2022

Conversation

matkoniecz
Copy link
Member

useful as otherwise you need high-quality elevation model
to do routing, and in some cases it may be unavailable at all in sufficient detail

I am writing this as I wanted to tag mtb scale info in some places to improve routing
for bicycle trekking where people like me are NOT interested at all in MTB routes
but mtb:scale=1 (or just mtb:scale=0) is still fine for them.

So mtb:scale=1 mtb:scale:uphill=3 is fine downhill but not uphill for such people,
but it is hard to check in which way given path goes uphill

Recommended by wiki: https://wiki.openstreetmap.org/w/index.php?title=Key:mtb:scale&uselang=en#mtb:scale:uphill=0-5

screen13

BTW, if someone cares/is aware about mtb tagging: comments on https://wiki.openstreetmap.org/wiki/Talk:Key:mtb:scale#Is_it_possible_to_have_mtb:scale:uphill=1_on_asphalt_road? and https://wiki.openstreetmap.org/wiki/Talk:Key:mtb:scale#Can_mtb:scale:uphill_be_lower_than_mtb:scale? are welcome as I am confused a bit by this tagging (and it seems a possible candidate for overlays)

Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

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

Looks good to me. A few remarks:

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
.filter { tagFilter.matches(it) }
.filter {
val geometry = mapData.getGeometry(it.type, it.id) as? ElementPolylinesGeometry
val overlyLong = geometry?.polylines?.filter { it.measuredLength() > OsmQuest.minLengthForMultiMarkers(false) }
Copy link
Member

Choose a reason for hiding this comment

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

This is arbitrary. Please just specify a constant here instead of referring to some unrelated function. Also, 800+ meters sounds a bit long for choosing to not display this quest for long ways.

Also, why can the user not simply split up the way in such a case?

Copy link
Member Author

@matkoniecz matkoniecz Sep 16, 2022

Choose a reason for hiding this comment

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

That is actually a related function: if there are multiple markers on a windy paths then it becomes utterly unclear which direction should be actually applied.

So the plan was to avoid any cases where multiple markers would be shown.

screen14

Also, 800+ meters sounds a bit long for choosing to not display this quest for long ways.

Though I guess that some small distance can be also used, with assumption that it will always have a single marker

Also, why can the user not simply split up the way in such a case?

It is not entirely obvious (at least I have not thought about it) and in many cases would be done solely as workaround for SC interface so probably should not be encouraged

Copy link
Member

Choose a reason for hiding this comment

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

Why would it be done as a workaround to SC interface?

Copy link
Member Author

Choose a reason for hiding this comment

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

In case where incline changes it should be split, and if parts of it are small enough this stops being relevant.

In case where way has continuous this case primary motivation for splitting way would be to get its direction displayed clearly in SC (so: workaround to SC interface)

Copy link
Member

Choose a reason for hiding this comment

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

In case where incline changes it should be split, and if parts of it are small enough this stops being relevant.

Well you have the same for any length of way.

@westnordost
Copy link
Member

So my standpoint in the unresolved comment thread is that whether it is kind of impossible to declare what is the incline of a MTB track is pretty much independent of the length of the track in OSM. The track above may just as well be split into many segments already, this doesn't mean that anything on the ground is different.

But the code I dispute does just lead to less quests being displayed, i.e. it is not a merge-blocker.

However, in cases cases were it is not really answerable (because it goes up and down and up and down...), you need to think of how to handle this case because (if that actually occurs), it would not fulfill this point from the guideline:

🤔 No unanswerable quests: All generated quests need to be actually answerable (no false-positives). This means that any answer given by the user must result in something being tagged.

I.e. for cases where the answer is something like "it does not really have a upwards or downwards incline, it's more of a bumpy ride of some kind", there should be an equivalent tagging. Maybe e.g. remove/replace the mtb:scale:uphill value with something else here?

@matkoniecz
Copy link
Member Author

Maybe e.g. remove/replace the mtb:scale:uphill value with something else here?

In such case it may not be the best idea, as there is difference between up and down hoops on compacted surface and up and down hoops on surface of loose gravel and loose big stones.

I posted some suggestions on tagging mailing list for feedback.

@matkoniecz
Copy link
Member Author

matkoniecz commented Sep 28, 2022

Done. So far I used tag with some uses (I was unaware that it exists already) and I confirmed that it is actually used in this way by contacting mappers who used it already.

There are some limited voices on tagging mailing list that it should be deprecated and replaced by some other tag: if that happens SC can be easily changed.

Co-authored-by: Tobias Zwick <newton@westnordost.de>
@westnordost westnordost merged commit 70fb442 into streetcomplete:master Sep 28, 2022
@matkoniecz matkoniecz deleted the incline branch September 28, 2022 19:08
@torhovland torhovland mentioned this pull request Oct 12, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants