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

[Unified] Waypoints of stored Labs are not made visible visible until lab cache is tapped, "visited" waypoints are not filtered out #16459

Closed
eddiemuc opened this issue Dec 28, 2024 · 6 comments
Labels
Bug Issues classified as a bug Map: Unified

Comments

@eddiemuc
Copy link
Member

c:geo-Version: 2024.12.17-RC

Tested with Lab "Zwischen Einkaufstempel und Architektur" in Hamburg near N 53° 33.053' E 009° 59.636'.
This problem might also occur with other cache types, I haven't tested this.
Problems happens on Unified Mapsforge, VTM and Google.

Precondition:

  • unified map is selected
  • map position is currently placed somewhere such that an offline-stored Lab Cache and it's waypoints are inside the viewport.
  • Some of the waypoints are marked as "visited", some are not
  • map is not currently opened (eg we might be in "stored list" section)

Error reproduction:

  • Open the map --> Lab is shown (ok) but not its waypoints (Error):
    image

  • open the popup for the lab cache:
    image

  • close popup again --> waypoints of lab are now shown...
    image

  • ...but already visited waypoints are ALSO shown allthough they should be filtered out:

image

@eddiemuc eddiemuc added Bug Issues classified as a bug Map: Unified labels Dec 28, 2024
@eddiemuc
Copy link
Member Author

@moving-bits I debugged into this and found that when initially opening the map, the only place where waypoints are loaded is the method UnifiedMapActivity.loadWaypoints(). This method however is called only once with an illegal (just-a-dot) viewport, resulting in zero waypoints being loaded.

I am not sure how the cache/waypoint loading/filtering architecture of unified map is supposed to behave here.

@eddiemuc
Copy link
Member Author

Can't extract the systematics of this out of the code. If I would need to tackle this then I would need to redesign a lot.

@bekuno
Copy link
Member

bekuno commented Dec 29, 2024

Maybe the same reason as in #16461?

@eddiemuc
Copy link
Member Author

Maybe the same reason as in #16461?

There seem to be some things intertweened here. I will take a look at #16461 too.

While testing around I noticed that each change on a cache (eg marking a waypoint as visited) causes a retriggering of the online search in live mode. This is not ideal

@moving-bits
Copy link
Member

@moving-bits I debugged into this and found that when initially opening the map, the only place where waypoints are loaded is the method UnifiedMapActivity.loadWaypoints(). This method however is called only once with an illegal (just-a-dot) viewport, resulting in zero waypoints being loaded.

I am not sure how the cache/waypoint loading/filtering architecture of unified map is supposed to behave here.

loadWaypoints() should be called in LoadInBackgroundHandler on every cycle, with the then-valid viewport.

The other two calls in reloadCachesAndWaypoints() are added to make sure waypoints are being shown independent from current viewport in case of list or searchresult.

But there haven quite a few changes over time, and there will (still) be bugs in the code for sure, so if you plan to make adjustments or even to refactor, please go ahead.

@eddiemuc
Copy link
Member Author

eddiemuc commented Jan 2, 2025

PR #16484 changes the logic so much that this issue doesn't make much sense any more, thus closing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues classified as a bug Map: Unified
Projects
Status: Done
Development

No branches or pull requests

3 participants