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

split out escalators from AddStepsIncline quest to allow different quest wording #6135

Merged

Conversation

andrewharvey
Copy link
Contributor

At present the AddStepsIncline quest applies to both steps and escalators, however the quest wording only mentions steps. This can lead to confusion.

For example https://www.openstreetmap.org/way/1241003118 was tagged highway=steps + conveying=yes to indicate it is an escalator, however the user was then asked "Which direction leads upwards for these steps?" to which they answered via the note https://www.openstreetmap.org/note/4351347 "Not steps, escalator".

By creating a new quest we can present the wording "Which direction leads upwards for these escalators?" to reduce confusion, and also help catch mistaggings of conveying=yes/no.

#3652 requested the ability to convert between the two, but that's a different issue and this change can be applied even without giving an opportunity to answer "these are escalators" / "these are steps".

@Helium314
Copy link
Collaborator

By creating a new quest we can present the wording "Which direction leads upwards for these escalators?"

Why not just edit getTitle(tags: Map<String, String>) in the normal incline quest?

@andrewharvey
Copy link
Contributor Author

  1. Because I couldn't find that being done in any of other quests code I looked at and wanted to try and retain the code style.
  2. I don't actually know how to do it like that

@FloEdelmann
Copy link
Member

An example can seen in

override fun getTitle(tags: Map<String, String>) =
if (tags["amenity"] == "sanitary_dump_station") {
R.string.quest_generalFee_title
} else {
R.string.quest_generalFee_title2
}

Other examples (for reference)

in no particular order:

@andrewharvey
Copy link
Contributor Author

Thanks @FloEdelmann I obviously didn't check enough other quests! I can try to make the adjustment to use that style, assuming that the idea behind this PR is considered accetable?

@FloEdelmann
Copy link
Member

Yeah, I guess the wording change makes sense. However, maybe we can just rename "Which direction leads upwards for these steps?" to "Which direction leads upwards here?"? Does the name label below the question already mention "Stairs" or "Escalator"?

@riQQ
Copy link
Collaborator

riQQ commented Feb 20, 2025

Does the name label below the question already mention "Stairs" or "Escalator"?

Yes

Screenshot

@andrewharvey
Copy link
Contributor Author

Yeah, I guess the wording change makes sense. However, maybe we can just rename "Which direction leads upwards for these steps?" to "Which direction leads upwards here?"? Does the name label below the question already mention "Stairs" or "Escalator"?

I prefer to have "Steps/Stairs" and "Escalators" included in the quest title as it helps validate the data and catch any potentially mistagings. It also helps the quest mapper, user (quester? not sure what you call us solvers?) as you are more clear about exactly what the "direction leads up" is referring to.

@andrewharvey
Copy link
Contributor Author

I've refactored to use a single quest but just varying the title. I haven't tested this yet.

@westnordost
Copy link
Member

Actually, when I introduced the name label, I did that to reduce the text. Because the less text the better (as long as it remains clear). So, in general I concur with @FloEdelmann , it would be better if the wording was changed to "Which direction leads upwards here?". This then would also cover other things with an incline, such as ramps.

However, currently escalators are not labeled correctly in StreetComplete, they are labeled "steps". This is because the iD preset for escalators include a "*" value and osmfeatures, the library used to find the label for each feature, doesn't support this currently.

It's just a missing feature though, it would just need to be implemented.

When "escalator" was mentioned in the title, there'd be a mismatch with the displayed label (-> user is confused, thinks this is a bug, or wrong data), so this would be blocked by the linked issue.

However, you can apply @FloEdelmann's suggestion in any case, this is not blocked. (It should be a new key, like quest_incline_title).

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.

(see above)

@andrewharvey
Copy link
Contributor Author

Actually, when I introduced the name label, I did that to reduce the text. Because the less text the better (as long as it remains clear). So, in general I concur with @FloEdelmann , it would be better if the wording was changed to "Which direction leads upwards here?". This then would also cover other things with an incline, such as ramps.

Ah that makes sense, I can see you do this already for other quests like "What is the surface here?".

@westnordost
Copy link
Member

@goldfndr
Copy link
Contributor

Might specify "this escalator" instead of "these escalators" for the string, as more than one escalator seems unlikely to be tagged as a single object. [If they are, they're possibly perceived as a single mechanism (interconnected motors), although that doesn't seem safe (consider LOTO).]

@westnordost
Copy link
Member

Alright, so the mentioning of the escalator in the label is implemented in PR #6140. It was even much more work than I expected 😅

Would you make the change I requested?

@andrewharvey
Copy link
Contributor Author

Sorry I'm confused, if the label is fixed to display "escalator" then we don't need this PR, we can just change quest_steps_incline_title from "Which direction leads upwards for these steps?" to "Which direction leads upwards here?"

Changeset comment can probably stay the same.

@FloEdelmann
Copy link
Member

we can just change quest_steps_incline_title from "Which direction leads upwards for these steps?" to "Which direction leads upwards here?"

Exactly. You could adjust this PR to exactly this change (instead of adding another string and conditionally using that) if you want, so it can be merged. Otherwise, feel free to close it and someone else (e.g. me) could do that change instead, linking to this PR for reference.

…e quest wording is changed to apply to both steps and escalators
@andrewharvey
Copy link
Contributor Author

ok done, much simpler now.

@westnordost westnordost merged commit 1d9054c into streetcomplete:master Feb 27, 2025
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.

6 participants