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

Quest is asked again immediately after solving it #4258

Closed
mnalis opened this issue Jul 31, 2022 · 31 comments · Fixed by #5473
Closed

Quest is asked again immediately after solving it #4258

mnalis opened this issue Jul 31, 2022 · 31 comments · Fixed by #5473
Assignees
Labels

Comments

@mnalis
Copy link
Member

mnalis commented Jul 31, 2022

This is again reproduced #4124 (and before that, #4086). See them for more details and history of the problem.

I've managed to do both things requested in #4124 (comment):

  • try with 45.x version
  • verify history that edits were made

How to Reproduce

  • answered the cycleway quest with no at 2022-07-29 about 14:04 CEST (or minute before).
  • noticed at 14:04 CEST that I'm asked if the situation is still the same
  • started screen recording
  • verified in the undo menu that there is recorded answer Added cycleway:right=no
  • answered no to the is this still the cycleway situation here?
  • answered displayed separately on the map to the is this still the cycleway situation here?
    (note that this answer does not match reality, but I choose different answer than no to be sure to get multiple changes in changeset, so I can prove the bug is not only in my mind 😄)
  • verified in the undo menu that the new answer generated Updated to cycleway:right=separate
  • verified in the undo menu that there is also still recorded old answer Added cycleway:right=no
  • stopped screen recording
  • started uploading all quests immediately after the video ends (i.e. 14:05/14:06 CEST)

Video:

small_SVID_20220729_140434_1.mp4

I expect it is this changeset: https://www.openstreetmap.org/changeset/124228420#map=18/46.41630/16.69096
and this way specifically:
https://www.openstreetmap.org/way/670948488/history (v8 and v9)

Expected Behavior

  • is this still the cycleway situation here? quest should not have triggered, as the cycleway quest was answered seconds (and not months/years) ago.

Versions affected

45.0-based (my fork), Android 10 (on Huawei P30 Pro).
But note previous repetition of bug that looked exactly the same was in vanilla mainstream SC (not in my fork), so I hope the bug is the same, and so that this bugreport still helps.

@mnalis mnalis added the bug label Jul 31, 2022
@mnalis
Copy link
Member Author

mnalis commented Jul 31, 2022

I've also looked at keepright, osmose, and OSMI to check if there are duplicate/invalid OSM data, but it does not seem so (only warnings are minor and seemingly unrelated, e.g. "no maxspeed mapped on this road" or "nearby building should have address related to this street")

@mnalis
Copy link
Member Author

mnalis commented Jul 31, 2022

One additional minor thing that I've noticed (which I don't know if it is related - and I don't recall seeing it in previous manifestations of this bug) after videorecording this bug is that pressing GPS button in SC stopped working - i.e. it did not position map to current location, just acted as dummy button. At the same time GPS seemed to work fine, and doing same action in OsmAnd positioned the map there without any problems.

I have video of that too if you find it might be related (or if you find it unrelated, but want it as a separate bug report anyway).

@westnordost westnordost self-assigned this Aug 3, 2022
@westnordost
Copy link
Member

I looked closer through this issue, but could not find anything. I tried to reproduce the issue with that exact same cycleway. No result, wasn't able to.

I noticed that this and the last issue was always about the cycleway quest only. So maybe it is an issue in the isApplicableTo method in AddCycleway. In particular, if hasOldInvalidOrAmbiguousCyclewayTags returns true, a quest will be created. I checked the code - it should not return true in this case, but if it did, olderThan4Years.matches(this) is the most likely candidate fot returning true. Next steps:

  1. Question for @mnalis : Your smartphone clock shows the right time and date, right? Not anything like 4 years wrong or anything like that?
  2. Try to reproduce this issue for other quests. If it is reproducible for other quests, the theory that it is some issue with the AddCycleway quest is wrong
  3. When you can reproduce this again, try the following:
    a) Download the location manually. Does the quest now vanish? If it does, it is a weak indicator that the issue is somewhere in isApplicableTo.
    b) Undo the quest and answer it again with the same answer. Is it reproducible that way? If yes, it is a strong indicator for that it is not some race condition

The other relatively plausible explanation - that for some reason, the map pins were not updated - has been proven wrong because when clicking on the quest pin, the quest is fetched from the database. If it would not exist anymore in the database, the quest would not be opened (the pin appear to not be clickable).

@westnordost westnordost added the feedback required more info is needed, issue will be likely closed if it is not provided label Aug 4, 2022
@westnordost westnordost changed the title (reproduced2) Being immediately asked "is this still a cycleway situation here" after solving "cycleway type" quest Being immediately asked "is this still a cycleway situation here" after solving "cycleway type" quest Aug 4, 2022
@westnordost westnordost removed their assignment Aug 4, 2022
@mnalis
Copy link
Member Author

mnalis commented Aug 9, 2022

I looked closer through this issue, but could not find anything. I tried to reproduce the issue with that exact same cycleway. No result, wasn't able to.

Yeah, that behaviour is consistent with my previous tests in #4124 (comment) where I also immediately (before uploading answer) tried to reproduce the bug on the another phone on that same way and failed. So it does not seem to be reproducible-at-will, but only happens sometimes. I dislike such undeterministic bugs...

Question for @mnalis : Your smartphone clock shows the right time and date, right? Not anything like 4 years wrong or anything like that?

Time is correct, yes (at worst it might be several seconds wrong). It's my main phone so I'd notice if a hour or day (much less a year!) was wrong.

Try to reproduce this issue for other quests. If it is reproducible for other quests, the theory that it is some issue with the AddCycleway quest is wrong

So far, I've only noticed it in that quest, yes. That however might be because it is more noticeable because it throws suspicious Is this still the cycleway situation here? quest (which otherwise practically never happens to me).
If it simply asked the same quest (like most resurvey quests do), I'd probably be much more likely to miss it (i.e. attribute it to it being another very close small road segment). I'll try to monitor other quests more suspiciously and report here if I notice it elsewhere.

When you can reproduce this again, try the following:
a) Download the location manually. Does the quest now vanish? If it does, it is a weak indicator that the issue is somewhere in isApplicableTo.

OK, I'll try that.

b) Undo the quest and answer it again with the same answer. Is it reproducible that way? If yes, it is a strong indicator for that it is not some race condition

If I understand you correctly; I did that in the previous attempt (albeit in older SC version), and it was not reproducible #4086 (comment):

At one of them I did not answer the Is this still the cycleway situation here? quest, and instead I undid the original cycleway quest. When I then again answered that cycleway quest, the Is this still the cycleway situation here? did not pop up - so it might indicate that this is likely some kind of race condition in SC, and not related to problematic OSM data.

But I will try that again with new SC, when the bug strikes next time.
Would attempting to capture logcat output (after the bug shows itself) help?
Any debug I might add in my fork that might help?

@Helium314
Copy link
Collaborator

Would attempting to capture logcat output (after the bug shows itself) help?
Any debug I might add in my fork that might help?

You could trace what happens after the quest is solved: First isApplicableTo is called, if it returns false/true the quest is (not) created, if it returns null getApplicableElements is called.
So you could add logging at all places where isApplicableTo may return true or null, and where getApplicableElements may add an element to the list it returns. This should allow identifying the exact place where the quest is seen as applicable.

In particular, detailed logging of hasOldInvalidOrAmbiguousCyclewayTags would help.
If you intend to capture logcat only after the bug shows up, better set the log level to w or better e, lower levels tend to disappear quickly.

@mnalis
Copy link
Member Author

mnalis commented Aug 19, 2022

New datapoint: the bug happened again (in my 45.2 based fork). I solved cycleway quest, and got immediately presented with Is this still the cycleway situation here? which I dismissed (i.e. it was still actively shown as a purple cycleway quest on a map).

I then switched out to web browser (to lookup what should I check exactly, because I didn't remember 😄) and when I switched back to SC the cycleway quest was gone, with next-in-line smoothness quest displayed instead.

I'll try to do manual refresh next time to see how it handles, but I wanted to share this if it gives more clues.

@HolgerJeromin
Copy link
Contributor

Had this with seating quest.
Fetch new quest did not solved it.
Did not tried undo or app close.
Now after a day (mapped other location and restarted the phone) the next quest is shown.

@mnalis
Copy link
Member Author

mnalis commented Oct 7, 2022

what SC version was that @HolgerJeromin ? (i.e. still hoping that caching rewrite in 48.0-alpha1 might have fixed this elusive bug...)

@HolgerJeromin
Copy link
Contributor

It was v48.0-alpha1 sorry for that :-)
See linked issue #4466 for more context.

@westnordost
Copy link
Member

If anything, the caching will increase the potential for (such) heisenbugs, which is why @Helium314 added those extensive unit tests.

@hamishmb
Copy link

I sometimes get this and similar issues where I'm immediately asked the same question twice too.

I think it happens if StreetComplete is syncing data while I answer quests, sometimes. Usually when I'm on 3G/HSPA with a poor signal and the sync is taking a long time.

@riQQ
Copy link
Collaborator

riQQ commented Nov 29, 2022

Recently I was using StreetComplete 49.2 as a front passenger on a motorway. I answered a lot of bus stop quests and this bug happened several times. Sometimes there was a noticable lag when answering the quest the first time which was shown again later on. In some cases the quest was not shown again immediately and I was able to answer another quest on the same element before the same quest was shown again (e.g. bus stop shelter -> bench -> shelter again).

When checking the tagging via undo, the second answer to the same quest on the same element additionally had the checkdate tag set.

I don't remember if this was the case but maybe a download of OSM data was running in the background every time this occured as it's more likely to have a download running when moving fast.

Maybe another factor could be the fast answering of quests, as the answers were almost always the same.

@hamishmb
Copy link

Yep, my experience seems exactly the same as the above. I usually get it on faster roads too, and I think it is when it's syncing while I'm answering quests.

@eginhard
Copy link
Contributor

eginhard commented Dec 3, 2022

I just had this for the road smoothness quest: https://www.openstreetmap.org/way/545067568/history

I was standing in the same place and answered "good" 4 times, then stopped because the quest still didn't disappear. Re-scanning for quests did not help. 2 hours later and the quest is now gone.

@rhhsm
Copy link

rhhsm commented Dec 3, 2022

I reported similar behaviour in #4662 so it seems more likely to occur when the app is busy communicating with the net. However, something similar happened to me while being off line: after answering a surface quest for a track, it ask the surface quest again. Answering it the second time, the app asked if I was sure because what I answered was different from what was the value before, which I am sure was a mistake to ask because I entered the same answer twice. I 'm actually pretty sure it was asked for a track that already had the surface tagged months before (by me), so the quest shouldn't have shown up at all. Maybe that the "are you sure?" message popped up is a clue?

@westnordost
Copy link
Member

I wonder if we are on the wrong track here. If it also happens offline, maybe there is a race condition in the MapDataWithEditsSource, i.e. the part where the local edits get applied to the downloaded OSM data.

@westnordost
Copy link
Member

And it would explain why re-scanning for quests doesn't help. Even if the base OSM data is fine, if the MapDataWithEditsSource fucks up somehow, only restarting the app would help.

So: Are you sure it could be reproduced while being offline, @rhhsm? Or did you just have a bad connection or similar?

@rhhsm
Copy link

rhhsm commented Jan 6, 2023

I'll pay more attention next time it happens. Are there specific actions I could do to help find the source of the problem? "re-scanning for quests" certainly won't work when offline :)

@westnordost
Copy link
Member

westnordost commented Jan 6, 2023

Finding whether it is reproducible while offline would already be a big help, because this precludes a lot of other things as cause for this bug. Maybe notice how big is the number of not-uploaded changes.

Also, with offline I mean really offline, i.e. no download of new quests can happen while solving quests. Not the "manual upload" mode.

@Helium314
Copy link
Collaborator

I just had this issue. It was in EE, but fits the descriptions here, so it's most likely the same:

  • it was in online mode, but without internet connection, and with 30 not uploaded changes (and not logged in)
  • opening hours quest
  • only reproducible for one specific node, but here the quest wouldn't go away even after answering many times
    • unfortunately I didn't take logs or check what happens when answering other quests for the same node
  • after a re-scan the issue was gone

@hamishmb
Copy link

hamishmb commented Apr 2, 2023

I haven't had this issue for a little while now, has it potentially been fixed?

@peternewman
Copy link
Collaborator

I haven't had this issue for a little while now, has it potentially been fixed?

I had it on v52.0 of SC for some tactile paving or crossing button or both despite answering many times times, on 4G rather than my usual offline/WiFi situation.

@riQQ

This comment was marked as resolved.

@westnordost westnordost changed the title Being immediately asked "is this still a cycleway situation here" after solving "cycleway type" quest Quest is asked again immediately after solving it May 2, 2023
@Helium314
Copy link
Collaborator

How to reproduce the issue:

  • have some sort of live-readout for the log (e.g. connect phone to Android Studio)
  • start downloading a large area
  • prepare answering a quest inside the area you're downloading
  • when OsmQuestController starts with AddSuspectedOneway: Found 0 quests in 392ms and similar messages, answer the quest
  • wait a little
  • get your answered quest back
  • answer again, quest will still show

What is happening:
The quest is answered while quests from download are being created. The answered quest is created again, because the quest isn't answered in the used map data.
First, the created quests are persisted (in database), then the removed quest is persisted (so the database state is correct).

Now there are no details provided by the log, and I'm not 100% sure what's going on.
Since the OsmQuestController.onUpdated call is not synchronized, the call from the newly created quest, even if started later, might finish faster and call listeners first. In this case, the answered quest is first removed, and then added in the listener call from the downloaded quests.

Once the quest is not in the database, but in VisibleQuestsSource, the quest can be answered and will not be removed, because quests to remove are created from the database.

@westnordost
Copy link
Member

westnordost commented Sep 6, 2023

To summarize, creating the quests in onReplacedForBBox in OsmQuestController can take a moment. If the user edited the data (=solved quests) in this time, the data the quests created in onReplacedForBBox are based on will be outdated.

Making this code all synchronized would mean that the user effectively cannot make any edits while downloading, which will feel like the app is not responding. So, there'd need to be something like a modal downloading window to notify users of what is happening. However, this is not desirable.

The only solution that comes to my mind right now would be to keep track of the map data that is edited (i.e. anything that is passed to onUpdated in OsmQuestController) while onReplacedForBBox executes and call onUpdated for all this data again. In pseudocode:

var isReplacingForBbox = false
var dataToUpdate

fun onUpdated(updatedData) {
   ...

    if (isReplacingForBbox) dataToUpdate += updatedData
}

fun onReplacedForBBox {
    isReplacingForBbox = true

    ...

    isReplacingForBbox = false
    if (dataToUpdate.isNotEmpty()) {
        onUpdated(dataToUpdate)
        dataToUpdate.clear()
    }
}

(What the pseudocode disregards here but should be regarded in proper implementation: several onReplacedForBBox could be executed in parallel; the two functions could be executed on different threads -> use atomic? / synchronization when dealing with the isReplacingForBbox flag)

@westnordost westnordost removed the feedback required more info is needed, issue will be likely closed if it is not provided label Sep 6, 2023
@Helium314
Copy link
Collaborator

onUpdated(dataToUpdate)

Better pass a copy of dataToUpdate; if it's just some mutable list(s) it will likely be cleared before QuestPinsManager.updateQuestPins is called.

@Helium314
Copy link
Collaborator

This works for me, though it may miss something: Helium314@cae27b4

@rhhsm
Copy link

rhhsm commented Sep 22, 2023

I'd like to mention #4662 again here. Now that I've recently switched to a provider plan including mobile data, the issue of not being able to use the app for an annoyingly long time because an update is taking place occurs more frequently. I now switch off mobile data and wifi before starting to use SC during a survey to avoid having this issue, knowing this is against the idea of always working with up-to-date quests. Just saying that this is a weakness of SC, without having a good idea about solving it in an elegant way (I don't have much IT expertise, and admit that the "emergency stop" button I proposed before is not very elegant).

@Helium314
Copy link
Collaborator

Ideally disabling internet connection would cancel donwload, but that doesn't work. What does work for me is stopping VPN. But most people don't use VPN, so that's barely useful advice...

@rhhsm
Copy link

rhhsm commented Sep 22, 2023

Could updating the quests be limited to times when the phone's screen is off? Surely the user is not using the phone at those times, and wouldn't be bothered by the update taking place.

@westnordost westnordost self-assigned this Feb 3, 2024
@westnordost
Copy link
Member

I noticed that this should rather be solved one level higher, at MapDataWithEditsSource. Will look into it.

westnordost added a commit that referenced this issue Feb 3, 2024
…all onReplacedForBBox to listeners completed for updates that happened while the latter was being executed (fixes #4258)
westnordost added a commit that referenced this issue Feb 5, 2024
MapDataWithEditsSource: call onUpdated to listeners again after the call onReplacedForBBox to listeners completed for updates that happened while the latter was being executed (fixes #4258)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants