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

display operator for collection times of this postbox quest #1473

Closed
matkoniecz opened this issue Jul 7, 2019 · 6 comments · Fixed by #1484
Closed

display operator for collection times of this postbox quest #1473

matkoniecz opened this issue Jul 7, 2019 · 6 comments · Fixed by #1484

Comments

@matkoniecz
Copy link
Member

matkoniecz commented Jul 7, 2019

See https://www.openstreetmap.org/note/1833373 where user was unable to solve quest as postboxes were not identified - note that https://www.openstreetmap.org/node/3705957048 and https://www.openstreetmap.org/node/3705957049 are mapped separately, each tagged with operator.

@BjornRasmussen
Copy link

I've also had to leave a nearly identical note because of this... A fedex and ups post box were right next to each other, and I wasn't sure which one the quest was about.

@matkoniecz
Copy link
Member Author

matkoniecz commented Jul 7, 2019

(1) getLevelLabelText may be made more general or (2) in some cases extra tags, not only name may be used for name %s name formatting of the quest title in getElementName.

@westnordost
Copy link
Member

Yes indeed implementation-wise it is a challenge how to do this cleanly.

@goldfndr
Copy link
Contributor

goldfndr commented Jul 8, 2019

I'm confused. Why wouldn't it be as simple as changing getTitle like is done for add bus stop shelter? In english, the brand is usable as an adjective, I'd expect the same for operator for postboxes.

@matkoniecz
Copy link
Member Author

That is only one part, it selects version of title with space for name (%s) - see strings in https://github.com/westnordost/StreetComplete/blob/master/app/src/main/res/values/strings.xml#L457-L458

There is %s that will be replaced by tag value.

And in https://github.com/westnordost/StreetComplete/blob/a137ccc6c04dbcd60386c1556650f5de7b60226f/app/src/main/java/de/westnordost/streetcomplete/quests/QuestUtil.kt#L25 %s is replaced by tag value.

In case of bus/tram stops only selection of title with space for name and nameless variant was needed as bus stops are fine with using name tag.

The tricky part is that name is always name, brand if tagged may be treated as name but operator is equivalent as a name only for some objects like post boxes (not for shops, roads, schools and many more).


Though on a second thought:

  • getElementName gives priority to name tag
  • so for example bus/tram stop with both name and brand tag will use value of name for the title
  • but bus/tram stop with only brand tag will get selected version of title without name so operator will not be displayed.
  • the same will happen once operator will be also a candidate - either nameless title variant will be used, or name will have higher priority and operator will not be used anyway.

And for post box it does not matter which value of name, brand, operator will be used.

Overall, no architectural changes appear to be needed. Thanks for being a rubber duck!

@matkoniecz
Copy link
Member Author

And once I started coding I found obvious mistake - what about a name quest for school that has operator tag?

It should specify object as its type (given by featureDictionaryFuture?.get()?.let { it.byTags(tags).forLocale(locale).find()?.firstOrNull()?.name } - for amenity=school it give "School grounds", not operator tag).

For postbox with operator tag it should give value of operator tag, not its type (postbox).

matkoniecz added a commit to matkoniecz/Zazolc that referenced this issue Jul 8, 2019
also, allow each quest to give its own list of name giving tags

fixes streetcomplete#1473
matkoniecz added a commit to matkoniecz/Zazolc that referenced this issue Jul 13, 2019
also, allow each quest to define its own title replacement(s)
(for now only the first one will be used, as there is no
quest at this moment that needs to use multiple ones)

fixes streetcomplete#1473, closes streetcomplete#1474
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants