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

Regard country-specific presets #5969

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import de.westnordost.streetcomplete.quests.AbstractOsmQuestForm

open class TestQuestType : OsmElementQuestType<String> {

override fun isApplicableTo(element: Element): Boolean? = null
override fun isApplicableTo(element: Element, geometry: ElementGeometry): Boolean? = null
override fun applyAnswerTo(answer: String, tags: Tags, geometry: ElementGeometry, timestampEdited: Long) {}
override val icon = 0
override fun createForm(): AbstractOsmQuestForm<String> = object : AbstractOsmQuestForm<String>() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,6 @@ class NameAndLocationLabelTest {
)))
}

// https://github.com/streetcomplete/StreetComplete/issues/2640
@Ignore("https://github.com/streetcomplete/StreetComplete/issues/4916")
@Test fun postBox() {
assertEquals("Deutsche Post (Mail Drop Box)", getQuestLabelForNode(mapOf(
"amenity" to "post_box",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ interface OsmElementQuestType<T> : QuestType, ElementEditType {
* The implications of returning null here is that the quest controller needs to fetch a
* bounding box around the given element (from the database) to determine it is applicable or
* not (this is slow). */
fun isApplicableTo(element: Element): Boolean?
fun isApplicableTo(element: Element, geometry: ElementGeometry): Boolean?

/** Elements that should be highlighted on the map alongside the selected one because they
* provide context for the given element. For example, nearby benches should be shown when
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package de.westnordost.streetcomplete.data.osm.osmquests

import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry
import de.westnordost.streetcomplete.data.osm.mapdata.Element
import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry
import de.westnordost.streetcomplete.data.osm.mapdata.filter
Expand All @@ -17,5 +18,5 @@ abstract class OsmFilterQuestType<T> : OsmElementQuestType<T> {
override fun getApplicableElements(mapData: MapDataWithGeometry): Iterable<Element> =
mapData.filter(elementFilter).asIterable()

override fun isApplicableTo(element: Element) = filter.matches(element)
override fun isApplicableTo(element: Element, geometry: ElementGeometry) = filter.matches(element)
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ class OsmQuestController internal constructor(

return questTypes.map { questType ->
scope.async {
var appliesToElement = questType.isApplicableTo(element)
var appliesToElement = questType.isApplicableTo(element, geometry)
if (appliesToElement == null) {
Log.d(TAG, "${questType.name} requires surrounding map data to determine applicability to ${element.type.name}#${element.id}")
val mapData = withContext(Dispatchers.IO) { lazyMapData }
Expand Down
22 changes: 10 additions & 12 deletions app/src/main/java/de/westnordost/streetcomplete/osm/Things.kt
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private val IS_THING_EXPRESSION by lazy {
"whirlpool",

// public service
// "post_box", - blocked by https://github.com/streetcomplete/StreetComplete/issues/4916 - which is blocked by https://github.com/westnordost/osmfeatures/issues/23
"post_box",

// facilities & others
"baking_oven",
Expand All @@ -86,7 +86,7 @@ private val IS_THING_EXPRESSION by lazy {
"give_box",
"karaoke_box",
"kitchen", // usually an amenity within campsites etc, i.e. like shower, toilets, ...
// "letter_box", - see "post_box", but also, it would be very spammy to comprehensively map this
// "letter_box", - it would be very spammy to comprehensively map this
"library_dropoff",
"locker",
"lounger",
Expand Down Expand Up @@ -205,7 +205,7 @@ private val IS_THING_EXPRESSION by lazy {
"planter",
"snow_cannon",
"stele",
// "street_cabinet", - blocked; see note at amenity=post_box, most are included by dedicated filter below
"street_cabinet",
"surveillance",
// "survey_point" - this can be very very small -> verifiability issue
// danger that mapper deletes it because he can't find it
Expand Down Expand Up @@ -241,7 +241,6 @@ private val IS_THING_EXPRESSION by lazy {
or attraction
or boundary = marker
or leisure = pitch and sport ~ chess|table_soccer|table_tennis|teqball
or man_made = street_cabinet and street_cabinet != postal_service
or playground
or public_transport = platform and (
bus = yes
Expand All @@ -254,20 +253,19 @@ private val IS_THING_EXPRESSION by lazy {
}

val POPULAR_THING_FEATURE_IDS = listOf(
"natural/tree/broadleaved", // 4.0 M
"highway/street_lamp", // 4.0 M
"amenity/bench", // 2.4 M
"emergency/fire_hydrant", // 2.0 M
"natural/tree/broadleaved", // 4.8 M
"highway/street_lamp", // 4.3 M
"amenity/bench", // 2.6 M
"emergency/fire_hydrant", // 2.1 M

"amenity/waste_basket", // 0.7 M
"amenity/bicycle_parking", // 0.6 M
"amenity/waste_basket", // 0.9 M
"amenity/bicycle_parking", // 0.7 M
"amenity/shelter", // 0.5 M

"amenity/recycling_container", // 0.4 M
"amenity/toilets", // 0.4 M

// "amenity/post_box", // 0.4 M
// blocked by https://github.com/streetcomplete/StreetComplete/issues/4916
"amenity/post_box", // 0.4 M

// More:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ abstract class AbstractOverlayForm :
binding.speechbubbleContentContainer.clipToOutline = true

setTitleHintLabel(
element?.let { getNameAndLocationSpanned(it, resources, featureDictionary) }
element?.let { getNameAndLocationSpanned(it, resources, featureDictionary, countryOrSubdivisionCode) }
)
setObjNote(element?.tags?.get("note"))

Expand Down Expand Up @@ -415,7 +415,7 @@ abstract class AbstractOverlayForm :

protected fun composeNote(element: Element) {
val overlayTitle = englishResources.getString(overlay.title)
val hintLabel = getNameAndLocationSpanned(element, englishResources, featureDictionary)
val hintLabel = getNameAndLocationSpanned(element, englishResources, featureDictionary, countryOrSubdivisionCode)
val leaveNoteContext = if (hintLabel.isNullOrBlank()) {
"In context of overlay \"$overlayTitle\""
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ val overlaysModule = module {
val countryBoundaries = get<Lazy<CountryBoundaries>>(named("CountryBoundariesLazy")).value
countryBoundaries.getIds(location).firstOrNull()
},
{ element ->
get<Lazy<FeatureDictionary>>(named("FeatureDictionaryLazy")).value.getFeature(element)
{ element, location ->
val countryBoundaries = get<Lazy<CountryBoundaries>>(named("CountryBoundariesLazy")).value
val featureDictionary = get<Lazy<FeatureDictionary>>(named("FeatureDictionaryLazy")).value
val country = location?.let { countryBoundaries.getIds(it).firstOrNull() }
featureDictionary.getFeature(element, country)
}
)
}
Expand All @@ -47,7 +50,7 @@ val overlaysModule = module {
fun overlaysRegistry(
getCountryInfoByLocation: (LatLon) -> CountryInfo,
getCountryCodeByLocation: (LatLon) -> String?,
getFeature: (Element) -> Feature?,
getFeature: (Element, LatLon?) -> Feature?,
) = OverlayRegistry(listOf(

0 to WayLitOverlay(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ class AddressOverlayForm : AbstractOverlayForm(), IsMapPositionAware {

if (element != null) {
setTitleHintLabel(getNameAndLocationSpanned(
element, resources, featureDictionary,
element, resources, featureDictionary, countryOrSubdivisionCode,
showHouseNumber = false
))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class BuildingsOverlayForm : AGroupedImageSelectOverlayForm<BuildingType>() {
originalBuilding = createBuildingType(element!!.tags)
selectedItem = originalBuilding?.asItem()

setTitleHintLabel(getNameAndLocationSpanned(element!!, resources, null, true))
setTitleHintLabel(getNameAndLocationSpanned(element!!, resources, null, null, true))
}

override fun hasChanges(): Boolean =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package de.westnordost.streetcomplete.overlays.places
import de.westnordost.osmfeatures.Feature
import de.westnordost.streetcomplete.R
import de.westnordost.streetcomplete.data.osm.mapdata.Element
import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry
import de.westnordost.streetcomplete.data.osm.mapdata.Node
import de.westnordost.streetcomplete.data.osm.mapdata.filter
Expand All @@ -18,7 +19,7 @@ import de.westnordost.streetcomplete.quests.shop_type.SpecifyShopType
import de.westnordost.streetcomplete.util.getNameLabel
import de.westnordost.streetcomplete.view.presetIconIndex

class PlacesOverlay(private val getFeature: (Element) -> Feature?) : Overlay {
class PlacesOverlay(private val getFeature: (Element, LatLon?) -> Feature?) : Overlay {

override val title = R.string.overlay_places
override val icon = R.drawable.ic_quest_shop
Expand All @@ -37,7 +38,8 @@ class PlacesOverlay(private val getFeature: (Element) -> Feature?) : Overlay {
.asSequence()
.filter { it.isPlaceOrDisusedPlace() }
.map { element ->
val feature = getFeature(element)
val position = mapData.getGeometry(element.type, element.id)?.center
val feature = getFeature(element, position)

val icon = feature?.icon?.let { presetIconIndex[it] } ?: R.drawable.ic_preset_maki_shop
val label = getNameLabel(element.tags)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package de.westnordost.streetcomplete.overlays.things
import de.westnordost.osmfeatures.Feature
import de.westnordost.streetcomplete.R
import de.westnordost.streetcomplete.data.osm.mapdata.Element
import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry
import de.westnordost.streetcomplete.data.osm.mapdata.Node
import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement
Expand All @@ -15,7 +16,7 @@ import de.westnordost.streetcomplete.overlays.PointStyle
import de.westnordost.streetcomplete.overlays.PolygonStyle
import de.westnordost.streetcomplete.view.presetIconIndex

class ThingsOverlay(private val getFeature: (Element) -> Feature?) : Overlay {
class ThingsOverlay(private val getFeature: (Element, LatLon?) -> Feature?) : Overlay {

override val title = R.string.overlay_things
override val icon = R.drawable.ic_quest_dot
Expand All @@ -29,8 +30,9 @@ class ThingsOverlay(private val getFeature: (Element) -> Feature?) : Overlay {
.asSequence()
.filter { it.isThing() || it.isDisusedThing() }
.mapNotNull { element ->
val feature = getFeature(element)
?: element.asIfItWasnt("disused")?.let { getFeature(it) }
val position = mapData.getGeometry(element.type, element.id)?.center
val feature = getFeature(element, position)
?: element.asIfItWasnt("disused")?.let { getFeature(it, position) }
?: return@mapNotNull null

val icon = feature.icon?.let { presetIconIndex[it] } ?: R.drawable.ic_preset_maki_marker_stroked
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class ThingsOverlayForm : AbstractOverlayForm() {
super.onViewCreated(view, savedInstanceState)

// title hint label with name is a duplication, it is displayed in the UI already
setTitleHintLabel(element?.let { getNameAndLocationSpanned(it, resources, null) })
setTitleHintLabel(element?.let { getNameAndLocationSpanned(it, resources, null, null) })
setMarkerIcon(R.drawable.ic_quest_dot)

featureCtrl = FeatureViewController(featureDictionary, binding.featureTextView, binding.featureIconView)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ abstract class AbstractOsmQuestForm<T> : AbstractQuestForm(), IsShowingQuestDeta
super.onViewCreated(view, savedInstanceState)

setTitle(getString(osmElementQuestType.getTitle(element.tags)))
setTitleHintLabel(getNameAndLocationSpanned(element, resources, featureDictionary))
setTitleHintLabel(getNameAndLocationSpanned(element, resources, featureDictionary, countryOrSubdivisionCode))
setObjNote(element.tags["note"])
}

Expand Down Expand Up @@ -237,7 +237,7 @@ abstract class AbstractOsmQuestForm<T> : AbstractQuestForm(), IsShowingQuestDeta
protected fun composeNote() {

val questTitle = englishResources.getString(osmElementQuestType.getTitle(element.tags))
val hintLabel = getNameAndLocationSpanned(element, englishResources, featureDictionary)
val hintLabel = getNameAndLocationSpanned(element, englishResources, featureDictionary, countryOrSubdivisionCode)
val leaveNoteContext = if (hintLabel.isNullOrBlank()) {
"Unable to answer \"$questTitle\""
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ import de.westnordost.streetcomplete.quests.width.AddCyclewayWidth
import de.westnordost.streetcomplete.quests.width.AddRoadWidth
import de.westnordost.streetcomplete.screens.measure.ArSupportChecker
import de.westnordost.streetcomplete.util.ktx.getFeature
import de.westnordost.streetcomplete.util.ktx.getIds
import org.koin.core.qualifier.named
import org.koin.dsl.module

Expand All @@ -193,8 +194,11 @@ val questsModule = module {
val countryBoundaries = get<Lazy<CountryBoundaries>>(named("CountryBoundariesLazy")).value
countryInfos.getByLocation(countryBoundaries, location.longitude, location.latitude)
},
{ element ->
get<Lazy<FeatureDictionary>>(named("FeatureDictionaryLazy")).value.getFeature(element)
{ element, location ->
val countryBoundaries = get<Lazy<CountryBoundaries>>(named("CountryBoundariesLazy")).value
val featureDictionary = get<Lazy<FeatureDictionary>>(named("FeatureDictionaryLazy")).value
val country = location?.let { countryBoundaries.getIds(it).firstOrNull() }
featureDictionary.getFeature(element, country)
}
)
}
Expand All @@ -203,7 +207,7 @@ val questsModule = module {
fun questTypeRegistry(
arSupportChecker: ArSupportChecker,
getCountryInfoByLocation: (LatLon) -> CountryInfo,
getFeature: (Element) -> Feature?,
getFeature: (Element, LatLon?) -> Feature?,
) = QuestTypeRegistry(listOf(

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class AddAddressStreet : OsmElementQuestType<StreetOrPlaceName> {
}

// cannot be determined because of the associated street relations
override fun isApplicableTo(element: Element): Boolean? =
override fun isApplicableTo(element: Element, geometry: ElementGeometry): Boolean? =
if (!filter.matches(element)) false else null

override fun getHighlightedElements(element: Element, getMapData: () -> MapDataWithGeometry) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class AddAddressStreetForm : AbstractOsmQuestForm<StreetOrPlaceName>() {
super.onViewCreated(view, savedInstanceState)

setTitleHintLabel(getNameAndLocationSpanned(
element, resources, featureDictionary,
element, resources, featureDictionary, countryOrSubdivisionCode,
showHouseNumber = true
))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ class AddHousenumber : OsmElementQuestType<HouseNumberAnswer> {
return buildings
}

override fun isApplicableTo(element: Element): Boolean? =
override fun isApplicableTo(element: Element, geometry: ElementGeometry): Boolean? =
if (!buildingsWithMissingAddressFilter.matches(element)) false else null

override fun getHighlightedElements(element: Element, getMapData: () -> MapDataWithGeometry) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import de.westnordost.streetcomplete.R
import de.westnordost.streetcomplete.data.elementfilter.toElementFilterExpression
import de.westnordost.streetcomplete.data.osm.geometry.ElementGeometry
import de.westnordost.streetcomplete.data.osm.mapdata.Element
import de.westnordost.streetcomplete.data.osm.mapdata.LatLon
import de.westnordost.streetcomplete.data.osm.mapdata.MapDataWithGeometry
import de.westnordost.streetcomplete.data.osm.osmquests.OsmElementQuestType
import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement.OUTDOORS
Expand All @@ -14,7 +15,7 @@ import de.westnordost.streetcomplete.util.ktx.containsAll
import de.westnordost.streetcomplete.util.ktx.toYesNo

class AddAmenityCover(
private val getFeature: (Element) -> Feature?
private val getFeature: (Element, LatLon?) -> Feature?
) : OsmElementQuestType<Boolean> {

private val nodesFilter by lazy { """
Expand All @@ -34,16 +35,21 @@ class AddAmenityCover(
override fun getTitle(tags: Map<String, String>) = R.string.quest_amenityCover_title

override fun getApplicableElements(mapData: MapDataWithGeometry): Iterable<Element> =
mapData.filter { isApplicableTo(it) }
mapData.filter { element ->
val geometry = mapData.getGeometry(element.type, element.id) ?: return@filter false
isApplicableTo(element, geometry)
}

override fun isApplicableTo(element: Element) =
nodesFilter.matches(element) && getFeature(element) != null
override fun isApplicableTo(element: Element, geometry: ElementGeometry) =
nodesFilter.matches(element) && getFeature(element, geometry.center) != null

override fun getHighlightedElements(element: Element, getMapData: () -> MapDataWithGeometry): Sequence<Element> {
/* put markers for objects that are exactly the same as for which this quest is asking for
e.g. it's a ticket validator? -> display other ticket validators. Etc. */
val feature = getFeature(element) ?: return emptySequence()
return getMapData().filter { it.tags.containsAll(feature.tags) }.asSequence()
val mapData = getMapData()
val position = mapData.getGeometry(element.type, element.id)?.center
val feature = getFeature(element, position) ?: return emptySequence()
return mapData.filter { it.tags.containsAll(feature.tags) }.asSequence()
}

override fun createForm() = YesNoQuestForm()
Expand Down
Loading