Skip to content

nitpicky kotlin/mvvm stuff (audio PR feedback)#123

Merged
dupontgu merged 3 commits into
feat/audiofrom
guy/feat/audio
Jul 24, 2026
Merged

nitpicky kotlin/mvvm stuff (audio PR feedback)#123
dupontgu merged 3 commits into
feat/audiofrom
guy/feat/audio

Conversation

@dupontgu

@dupontgu dupontgu commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Going to test in the morning before merging, but I have Gus' permission to do so.

@dupontgu
dupontgu requested a review from brunoro July 24, 2026 02:04
private var player: LightAudioPlayer? = null
private var stateJob: Job? = null

fun attachAudio(value: LightAudio) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main change to these viewmodels is I got rid of the attachAudio calls. LightAudio can be passed into the constructor in all these cases so you can reduce the number of nullable fields and get rid of initialization step inside the @Composables (which can get wonky with recomposition)

val newPlayer = value.newPlayer()
player = newPlayer
stateJob = viewModelScope.launch {
launch { newPlayer.positionMs.collect(positionMs::emit) }

@dupontgu dupontgu Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just pass a reference to the player's StateFlow's directly rather than pipe them through a new one

@dupontgu dupontgu changed the title nitpicky kotlin/mvvm stuff nitpicky kotlin/mvvm stuff (audio PR feedback) Jul 24, 2026
@dupontgu
dupontgu merged commit ce49fa7 into feat/audio Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant