Only show lit quests during night-time #2872

TurnrDev · 2021-05-11T19:33:14Z

Fixes #1285 by working out the time of civil sunset and sunrise

It currently works via mayCreateQuest the same way blocking quests out of the country works, but that's causing that sync issue when toggling the setting. I need to move the logic to displaying the quest instead of creating the quest, I'm not so sure where that would be in the code base so any help would be appreciated.

TurnrDev · 2021-05-11T19:34:18Z

westnordost

What is the use case for displaying this option at all and not always only show those quests that should be only visible by night at night? Would make implementation a little easier, less code and less clutter in the settings menu.

It would be ideal if the app could register a callback whenever the sunset or sunrise event occurs. Maybe one of the libraries mentioned can do that. Then, the VisibleQuestsSource could listen for that and invalidate the quests so that they are refreshed.

app/src/main/java/de/westnordost/streetcomplete/data/quest/DayNightCycle.kt

app/build.gradle.kts

app/src/main/java/de/westnordost/streetcomplete/data/osm/osmquests/OsmQuestController.kt

TurnrDev · 2021-05-11T20:45:36Z

What is the use case for displaying this option at all and not always only show those quests that should be only visible by night at night? Would make implementation a little easier, less code and less clutter in the settings menu.

Some roads are lit day and night, so I mainly added it because of that

TurnrDev · 2021-05-11T20:51:43Z

Thank you for the review, I will take a closer look tomorrow and see what I can do to fix it up!

smichel17 · 2021-05-12T00:16:42Z

For reference, other conversations related to displaying quests are collected here: #2789 (comment).
It might be worth giving them a skim — or not, some are quite long :P

app/src/main/java/de/westnordost/streetcomplete/data/visiblequests/DayNightQuestFilter.kt

app/src/main/java/de/westnordost/streetcomplete/util/CheckIfNight.kt

westnordost · 2021-05-14T11:16:43Z

app/src/main/java/de/westnordost/streetcomplete/util/CheckIfNight.kt

+    return calendar
+}
+
+fun isNight(pos: LatLon): Boolean {


Ok, I now recognize that here is a potential for a performance issue. Let's say you have 1000 quests that shall not be visible during day and they shall now be displayed on the map.

Then, the calculation in CheckIfNight is executed 1000 times. I am not sure how expensive the calculation is, but to circumvent that, I suggest the following:

isNight calculates the sunset/dawn with the given latitude and longitude truncated to two decimals (precision of ~1km). That should be enough precision

the calculation is cached in a dayTime: Map<LatLon, Pair<LocalTime,LocalTime>>. Only if there is no entry in the map, the calculation is done. LatLon is always the truncated latlon.

If isNight is going to have a state, it would make sense to make this into a class, or maybe just put the code here in DayNightQuestFilter

Whilst I fully agree with you, I'm not sure how to go about implementing this

Hmm… … how should this feature actually work?

When it is detected to be after sunset at the user's current position, all lit quests appear.

How I imagined it would work before reading the code.

Should be synchronized with using the dark theme, if the "Auto" theme is selected.

Simple to implement, cheap to compute.

What if we don't have the user's current location?

What if the user is looking (say, to see what quests are available) at a position different than their current location? Use gps or screen/scroll position?

Lit quests appear if it is past sunset at their current location.

How it's currently implemented, I think.

Potential performance issues, or more complicated implemented to mitigate them.

Might be confusing (hard to figure out when a given lit quest will appear), particularly along with the "available in 3 hours" feature suggested below.

Might also be cool — seeing lit quests gradually appear as the sun crawls across the map. But would definitely work better if the map visualized the current sun line instead of switching all at once.

Although solution 2 is more robust, personally I'm more inclined toward solution 1 and not worrying about making the app work in those edge cases, since they are not the primary use case of StreetComplete (in-person surveying).

@smichel17, if you are going to comment on the code, you should read it first before making assumptions.

In the diff, I came across this code:

StreetComplete/app/src/main/java/de/westnordost/streetcomplete/data/visiblequests/DayNightQuestFilter.kt

Lines 13 to 19 in e5c6135

fun isVisible(quest: Quest): Boolean {

return when (quest.type.dayNightCycle) {

DAY_AND_NIGHT -> true

ONLY_DAY -> !isNight(quest.position)

ONLY_NIGHT -> isNight(quest.position)

}

}

Which seems to be applied per-quest. I admit I have not traced the call stack more than one step, to here:

StreetComplete/app/src/main/java/de/westnordost/streetcomplete/data/quest/VisibleQuestsSource.kt

Lines 86 to 87 in e5c6135

private fun isVisible(quest: Quest): Boolean =

visibleQuestTypeSource.isVisible(quest.type) && teamModeQuestFilter.isVisible(quest) && dayNightQuestFilter.isVisible(quest)

to see if it is called differently, but I don't think my assumption — made after reading the code — was unreasonable.

It seems like I have missed something that you find obvious, but I'm honestly not sure what it is. Also happy to figure it out myself (no need for you to spend time explaining if it really is that obvious) if you want to point me in the right direction (to the code you expected I'd seen).

Alright, maybe a misunderstanding. The way you wrote the other comment sounded like you would comment on the functionality in general as if no code had been written yet.

In any case, I'd also prefer 1, but I am not going to reject this PR just because 2 and not 1 has been implemented.

If @TurnrDev or you would like to move to behavior 1 instead, I can give advice how to do that on the technical side.

app/src/main/java/de/westnordost/streetcomplete/util/CheckIfNight.kt

dbdean · 2021-05-21T06:49:36Z

Would it be possible to tag (like survey:atnight=yes or something) to indicate that the answer was surveyed during night time?That way we could re-ask this for roads/paths/etc that are already tagged lit but weren't surveyed at night to verify that they are actually lit at night time, and didn't just look like they should be lit.

westnordost · 2021-05-21T12:50:17Z

Well, this is the wrong place to propose a tag like this.

westnordost · 2021-05-21T12:51:50Z

@TurnrDev What would be quite cool would be that if you go into the quest selection settings, it will actually show the quest as disabled and say something like "available in 3 hours" or something

10992-osm · 2021-06-07T21:43:31Z

Would it be possible to tag (like survey:atnight=yes or something) to indicate that the answer was surveyed during night time?That way we could re-ask this for roads/paths/etc that are already tagged lit but weren't surveyed at night to verify that they are actually lit at night time, and didn't just look like they should be lit.

Could you not infer that from when the changeset was made? (I suppose that might not work with delayed uploads, whether due to connection issues or deliberate choice on the part of the user).

TurnrDev · 2021-06-07T22:30:44Z

I got in my head with imposter syndrome, I doubt this will be ready in time for v33

Gonna have to break all my to-do up into smaller tasks and focus on one at a time lol

TurnrDev · 2021-06-08T11:28:39Z

To Do

Fix comment on isNight

StreetComplete/app/src/main/java/de/westnordost/streetcomplete/util/CheckIfNight.kt

Lines 20 to 24 in e5c6135

    
               /* This functions job is to check if it's currently dark out. 
        
               It will use the location of the node and check the civil sunset time. 
        
               Currently it checks the phone time and returns true if between 7pm and 7am. 
        
                */

Cache the ifNight calculation? Only show lit quests during night-time #2872 (comment) @westnordost said he could potentially help with this.
- At the very least, time the impact isNight is adding to performance
Find out if timezone is needed Only show lit quests during night-time #2872 (comment)
Add countdown to sunset in quest list Only show lit quests during night-time #2872 (comment)
Cry

westnordost · 2021-07-05T16:20:56Z

Note: If this is merged, the quest priority of lit-quests should go up, as during night, those should have higher priority (because they are easier to solve)

dbdean · 2021-07-18T07:12:33Z

I was having a thought about this PR recently. What would the plan be for lit on sports pitches? I treat the current lit quest for them as 'can this sporting pitch be lit', rather than is it lit every night like I would treat a similar quest on roads and paths.

So (if I'm treating the sporting lit quest appropriately) the sporting lit quest is actually generally easier to answer in the day time as you can see the lighting poles, which might be harder to see at night if they aren't on that particular night.

TurnrDev · 2021-07-18T13:15:40Z

So (if I'm treating the sporting lit quest appropriately) the sporting lit quest is actually generally easier to answer in the day time as you can see the lighting poles, which might be harder to see at night if they aren't on that particular night.

Good point, and I agree with you there. I will make changes

TurnrDev · 2021-08-21T08:14:35Z

@westnordost As much as I'd like to cache the result, I'm not entirely sure on how to do that! I have changed the function to isDay to resolve a bug with regards to after midnight. I'm testing it soon :)

I removed AddPitchLit from the PR, @dbdean raised a good point regarding pitches

I also movet the remaining lit quests to the top. Building now :)

Squashed Commit for streetcomplete#2872

TurnrDev · 2021-08-21T09:20:33Z

Just tested in Alert, Nunavut, Canada which currently doesn't experience sundown. Worked fine.

westnordost · 2021-08-21T12:38:41Z

app/src/main/java/de/westnordost/streetcomplete/quests/QuestModule.kt

        //      the sake of mapping it in case it makes sense later
        AddPitchSurface(),
-        AddPitchLit(),
+        AddPitchLit(), // Not affected by new DayNight cycle because the lights are usually only on during games


I know this has been requested by someone before, but I don't completely follow that logic.

The lights are usually only on during games, but only if it is dark outside, right? So, that should not change the fact that whether it is lit or not will only be possible to see when it is dark.

True, but given how infrequent games are- It would be possible to pass by on the street when there isn't a game, see no light on and assume there aren't any.

And during the daytime, it will be easier to see if there are lights at all.

Hm okay, if you agree with each other there..

Doesn't the same argument apply to all the lit quests though, well in the positive direction at least, I can see street lights during the day so I should be able to tag them during the daytime. Although I appreciate marking no during the day is vaguer.

westnordost · 2021-08-21T12:52:25Z

I'll also probably add a cache after merge, so the isDay will not be calculated for every single road.

TurnrDev · 2021-08-21T13:04:17Z

I'll also probably add a cache after merge, so the isDay will not be calculated for every single road.

I was thinking it might be better to calculate for user position, but I don't know what the course of action to work that out is.

smichel17 · 2021-08-22T23:33:30Z

I have a couple dev tasks I've volunteered to do, so I don't know about my bandwidth here, but besides that I'd be up to help with performance work here (caching and/or using the current map position instead of calculating for each quest individually).
…I'd be more exited about it if someone wanted to do the profiling exploration first (the most boring and most important part :P)

westnordost · 2021-08-22T23:34:00Z

No, I am right now doing it myself but thank you for the offer

TurnrDev · 2021-08-23T05:25:15Z

@westnordost Thank you for your improvements. I'm really not sure I would have ever figured most of that out

Squashed Commit for streetcomplete#2872

westnordost requested changes May 11, 2021

View reviewed changes

app/src/main/java/de/westnordost/streetcomplete/data/quest/DayNightCycle.kt Outdated Show resolved Hide resolved

app/build.gradle.kts Show resolved Hide resolved

app/src/main/java/de/westnordost/streetcomplete/data/osm/osmquests/OsmQuestController.kt Outdated Show resolved Hide resolved

TurnrDev force-pushed the feature/sunsetlit branch from d74f847 to e5c6135 Compare May 12, 2021 21:39

TurnrDev requested a review from westnordost May 13, 2021 21:56

westnordost reviewed May 14, 2021

View reviewed changes

TurnrDev requested a review from westnordost August 21, 2021 08:11

Only show lit quests during night-time

a9f9cdf

Squashed Commit for streetcomplete#2872

TurnrDev force-pushed the feature/sunsetlit branch from 37fa7c0 to a9f9cdf Compare August 21, 2021 08:39

westnordost approved these changes Aug 21, 2021

View reviewed changes

westnordost merged commit 03645ed into streetcomplete:master Aug 22, 2021

TurnrDev deleted the feature/sunsetlit branch August 23, 2021 05:17

FloEdelmann mentioned this pull request Sep 19, 2021

The sports pitch lighting quest shows up during daytime #3299

Closed

westnordost mentioned this pull request Feb 13, 2022

Add seating quest #3758

Merged

Helium314 pushed a commit to Helium314/SCEE that referenced this pull request May 14, 2022

Only show lit quests during night-time

9370ef7

Squashed Commit for streetcomplete#2872

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only show lit quests during night-time #2872

Only show lit quests during night-time #2872

TurnrDev commented May 11, 2021

TurnrDev commented May 11, 2021

westnordost left a comment

TurnrDev commented May 11, 2021

TurnrDev commented May 11, 2021

smichel17 commented May 12, 2021

westnordost May 14, 2021

TurnrDev May 21, 2021

smichel17 May 21, 2021 •

edited

Loading

westnordost May 21, 2021

smichel17 May 21, 2021 •

edited

Loading

westnordost May 22, 2021

westnordost May 22, 2021

dbdean commented May 21, 2021

westnordost commented May 21, 2021

westnordost commented May 21, 2021

10992-osm commented Jun 7, 2021

TurnrDev commented Jun 7, 2021

TurnrDev commented Jun 8, 2021 •

edited

Loading

westnordost commented Jul 5, 2021

dbdean commented Jul 18, 2021

TurnrDev commented Jul 18, 2021

TurnrDev commented Aug 21, 2021

TurnrDev commented Aug 21, 2021

westnordost Aug 21, 2021

TurnrDev Aug 21, 2021

dbdean Aug 22, 2021

westnordost Aug 22, 2021

peternewman Aug 27, 2021

westnordost commented Aug 21, 2021

TurnrDev commented Aug 21, 2021

smichel17 commented Aug 22, 2021

westnordost commented Aug 22, 2021

TurnrDev commented Aug 23, 2021

	fun isVisible(quest: Quest): Boolean {
	return when (quest.type.dayNightCycle) {
	DAY_AND_NIGHT -> true
	ONLY_DAY -> !isNight(quest.position)
	ONLY_NIGHT -> isNight(quest.position)
	}
	}

	private fun isVisible(quest: Quest): Boolean =
	visibleQuestTypeSource.isVisible(quest.type) && teamModeQuestFilter.isVisible(quest) && dayNightQuestFilter.isVisible(quest)

Only show lit quests during night-time #2872

Only show lit quests during night-time #2872

Conversation

TurnrDev commented May 11, 2021

TurnrDev commented May 11, 2021

westnordost left a comment

Choose a reason for hiding this comment

TurnrDev commented May 11, 2021

TurnrDev commented May 11, 2021

smichel17 commented May 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smichel17 May 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smichel17 May 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbdean commented May 21, 2021

westnordost commented May 21, 2021

westnordost commented May 21, 2021

10992-osm commented Jun 7, 2021

TurnrDev commented Jun 7, 2021

TurnrDev commented Jun 8, 2021 • edited Loading

To Do

westnordost commented Jul 5, 2021

dbdean commented Jul 18, 2021

TurnrDev commented Jul 18, 2021

TurnrDev commented Aug 21, 2021

TurnrDev commented Aug 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westnordost commented Aug 21, 2021

TurnrDev commented Aug 21, 2021

smichel17 commented Aug 22, 2021

westnordost commented Aug 22, 2021

TurnrDev commented Aug 23, 2021

smichel17 May 21, 2021 •

edited

Loading

smichel17 May 21, 2021 •

edited

Loading

TurnrDev commented Jun 8, 2021 •

edited

Loading