Any reason why ```var done by remember(mediaItems) { mutableStateOf(false) } <inside callback&gt...

# compose

[reactormonk]

Any reason why

var done by remember(mediaItems) { mutableStateOf(false) }

<inside callback>
  done = true

doesn't trigger a recompose? the done = true is getting invoked, println debugging.

[vide]

you only get recomposition in the scopes you read the value of done in

[reactormonk]

At the end of the function defining it, I got

Box(
        modifier = Modifier.fillMaxWidth(),
        contentAlignment = Alignment.Center,
    ) {
        if (!done) {
            VideoPlayer(exoPlayer)
        } else {
            ErrorTemplate(message = stringResource(R.string.end_of_playlist))
        }
    }

And I'm wondering why the ErrorTemplate isn't being displayed

[vide]

are you changing mediaItems?

[vide]

you will lose the previous instance of the state because you create a new one when mediaItems changes

[reactormonk]

No, added a log statement in the remember block, no recreation

[vide]

can you post a bit more context?

[reactormonk]

Sure.

var done by remember(mediaItems) { Log.d(TAG, "Recreating done."); mutableStateOf(false) }
    var started = false
    val listener = object : Player.Listener {
        override fun onEvents(player: Player, events: Player.Events) {
            if (events.contains(Player.EVENT_PLAYBACK_STATE_CHANGED)) {
                if (player.playbackState == Player.STATE_ENDED && started) {
                    done = true
                    Log.d(TAG, "Done.")
                }
            }
        }

        override fun onIsPlayingChanged(isPlaying: Boolean) {
            if (isPlaying) {
                started = true
            }
        }
    }

    val exoPlayer = remember(context) {
        val mediaSourceFactory = DefaultMediaSourceFactory(context).setLoadErrorHandlingPolicy(
            DefaultLoadErrorHandlingPolicy()
        )
        val player = ExoPlayer.Builder(context).setMediaSourceFactory(mediaSourceFactory).build()
        player.addListener(listener)
//        player.addAnalyticsListener(EventLogger())
        player.repeatMode = REPEAT_MODE_OFF

        player
    }
    if (exoPlayer.availableCommands.contains(Player.COMMAND_CHANGE_MEDIA_ITEMS)) {
        exoPlayer.setMediaItems(mediaItems)
        exoPlayer.prepare()
        exoPlayer.playWhenReady = true
    } else {
        Log.e(TAG, "Can't change media items!?!")
    }

    if (mediaItemIndex > -1) {
        exoPlayer.seekTo(mediaItemIndex, position)
    }

    Box(
        modifier = Modifier.fillMaxWidth(),
        contentAlignment = Alignment.Center,
    ) {
        if (!done) {
            VideoPlayer(exoPlayer)
        } else {
            ErrorTemplate(message = stringResource(R.string.end_of_playlist))
        }
    }

... and a bit more lifecycle stuff to clean up things.

[vide]

this seems like you're calling exoPlayer.setMediaItems(mediaItems) exoPlayer.prepare() exoPlayer.seekTo(mediaItemIndex, position) on every recomposition

[vide]

you're also creating a new var started and a new listener on every recomposition

[reactormonk]

Thanks, moved things inside remember blocks

[vide]

you're probably seeing started update correctly because the player is memoized in the remember block for creating the exoplayer instance 🤔

[reactormonk]

Huh, I can't used remember blocks with Unit as return?

[vide]

you need to have something to remember, right? 😅 what are you trying to do?

[reactormonk]

remember(mediaItems) {

        if (exoPlayer.availableCommands.contains(Player.COMMAND_CHANGE_MEDIA_ITEMS)) {
            exoPlayer.setMediaItems(mediaItems)
            exoPlayer.prepare()
            exoPlayer.playWhenReady = true
        } else {
            Log.e(TAG, "Can't change media items!?!")
        }
}

[vide]

It looks like you could use a LaunchedEffect here? https://developer.android.com/jetpack/compose/side-effects#launchedeffect

[reactormonk]

Yup, exactly.

[reactormonk]

Nope, still not recomposing as it should 😭

[vide]

Can you post your updated source? I can try to to take a look again simple smile

[reactormonk]

ChatGPT tells me I shouldn't update MutableState from non-main-threads

[reactormonk]

https://gist.github.com/reactormonk/563a13bcb00f16971ce1145907e98553

[vide]

what kind of logs do you get by running that?

[reactormonk]

MediaItems: []
Recreating done.
Exo player: androidx.media3.exoplayer.ExoPlayerImpl@fef39da
currentMediaItem: null
Done: false
MediaItems: [androidx.media3.common.MediaItem@1db4b9bf]
Recreating done.
Exo player: androidx.media3.exoplayer.ExoPlayerImpl@fef39da
currentMediaItem: null
Done: false
Done.

[reactormonk]

The second Recreating done. is fine, I'm changing the MediaItems, so it's expected. The final Done. is where I'm changing the done variable.

[vide]

Ah, so you are changing your mediaItems. That is a problem, can you see why?

[reactormonk]

This one should go in there certainly:

var started = false
    
    LaunchedEffect(mediaItems) {
        started = false
    }

[reactormonk]

... but that's not the exact issue here 🤔

[vide]

The issue is that your listener contains a reference to the first done state. You change mediaItems after it has captured it in the closure, and it creates a new state, which always stays in false. Because your listener is actually updating the first instance.

[reactormonk]

🤦 , moved it inside a DisposableEffect to change out the listener

[vide]

I would suggest considering using a state holder class since managing complex state inside composables can be quite hard.

[reactormonk]

Aka shoving things into a data class?

[reactormonk]

Sorry, my background is FP, not OOP, so I'm not sure what a "state holder class" is 😅

[Stylianos Gakis]

Are you using rememberPagerState anywhere in your app? PagerState is an example of such a class

[vide]

It doesn't have to be data, but it can also be that. You can just use a regular class too. The main point is to separate your state and mutation logic from the UI code

[Stylianos Gakis]

https://developer.android.com/jetpack/compose/state#managing-state

[reactormonk]

Tried doing so already, but because the state depends a whole lot on callbacks because of the ExoPlayer API, I'm not sure which state I can push outside of the composable 🤔

[reactormonk]

Good think about the LaunchedEffect and recomposing, had similar bugs in other parts of the code 😅

[vide]

I don't have too much time to think about the architecture, but you could for example move creating the exoplayer object to a class, and have a method for changing the active index. Then you could just use state exposed by the class to drive your UI

[Filip Wiesner]

Ah, so you are changing your mediaItems. That is a problem, can you see why?

I love this ❤️ I wish someone gave me code reviews like this ☺️