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.

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

Inconsistent playback_state configuration behavior #1364

Open
ThomasFrans opened this issue Jan 4, 2024 · 3 comments
Open

Inconsistent playback_state configuration behavior #1364

ThomasFrans opened this issue Jan 4, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@ThomasFrans
Copy link
Contributor

ThomasFrans commented Jan 4, 2024

Describe the bug
When the playback_state value is set and removed in the configuration file, the behavior isn't reverted to before setting the value. Instead, it uses the value stored in the userstate file. This is confusing as most users probably expect the behavior to change back to the one they had before setting the value.

To Reproduce
Steps to reproduce the behavior:

  1. Add playback_state = "Paused" to config.toml
  2. Open ncspot. The playback is paused as expected.
  3. Remove playback_state = "Paused" from config.toml
  4. Open ncspot. The playback is paused as expected.
  5. Add playback_state = "playing" to config.toml
  6. Open ncspot. The playback is resumed as expected.
  7. Remove playback_state = "playing" from config.toml
  8. Open ncspot. The playback is resumed, which isn't expected. It should probably pause like it did in step 4

Expected behavior
When no value is set for playback_state the behavior should go back to the one before any value was set.

System (please complete the following information):

  • OS: Arch Linux
  • Terminal: Alacritty
  • Version: 1.0.0
  • Installed from: Cargo

Additional context
I encountered this while working on #1363 which is now a draft as it makes sense to fix all the playback_state behavior together. I wanted to ask what the intended behavior is. The current one is confusing which makes me think it's not intended. The configuration documentation also isn't clear here and probably isn't correct for the current behavior. It makes it seem like a default value of Paused is assumed when the value is absent, which isn't the case.

@ThomasFrans ThomasFrans added the bug Something isn't working label Jan 4, 2024
@ThomasFrans ThomasFrans changed the title Inconsistent playback_state behavior Inconsistent playback_state configuration behavior Jan 4, 2024
@Bettehem
Copy link
Contributor

Bettehem commented Jan 5, 2024

Yeah indeed, removing the config option should probably revert back to the default behaviour. I think the default behaviour should be to restore the previous playback state the player was in before exiting.

So if for example, the user has a song playing and they close ncspot (without pausing/stopping playback first), when they open ncspot the next time, it should resume playing the song automatically, assuming they haven't changed the default playback_state in the config.

@ThomasFrans
Copy link
Contributor Author

I think the default behaviour should be to restore the previous playback state the player was in before exiting.

I think it would make more sense to mimic the behavior of other audio players: Pause on startup and let the user resume playback. It would be what new users expect. I'm just guessing but I think most users use that behavior, and the other more advanced users can customize the behavior in the configuration file.

@Bettehem
Copy link
Contributor

Bettehem commented Jan 8, 2024

Ok yeah, good point. If that is the norm, ncspot should probably follow it too. I don't really use other players and even ncspot I only mainly use via mpris 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants