diff --git a/android/app/src/main/java/com/guidecity/app/ui/guide/GuideScreen.kt b/android/app/src/main/java/com/guidecity/app/ui/guide/GuideScreen.kt index e294b68..3c1e8e5 100644 --- a/android/app/src/main/java/com/guidecity/app/ui/guide/GuideScreen.kt +++ b/android/app/src/main/java/com/guidecity/app/ui/guide/GuideScreen.kt @@ -114,11 +114,7 @@ fun GuideScreen( ) LazyColumn(state = listState) { itemsIndexed(state.places, key = { _, place -> place.id }) { index, place -> - val cardState = when { - index == state.activeIndex -> CardState.ACTIVE - index in state.doneIndices -> CardState.DONE - else -> CardState.UPCOMING - } + val cardState = if (index == state.activeIndex) CardState.ACTIVE else CardState.UPCOMING PlaceCard( place = place, state = cardState, diff --git a/android/app/src/main/java/com/guidecity/app/ui/guide/GuideViewModel.kt b/android/app/src/main/java/com/guidecity/app/ui/guide/GuideViewModel.kt index 154fb10..a2e3254 100644 --- a/android/app/src/main/java/com/guidecity/app/ui/guide/GuideViewModel.kt +++ b/android/app/src/main/java/com/guidecity/app/ui/guide/GuideViewModel.kt @@ -29,22 +29,24 @@ data class GuideUiState( val isLoading: Boolean = true, val places: List = emptyList(), val activeIndex: Int = 0, - val doneIndices: Set = emptySet(), val isMuted: Boolean = false, val errorMessage: String? = null, ) /** - * Owns the nearby-places list, which card is actively narrating, which ones - * have finished, and the 60-second re-scan loop. Re-scanning currently only - * re-sorts by fresh distance (TODO(v2): factor in the user's speed/heading - * once the backend's reserved `speed`/`heading` nearby params are used, to - * auto-pick a shorter content length when moving fast through a dense area). + * Owns the nearby-places list, which card is actively narrating, and the + * 60-second re-scan loop. Re-scanning currently only re-sorts by fresh + * distance (TODO(v2): factor in the user's speed/heading once the backend's + * reserved `speed`/`heading` nearby params are used, to auto-pick a shorter + * content length when moving fast through a dense area). * - * "Done" is derived from [UserPrefsDataStore.listenedPlaceIds] — a place - * that was ever narrated to completion (in this screen or the detail - * screen) is never auto-narrated again, even across re-scans or an app - * restart, until the user resets it from Settings. + * [GuideUiState.places] only ever holds places *not yet* narrated to + * completion, sorted by distance — a place disappears from the list the + * instant its narration ends (not mid-read), and every re-scan merges in + * newly-in-radius places while dropping any that are already listened, + * always re-sorted by fresh distance. "Listened" is persisted via + * [UserPrefsDataStore.listenedPlaceIds], so it survives re-scans and app + * restarts until the user resets it from Settings. */ @HiltViewModel class GuideViewModel @Inject constructor( @@ -66,31 +68,31 @@ class GuideViewModel @Inject constructor( viewModelScope.launch { contentLength = userPrefsDataStore.contentLength.first() ?: ContentLength.MEDIUM // Must be populated before the first loadNearby(), so places already - // listened in a previous session are correctly skipped from the start. + // listened in a previous session are correctly excluded from the start. listenedIds = userPrefsDataStore.listenedPlaceIds.first() Log.i(TAG, "init: contentLength=$contentLength, ${listenedIds.size} place(s) already listened") ttsManager.setLanguage(Locale("ru")) - loadNearby(startNarration = true) + loadNearby() startRescanLoop() } // React to *later* changes only (e.g. the Settings "reset" button) — - // the initial value was already consumed above via .first(). + // the initial value was already consumed above via .first(). A normal + // narration completion also grows this set (see onNarrationDone, + // which already removes the card synchronously), so only a *shrink* + // (a reset) needs a fresh fetch to bring previously-hidden places back. viewModelScope.launch { userPrefsDataStore.listenedPlaceIds.drop(1).collect { ids -> - Log.d(TAG, "listenedPlaceIds changed: ${ids.size} total") + Log.d(TAG, "listenedPlaceIds changed externally: ${ids.size} total (was ${listenedIds.size})") + val wasReset = ids.size < listenedIds.size listenedIds = ids - recomputeDoneIndices() + if (wasReset) { + Log.i(TAG, "listenedPlaceIds shrank — reset detected, reloading") + loadNearby() + } } } } - private fun recomputeDoneIndices() { - val state = _uiState.value - if (state.places.isEmpty()) return - val doneIdx = state.places.indices.filter { state.places[it].id in listenedIds }.toSet() - _uiState.value = state.copy(doneIndices = doneIdx) - } - private fun startRescanLoop() { if (rescanStarted) return rescanStarted = true @@ -99,12 +101,12 @@ class GuideViewModel @Inject constructor( while (isActive) { delay(60_000) Log.d(TAG, "re-scan tick") - loadNearby(startNarration = false) + loadNearby() } } } - private suspend fun loadNearby(startNarration: Boolean) { + private suspend fun loadNearby() { val location = selectedLocationHolder.location.value ?: locationProvider.getCurrentLocation() if (location == null) { Log.w(TAG, "loadNearby: no location available") @@ -124,19 +126,7 @@ class GuideViewModel @Inject constructor( ) }.onSuccess { response -> Log.i(TAG, "loadNearby success: ${response.places.size} place(s), radius=${response.searchRadiusM}m") - val places = response.places - val doneIdx = places.indices.filter { places[it].id in listenedIds }.toSet() - val firstUnlistened = places.indices.firstOrNull { it !in doneIdx } ?: 0 - _uiState.value = _uiState.value.copy( - isLoading = false, - places = places, - activeIndex = firstUnlistened, - doneIndices = doneIdx, - errorMessage = null, - ) - if (startNarration && firstUnlistened !in doneIdx) { - narrateActive() - } + applyFreshPlaces(response.places) }.onFailure { error -> Log.e(TAG, "loadNearby failed", error) _uiState.value = _uiState.value.copy( @@ -146,12 +136,40 @@ class GuideViewModel @Inject constructor( } } + /** + * Merges a fresh (already distance-sorted) nearby response into the + * visible list: places already listened are dropped entirely, and + * whichever place was actively narrating stays active — by id, not by + * position, since a re-scan can shuffle everyone's index — so a re-scan + * never visually skips ahead while a card is still mid-narration. + */ + private fun applyFreshPlaces(freshPlaces: List) { + val filtered = freshPlaces.filter { it.id !in listenedIds } + val currentState = _uiState.value + val activePlaceId = currentState.places.getOrNull(currentState.activeIndex)?.id + val newActiveIndex = activePlaceId + ?.let { id -> filtered.indexOfFirst { it.id == id }.takeIf { it >= 0 } } + ?: 0 + + _uiState.value = currentState.copy( + isLoading = false, + places = filtered, + activeIndex = newActiveIndex, + errorMessage = null, + ) + if (filtered.isNotEmpty()) { + // No-ops (just re-attaches the callback) if this exact place is + // already the one TtsManager is speaking — see TtsManager.speak. + narrateActive() + } + } + /** Retries after a failed load (e.g. tap a "retry" button shown on error). */ fun retry() { Log.d(TAG, "retry() called") viewModelScope.launch { _uiState.value = _uiState.value.copy(isLoading = true, errorMessage = null) - loadNearby(startNarration = true) + loadNearby() } } @@ -163,19 +181,22 @@ class GuideViewModel @Inject constructor( ttsManager.speak(place.id, text) { onNarrationDone() } } - /** Advances to the next place that hasn't been listened to yet, skipping any already-done ones. */ + /** + * Fires when the active card finishes reading all the way through (never + * on a mid-read interruption). The finished place is removed from the + * list immediately — it disappears rather than lingering in a "done" + * state — and narration advances to whichever place is now closest. + */ private fun onNarrationDone() { val state = _uiState.value - val newDone = state.doneIndices + state.activeIndex - val nextIndex = state.places.indices.firstOrNull { it !in newDone } - Log.d(TAG, "onNarrationDone: finished index=${state.activeIndex}, nextIndex=$nextIndex") - if (nextIndex == null) { - Log.i(TAG, "onNarrationDone: no more unlistened places") - _uiState.value = state.copy(doneIndices = newDone) - return + val finished = state.places.getOrNull(state.activeIndex) ?: return + listenedIds = listenedIds + finished.id + val remaining = state.places.filterNot { it.id == finished.id } + Log.d(TAG, "onNarrationDone: finished placeId=${finished.id}, ${remaining.size} place(s) left") + _uiState.value = state.copy(places = remaining, activeIndex = 0) + if (remaining.isNotEmpty()) { + narrateActive() } - _uiState.value = state.copy(doneIndices = newDone, activeIndex = nextIndex) - narrateActive() } fun toggleMute() { diff --git a/android/app/src/test/java/com/guidecity/app/ui/guide/GuideViewModelTest.kt b/android/app/src/test/java/com/guidecity/app/ui/guide/GuideViewModelTest.kt index a77b124..efa03e3 100644 --- a/android/app/src/test/java/com/guidecity/app/ui/guide/GuideViewModelTest.kt +++ b/android/app/src/test/java/com/guidecity/app/ui/guide/GuideViewModelTest.kt @@ -121,7 +121,7 @@ class GuideViewModelTest { } @Test - fun `places already listened are marked done and skipped, not re-narrated`() = runTest { + fun `places already listened are excluded from the list entirely, not re-narrated`() = runTest { val placesRepository = mockk() val response = NearbyResponseDto( searchRadiusM = 300, @@ -133,12 +133,12 @@ class GuideViewModelTest { } returns response val ttsManager = mockk(relaxed = true) - // place id 1 (index 0) was already listened in a previous session + // place id 1 was already listened in a previous session val viewModel = buildViewModel(placesRepository, ttsManager, listenedIds = setOf(1)) try { val state = viewModel.uiState.value - assertTrue(0 in state.doneIndices) - assertEquals(1, state.activeIndex) + assertEquals(listOf(2, 3), state.places.map { it.id }) + assertEquals(0, state.activeIndex) verify(exactly = 0) { ttsManager.speak(1, any(), any()) } verify { ttsManager.speak(2, any(), any()) } } finally { @@ -146,6 +146,75 @@ class GuideViewModelTest { } } + @Test + fun `a finished card disappears immediately and narration advances to the next`() = runTest { + val placesRepository = mockk() + val response = NearbyResponseDto( + searchRadiusM = 300, + count = 2, + places = listOf(fakePlace(1), fakePlace(2)), + ) + coEvery { + placesRepository.getNearby(lat = any(), lon = any(), citySlug = any(), lang = any(), length = any()) + } returns response + + val onDoneCallbacks = mutableMapOf Unit>() + val ttsManager = mockk(relaxed = true) + every { ttsManager.speak(any(), any(), any()) } answers { + onDoneCallbacks[firstArg()] = thirdArg() + } + + val viewModel = buildViewModel(placesRepository, ttsManager) + try { + assertEquals(listOf(1, 2), viewModel.uiState.value.places.map { it.id }) + + onDoneCallbacks.getValue(1).invoke() + + val state = viewModel.uiState.value + assertEquals(listOf(2), state.places.map { it.id }) + assertEquals(0, state.activeIndex) + verify { ttsManager.speak(2, any(), any()) } + } finally { + viewModel.viewModelScope.cancel() + } + } + + @Test + fun `a re-scan merges fresh places but keeps narrating the same active place by id`() = runTest { + val placesRepository = mockk() + val firstResponse = NearbyResponseDto( + searchRadiusM = 300, + count = 2, + places = listOf(fakePlace(1), fakePlace(2)), + ) + // A rescan brings in a place *closer* than the one already narrating, + // and place 2 has fallen out of radius. + val secondResponse = NearbyResponseDto( + searchRadiusM = 300, + count = 2, + places = listOf(fakePlace(0), fakePlace(1)), + ) + coEvery { + placesRepository.getNearby(lat = any(), lon = any(), citySlug = any(), lang = any(), length = any()) + } returns firstResponse andThen secondResponse + + val ttsManager = mockk(relaxed = true) + val viewModel = buildViewModel(placesRepository, ttsManager) + try { + val initial = viewModel.uiState.value + assertEquals(1, initial.places[initial.activeIndex].id) + + // Re-uses the same loadNearby()/applyFreshPlaces() path a 60s re-scan tick takes. + viewModel.retry() + + val state = viewModel.uiState.value + assertEquals(listOf(0, 1), state.places.map { it.id }) + assertEquals(1, state.places[state.activeIndex].id) + } finally { + viewModel.viewModelScope.cancel() + } + } + @Test fun `retry after a failure can succeed and clears the error`() = runTest { val placesRepository = mockk()