Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: option state crash #1456

Merged
merged 5 commits into from
Nov 2, 2023
Merged

fix: option state crash #1456

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f474b2b
fix: option state crash
BenjaminHalko Nov 1, 2023
1a80164
Update SelectedAppInfoViewModel.kt
BenjaminHalko Nov 1, 2023
0af456f
Update SelectedAppInfoViewModel.kt
BenjaminHalko Nov 1, 2023
2e18289
Update SelectedAppInfoViewModel.kt
BenjaminHalko Nov 1, 2023
a5e19f3
Update app/src/main/java/app/revanced/manager/ui/viewmodel/SelectedAp…
BenjaminHalko Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/src/main/java/app/revanced/manager/ui/viewmodel/SelectedAppInfoViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ class SelectedAppInfoViewModel(input: Params) : ViewModel(), KoinComponent {
viewModelScope.launch(Dispatchers.Default) {
if (!persistConfiguration) return@launch // TODO: save options for patched apps.

state.value = optionsRepository.getOptions(selectedApp.packageName)
withContext(Dispatchers.Main) {
state.value = optionsRepository.getOptions(selectedApp.packageName)
}
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for catching this. It would be better to remove the Dispatchers.Default parameter from the launch call and do something like state.value = withContext(Dispatchers.Default) { optionsRepository.getOptions() } instead. This will run the database query on another thread, which was the original goal of the code.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

viewModelScope.launch {
    if (!persistConfiguration) return@launch // TODO: save options for patched apps.

    state.value = withContext(Dispatchers.Default) { optionsRepository.getOptions(selectedApp.packageName) }
}

This still causes it to crash. Did I do something wrong?

Copy link
Member

Choose a reason for hiding this comment

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

You did everything correctly. It appears accessing selectedApp.packageName in the Default dispatcher is causing the issue, since that is also a compose state. Strange behavior by compose. Lets just get rid of the withContext instead I guess.

Copy link
Sponsor Member Author

@BenjaminHalko BenjaminHalko Nov 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

viewModelScope.launch {
    if (!persistConfiguration) return@launch // TODO: save options for patched apps.

    val packageName = selectedApp.packageName
    state.value = withContext(Dispatchers.Default) { optionsRepository.getOptions(packageName) }
}

Would this work if we wanted to keep withContext? This doesn't cause the crash.

Copy link
Member

Choose a reason for hiding this comment

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

That would work

}

state
Expand Down