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

Missing meta data / context in wheelchair quest #4448

Closed
HolgerJeromin opened this issue Oct 1, 2022 · 8 comments
Closed

Missing meta data / context in wheelchair quest #4448

HolgerJeromin opened this issue Oct 1, 2022 · 8 comments
Labels

Comments

@HolgerJeromin
Copy link
Contributor

HolgerJeromin commented Oct 1, 2022

https://www.openstreetmap.org/node/1423239695

entrance : yes
shop : estate_agent

rps20221001_181016_971

SC v48.0-alpha1

@HolgerJeromin HolgerJeromin changed the title Missing meta data / context on overlay "quest" Missing meta data / context in wheelchair quest Oct 1, 2022
@westnordost westnordost self-assigned this Oct 2, 2022
@westnordost
Copy link
Member

westnordost commented Oct 2, 2022

This is because shop=estate_agent is not recognized by the iD presets. The iD presets support the much more widely used office=estate_agent instead.

taghistory(1)

It is mentioned here, too: openstreetmap/id-tagging-schema#529

Currently, iD presets do not know shop=estate_agent at all. There is also no deprecation rule that would suggest to users to "upgrade" to office=estate_agent, probably because shop=estate_agent is not deprecated. I don't know if it is possible to add synonyms to the id presets without promoting its usage and without deprecating them. Maybe with searchable=false?

In any case, this is an upstream issue.

@westnordost westnordost closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2022
@HolgerJeromin
Copy link
Contributor Author

If SC knows nothing we should perhaps present the raw (main) tag so the user get a clue that the quest is not for drachenpoint (in the screenshot).
At least for shop=/office= elements.

@westnordost
Copy link
Member

No, not making exceptions to not confronting the users with the tags. Drachenpoint is shown as a separate node.

@HolgerJeromin
Copy link
Contributor Author

HolgerJeromin commented Oct 2, 2022

Hide the quest because: How should the user be able to answer the quest?

@westnordost
Copy link
Member

westnordost commented Oct 2, 2022

In #4241 (@Helium314 ) it was changed that this quest is also asked for nameless properties. I guess this was done under the assumption that for every feature asked, a feature name can be found through the id editor presets.

A few quests, like the opening hours quest, explicitly check for whether the object can be named and only show it if that is the case

private fun hasName(tags: Map<String, String>) = hasProperName(tags) || hasFeatureName(tags)
private fun hasProperName(tags: Map<String, String>): Boolean =
tags.containsKey("name") || tags.containsKey("brand")
private fun hasFeatureName(tags: Map<String, String>): Boolean =
featureDictionaryFuture.get().byTags(tags).isSuggestion(false).find().isNotEmpty()

...but to do that for every (shop-POI) quest would be somewhat expensive to do I think - not to mention the code duplication.

So while the current situation is not favourable (because inconsistent), i.e. some quests are shown even if the feature cannot be referred to by name and some have these checks, the better long term solution is to "simply" improve the id presets to cover more shop types rather than dealing with it in the way as cited above.
Maybe we should (just) return to checking name or brand or name:signed=no for all shop-type quests (and trusting that name:signed=no is only set if it has a feature name as the AddPlaceName quest checks for that and name:signed=no is usually only set by that quest?)

@FloEdelmann @matkoniecz

@matkoniecz
Copy link
Member

Maybe we should (just) return to checking name or brand or name:signed=no for all shop-type quests (and trusting that name:signed=no is only set if it has a feature name as the AddPlaceName quest checks for that and name:signed=no is usually only set by that quest?)

That makes sense to me.

For nameless objects answering this quest requires identifying specific objects anyway, so it means that user can answer AddPlaceName

If user failed to answer AddPlaceName and name or brand or name:signed=no is missing it means that either happened:

@FloEdelmann
Copy link
Member

Checking name or brand or name:signed=no for all shop-type quests (except AddPlaceName, where we check for a iD feature name instead) sounds like a good mitigation to me.

@westnordost
Copy link
Member

I'll reopen this ticket so this does not get lost as long as noone created a PR for this yet

@westnordost westnordost reopened this Oct 3, 2022
@westnordost westnordost removed their assignment Oct 3, 2022
westnordost added a commit that referenced this issue Oct 5, 2022
…as been specified (fixes #4448)

... or it has been determined that this place has no name (=AddPlaceName has been answered before). AddPlaceName is only asked for shops whose type can be named somehow.
westnordost added a commit that referenced this issue Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants