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

Add housename to street quest #3457

Merged
merged 3 commits into from Nov 11, 2021
Merged

Add housename to street quest #3457

merged 3 commits into from Nov 11, 2021

Conversation

arrival-spring
Copy link
Contributor

I can't see that this has been discussed anywhere, I did see #213 (comment)

  • research what should be done with house names (exclude addr:housename?) - no, if addr:streetnumber is present then addr:housename also present is not changing anything at all

But this seems to be about excluding objects with both housenumber and housename

At least in the UK, houses with housename belong to a street/place in the same way that houses with housenumber do, so it feels odd when answering quests that only with housenumber do you get the street quest after answering.

I don't know if that's different in different parts of the world though.

(This is untested)

Copy link
Member

@matkoniecz matkoniecz left a comment

Choose a reason for hiding this comment

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

At least following changes are needed:

(1)
Quest title should change depending whether housename or housenumber is present

Right now quest_address_street_title is always used, for "What street does the (house) number %s belong to?"

See tram/bus stop quests, for example

override fun getTitle(tags: Map<String, String>): Int {
val hasName = tags.containsAnyKey("name", "ref")
val isTram = tags["tram"] == "yes"
return when {
isTram && hasName -> R.string.quest_busStopShelter_tram_name_title
isTram -> R.string.quest_busStopShelter_tram_title
hasName -> R.string.quest_busStopShelter_name_title
else -> R.string.quest_busStopShelter_title
}
}

(2)

arrayOfNotNull(tags["addr:streetnumber"] ?: tags["addr:housenumber"])
also needs to be modified

@arrival-spring
Copy link
Contributor Author

Is it preferable to introduce a new string "What street does the (house) %s belong to?" (for housename) or simply change the string to that for the whole quest, regardless of housename or housenumber.

e.g. one would see "What street does the (house) 12 belong to?"

@westnordost
Copy link
Member

If a string can be found that fits for both, that's best. But your suggestion doesn't sound correct - "what does the 12 belong to"

@arrival-spring
Copy link
Contributor Author

Hmm, yes. In fairness, "What street does the number 12 belong to" doesn't quite flow either.

How about just "What street does (house) %s belong to?"

@smichel17
Copy link
Member

What street does (house) %s belong to?

This sounds good. I might use building instead of house, because I think this also applies to things like apartment buildings.

So it is either,

What street does building 12 belong to?

or

What street does Wayne Manor belong to?

What about cases when there is both a number and a name? Ideally it would show both. Something like,

What street does building 12 (Wayne Manor) belong to?

@arrival-spring
Copy link
Contributor Author

This sounds good. I might use building instead of house, because I think this also applies to things like apartment buildings.

The problem with that is this quest also asks about nodes, so may be asking about e.g. entrances (this same criticism applies to using house).

@westnordost
Copy link
Member

@arrival-spring let's use

<string name="quest_address_street_title_housename">What street does (house) %s belong to?</string>

then

@arrival-spring
Copy link
Contributor Author

There we go, I think that's everything, I haven't made it a new string as it's a minor change (in terms of translations).

Works as expected.

When an object has both housename and housenumber tagged it only states the number in the title (which seems fine), but in the subtitle above it the housename is shown (this was also true before these changes). Not sure if that's an issue though.

@westnordost
Copy link
Member

@arrival-spring I actually meant adding another string. So there are two strings, one for number, one for name.

I guess ... your current solution would be fine for me too.

@arrival-spring
Copy link
Contributor Author

Ah yes, I now see that, I'm happy either way

Copy link
Member

@matkoniecz matkoniecz left a comment

Choose a reason for hiding this comment

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

Have not tested the code (has anyone tested?) but issues that I noticed are now fixed.

EDIT: "Works as expected." - so testing was done.

@westnordost
Copy link
Member

Would you mark it as approve then? I will only merge it later (goes into next major).

Copy link
Member

@matkoniecz matkoniecz left a comment

Choose a reason for hiding this comment

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

erasing "request changes" state, though I had not done testing

@arrival-spring
Copy link
Contributor Author

I tested with these same changes made locally and it seemed to work as expected, looking at objects with housename and housenumber and with just housename.

@westnordost westnordost merged commit 14fecde into streetcomplete:master Nov 11, 2021
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.

None yet

4 participants