handle operator as name for postboxes

[matkoniecz]

also, allow each quest to give its own list of name giving tags

fixes #1473

---

I also thought about adding this data to rather to OsmElementQuestType but it would require type checking on use, as getQuestTitle has parameter of QuestType<*> type.

I tested that it works on name quest for school with operator tag (no displayed), post box with operator tag (displayed), post box, without operator tag, note quest, one way quest, road name quest.

---

My work on this pull request and UX testing was sponsored by a NGI Zero Discovery grant

[matkoniecz]

I looked though other quests, currently there is nothing else that would benefit from a similar change.

[westnordost]

That is a good solution to the problem. However, I want to propose a slightly different one and pitch it to discussion. Not sure myself if it is superior:

What about if the QuestType does not specify the "name giving tags", but actually specifies the replacements itself. For example:

override fun getTitle(tags: Map<String, String>) =
    R.string.bla_title // "Does the road %s really have %d lanes?"

override fun getTitleReplacements(tags: Map<String, String>) =
    arrayOf(tags["name"] ?: tags["ref"], tags["lanes"])

The advantage is that it is more powerful (as shown - more replacements possible, also non-name-related replacements possible) and that the control over what title is shown is exactly where it should be - in the hands of the quest itself. The default behavior (use name, otherwise brand, otherwise,...) would be defined in the QuestType from which all inherit.

The downside is, that the example above does not include a solution how to fall back to the feature name. On the other hand, the fallback is only appropriate in 1 quest. (Maybe pass a future containing the feature name into the function?) Also, it is the question if this amount of control is actually necessary because for the popular case of...

• "Is this bus stop covered?"
• "Is this tram stop covered?"
• etc

... it can not be used, because the article (and more?) will differ in other languages than English if the word has a different grammatical gender.

[matkoniecz]

However, I want to propose a slightly different one and pitch it to discussion. Not sure myself if it is superior:

I really like this solution and I plan to implement it. But I admit that it is primarily because it would allow me to resurrect "show objects with fixme tags" feature in my fork, without diverging code from SC.

So basically for

The advantage is that it is more powerful

reason.

[BjornRasmussen]

I'm not sure if you are already planning this, but I think that using brand would be better than operator.
Would it be possible for SC to

1. check for brand=* and use it if it exists.
2. if not, check for operator=* and use it if it exists.
3. if not, use generic question.

[matkoniecz]

brand tag is already used and was used before this PR - see files changed tab with https://github.com/westnordost/StreetComplete/pull/1474/files

val nameGivingTags: List get() = listOf("name", "brand")
tags["brand"] ?:

[westnordost]

But I admit that it is primarily because it would allow me to resurrect "show objects with fixme tags" feature

How so?

[matkoniecz]

override fun getTitle(tags: Map<String, String>) =
    R.string.fixme_template // "fixme=%s"

override fun getTitleReplacements(tags: Map<String, String>) =
    arrayOf(tags["fixme"] ?: tags["FIXME"])

query would find all with fixme=* or FIXME=*

EDIT: Though method in the current PR would also work, just pass fixme, FIXME array.

[westnordost]

Wouldn't make including the fixme text into the question bubble not make it (potentially) very very long? (But anyway, this is something for Zazolc only anyway so +shrug+)

[matkoniecz]

Wouldn't make including the fixme text into the question bubble not make it (potentially) very very long?

Maybe, but most are short - and anyway primary use is "hey, it is useful to open Vespucci here".

[matkoniecz]

override fun getTitle(tags: Map<String, String>) =
    R.string.bla_title // "Does the road %s really have %d lanes?"

override fun getTitleReplacements(tags: Map<String, String>) =
    arrayOf(tags["name"] ?: tags["ref"], tags["lanes"])

I started implementing this, but my first though was - is there any reason to split it in a two functions at this point? Maybe allowing to simply return an arbitrary string would be better in case of going for a powerful and flexible title generation.

[westnordost]

The reason is that the quest should not have a dependency on Context or Resources, but that would be necessary to construct the string itself.

[matkoniecz]

Maybe pass a future containing the feature name into the function?

I got stuck on this one, https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.native.concurrent/-future/index.html is not too informative and searching for Future in existing code, searching on Internet for resources with kotlin future/kotlin future concurrency/kotlin future concurrency usage -coroutines -java/kotlin concurrency failed.

I will look at it again, but in case that someone can recommend some resources (maybe I missed something that would be useful) - I would be happy to get recommendations.

I now want to get back to checking why regression fixing PR is actually not fixing anything.

[westnordost]

Actually, more specifically I meant to propose to use a Lazy, sorry for that: A wrapper around a result that only computes its result once when it is actually queried. FYI:
A Future is a wrapper around an asynchronously computed result (so will eventually have a result)
A Provider creates new objects of a given type on-demand

override fun getTitleReplacements(tags: Map<String, String>, featureName: Lazy<String>) =
    arrayOf(tags["name"] ?: tags["brand"] ?: featureName.get())

//...

getTitleReplacements(someTags, lazy { /* query the feature dictionary for the name here */ } )

The reason why one would not simply pass the featureName directly as a String is that this would mean that every time, the feature dictionary is queried for the feature name even if the String is not actually used.