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

Replay edits done during quests download #5473

Merged
merged 4 commits into from
Feb 5, 2024
Merged

Replay edits done during quests download #5473

merged 4 commits into from
Feb 5, 2024

Conversation

westnordost
Copy link
Member

@westnordost westnordost commented Feb 3, 2024

fixes #4258

This actually does not only concern quests, but also overlays, which is why the solution has been done in MapDataWithEditsSource rather than in OsmQuestController.

Rather than making it really complicated for edge cases to deal with possible parallelism in onReplacedForBBox, I made it so that it cannot be executed in parallel. This doesn't change anything for the current configuration because OSM downloads can not happen in parallel (and it is very unlikely that this is ever changed).


Note that while testing this solution, I noticed that I could not reproduce the issue in the first place. This was because Listeners.kt replaced the previous CopyOnWriteArrayList-based listeners and Listeners just synchronizes all access:

I.e. any Listeners.forEach(action) was synchronized, meaning that a parallel
listeners.forEach { it.onReplacedForBBox(bbox, mapDataWithGeometry) }
after map data download which triggers creating anew the quests for the downloaded map data and
listeners.forEach { it.onUpdated(updated, deleted) }
after a single update of the data via an edit which triggers checking quests for just that one quest was not possible anymore. They were always executed in order.

So, when trying to solve a quest while the osm data is being updated (=the quests are created), the UI would just seemingly freeze. Argh! The very thing I wanted to avoid. I fixed this now, I hope my fix is fine. Second or third pair of eyes appreciated.

…all onReplacedForBBox to listeners completed for updates that happened while the latter was being executed (fixes #4258)
@Helium314
Copy link
Collaborator

Looks good to me.
The only very minor things I noticed is that isReplacingForBBoxLock and updatesWhileReplacingBBox could be vals

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.

Quest is asked again immediately after solving it
2 participants