Skip to content

Conversation

@matkoniecz
Copy link
Member

It is an attempt to add new quest type. It would take data from https://github.com/alltheplaces/alltheplaces that were processed with https://codeberg.org/matkoniecz/list_how_openstreetmap_can_be_improved_with_alltheplaces_data

you can see processed data at https://matkoniecz.codeberg.page/improving_openstreetmap_using_alltheplaces_dataset/


AllThePlaces POI quest would show to users as "here supposedly POI exists here - can you confirm its existence and specify an exact location?"

Also, it enables improvements to opening hours quest - to detect when OSM data and AllThePlaces data mismatches and to produce quest, before POI age crosses current threshold.
One more possible feature is to even prefill opening hours quest (if no data was collected yet) by AllThePlaces opening hours data (this one is not implemented yet at all).

Relevant discussions include https://community.openstreetmap.org/t/improving-openstreetmap-shop-coverage-with-alltheplaces/119979 and https://community.openstreetmap.org/t/what-you-think-about-importing-opening-hours-data-from-alltheplaces/120608 and https://osmfoundation.org/wiki/Licensing_Working_Group/Minutes/2023-08-14#Ticket#2023081110000064_%E2%80%94_First_party_websites_as_sources and alltheplaces/alltheplaces#5133 and alltheplaces/alltheplaces#8790

Would potentially open way for #5481 (but review of licensing is blocker for now anyway there) and similar ones, but I have no specific plans to work on either for now.

Review of specific spiders like done at https://community.openstreetmap.org/t/what-you-think-about-importing-opening-hours-data-from-alltheplaces/120608/72 could be helpful if anyone is interested. (it is stored in https://codeberg.org/matkoniecz/list_how_openstreetmap_can_be_improved_with_alltheplaces_data/src/branch/master/reviewed_data_sources.py )

If you see bogus data at https://matkoniecz.codeberg.page/improving_openstreetmap_using_alltheplaces_dataset/ reporting it also would be helpful. Probably in https://community.openstreetmap.org/t/improving-openstreetmap-shop-coverage-with-alltheplaces/119979 or https://codeberg.org/matkoniecz/list_how_openstreetmap_can_be_improved_with_alltheplaces_data/issues (@westnordost probably prefers to not get here notifications about all bugs in ATP or my code). Also, if someone is interested in their area but spiders are all disabled now due to lacking review: let me know, I can review at least some.

Helium314#523 was another attempt to do this, alltheplaces/alltheplaces#6787 was discussing making such ATP-OSM comparison a part of the ATP project. Both seem dead as of now.


I started building infrastructure for handling data from new API in SC, mostly copying how note handling is done (all this AtpDao, AtpTable, AtpApiClient)

I am now loooking through OsmQuestController/OsmNoteQuestController and I initially started building AtpQuestController.

So far all code is general, littered with TODOs and there is still ATP-specific code to be actually written. And current code is nonfunctional, quest is not even yet shown on the map. AI is not called, API response is for now hardcoded.

But maybe someone can take a look whether general layout makes sense?

If there is some serious general structure issue then maybe pointing it now would be nice. But I am also entirely fine with people waiting until proper code exists.


Disclaimer: my work on ATP-OSM matching to enable this quest and quest itself is sponsored by European Union via NGI0 via NLnet foundation

Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

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

Okay, let's focus on that add All the places-place quest first. Maybe you should just delete the opening hours one for now, to keep the size of the PR managable. It seems to be a stub right now anyway. The PR will be large in any case.

So, you want to introduce a new quest type, one that is not generated from OSM data. For this:

  • AtpDownloader hooks into Downloader

  • AtpDownloader tells AtpController to persist (update/add/delete) the data

  • The responsibility of AtpController is to wrap AtpDao and provide the ability to listen to updates of the data.

  • AtpDao's responsibility is to load/save the ATP data from the database

  • Now, you need a AtpQuestController. It listens to updates from AtpController and MapDataWithEditsSource and after whatever logic you need to produce quests from these two data sources, in turn creates quests of type AtpQuestType.

  • VisibleQuestsSource will listen to updates from AtpQuestController to include those quests for display

  • AtpQuestType inherits from QuestType and ElementEditType, but is not simply a OsmElementQuestType which makes this all so complex. It cannot be a OsmElementQuestType because it doesn't work on OSM elements (it works on the absence of OSM elements. It will be an object, just like the NoteQuestType. There is just one AtpQuestType, after all.

  • You need a AtpQuestForm in which whatever data needs to be recorded is recorded. On quest completion, if I understood this correctly, it will create a new OSM element. So, the UI will then create a new ElementEdit at the ElementEditsController.

  • The addition of a new ElementEdit to ElementEditsController will then automatically notify MapDataWithEditsSource of the update, thus triggering an update to AtpQuestController too. Since the AtpQuestController checks if there is a (similarily named) POI already a the location of the Atp POI, it will tell its listeners (i.e. VisibleQuestsSource) that this one quest (the user just solved) has been removed. That is how it works.

Now, regarding AtpQuestController, you might have noticed that OsmNoteQuestController and OsmQuestController look quite different. The former has no backing DAO in which note quests are persisted. This is because the quest creation logic for OsmNoteQuestController is relatively simple, so it basically just transforms the incoming data (notes) to note quests.
OsmQuestController persist the quests instead because the logic is much more involved (all this quest filter stuff) and thus also has to manage adding/removing quests based on the incoming data.
I can't say which of the two approaches would be more eligible for Atp, but I'd suggest you try the notes approach and see if the performance will be adequate.


That opening hours quest by the way doesn't need all this. It would (probably) be a normal OsmElementQuestType which just additional taps into the Atp data in the same vein as the old now-deleted improveOsm oneway quest did.
So, it would still need the downloader, the controller and the dao.

Comment on lines 25 to 32
/* See also AddWheelchairAccessBusiness and AddPlaceName, which has a similar list and is/should
be ordered in the same way for better overview */
private val filter by lazy { ("""
Copy link
Member

Choose a reason for hiding this comment

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

Did you copy-pasta that from the opening hours quest? That's of course not acceptable, if it is 100% shared, it should be defined in a shared location.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, there should be TODO for it somewhere (added now). But for now I am still unsure should it be a separate quest or added to main opening hours quest.

So have not tried to deduplicate content yet.

@matkoniecz

This comment was marked as resolved.

@westnordost

This comment was marked as resolved.

@matkoniecz
Copy link
Member Author

matkoniecz commented Aug 30, 2025

@westnordost

Is it expected that app/src/androidInstrumentedTest/kotlin/de/westnordost/streetcomplete/data/atp/AtpDaoTest.kt will have no access to helper function defined in app/src/commonTest/kotlin/de/westnordost/streetcomplete/testutils/TestDataShortcuts.kt ?

Due to being in a different subfolder of src ?

(I am irritated by fun createAtpEntry and fun atpEntry - tried to deduplicate but failed so far)

@westnordost
Copy link
Member

westnordost commented Aug 30, 2025 via email

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.

2 participants