StreetComplete icon indicating copy to clipboard operation
StreetComplete copied to clipboard

Being immediately asked "is this still a cycleway situation here" after solving "cycleway type" quest

Open mnalis opened this issue 2 years ago • 6 comments

This is again reproduced https://github.com/streetcomplete/StreetComplete/issues/4124 (and before that, https://github.com/streetcomplete/StreetComplete/issues/4086). See them for more details and history of the problem.

I've managed to do both things requested in https://github.com/streetcomplete/StreetComplete/issues/4124#issuecomment-1173187068:

  • 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 :smile:)
  • 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:

https://user-images.githubusercontent.com/156656/182037500-aeb8c647-8e17-46d9-8b61-c936509c68f6.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 avatar Jul 31 '22 17:07 mnalis

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 avatar Jul 31 '22 17:07 mnalis

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).

mnalis avatar Jul 31 '22 17:07 mnalis

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 avatar Aug 04 '22 14:08 westnordost

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 https://github.com/streetcomplete/StreetComplete/issues/4124#issue-1272891845 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 https://github.com/streetcomplete/StreetComplete/issues/4086#issue-1262635344:

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?

mnalis avatar Aug 09 '22 11:08 mnalis

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.

Helium314 avatar Aug 09 '22 15:08 Helium314

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 :smile:) 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.

mnalis avatar Aug 19 '22 07:08 mnalis

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.

HolgerJeromin avatar Oct 06 '22 19:10 HolgerJeromin

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

mnalis avatar Oct 07 '22 03:10 mnalis

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

HolgerJeromin avatar Oct 07 '22 06:10 HolgerJeromin

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

westnordost avatar Oct 07 '22 09:10 westnordost

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.

hamishmb avatar Oct 26 '22 15:10 hamishmb

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.

riQQ avatar Nov 29 '22 00:11 riQQ

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.

hamishmb avatar Nov 29 '22 08:11 hamishmb

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.

eginhard avatar Dec 03 '22 13:12 eginhard

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?

rhhsm avatar Dec 03 '22 17:12 rhhsm

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 avatar Jan 06 '23 12:01 westnordost

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?

westnordost avatar Jan 06 '23 12:01 westnordost

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 :)

rhhsm avatar Jan 06 '23 16:01 rhhsm

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.

westnordost avatar Jan 06 '23 16:01 westnordost

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

Helium314 avatar Jan 18 '23 09:01 Helium314

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

hamishmb avatar Apr 02 '23 12:04 hamishmb

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.

peternewman avatar Apr 26 '23 22:04 peternewman

Could someone with the required permissions please update the issue title to reflect that this issue happens with multiple / presumably every quest type?

riQQ avatar May 02 '23 14:05 riQQ

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.

Helium314 avatar Sep 05 '23 18:09 Helium314

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 avatar Sep 06 '23 10:09 westnordost

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 avatar Sep 17 '23 07:09 Helium314

This works for me, though it may miss something: https://github.com/Helium314/SCEE/commit/cae27b435ea1f2467fdeec1dc432b91180b0e7aa

Helium314 avatar Sep 17 '23 07:09 Helium314

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).

rhhsm avatar Sep 22 '23 12:09 rhhsm

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...

Helium314 avatar Sep 22 '23 17:09 Helium314

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.

rhhsm avatar Sep 22 '23 17:09 rhhsm