joshwcomeau / redux-sounds

Middleware for playing audio / sound effects using Howler.js
MIT License
131 stars 17 forks source link

Add mute functionality #3

Closed tikhonbelousko closed 7 years ago

tikhonbelousko commented 7 years ago

In our app really miss this option. I'm planning to help with this one and probably create a pull request soon, but I would like to discuss it first.

The way I see it is to have ActionCreators which are distributed together with redux-sounds. An example of it can be ActionCreators.mute(). Later on, we can extend it with something like ActionTypes.stop(id) and ActionCreators.setVolume(). An example of it can look like this:

import { ActionCreators } from 'redux-undo';

store.dispatch(ActionCreators.mute())

We can also make it possible to override action types to allow usage of custom action creators. Something like this:

const loadedSoundsMiddleware = soundsMiddleware(soundsData, {
  muteType: 'MyApp/MUTE'
  // ... etc
});

This approach is successfully used in redux-undo. What do you think?

joshwcomeau commented 7 years ago

Ah! Sorry for the delay D: I did see this issue but I was distracted at the time, and it fell off my radar.

So, I don't really like the idea of having explicit actions because, as I see it, sounds are often a side-effect of the other thing you're doing. For example, I don't think you should have a playSound action, because at least in the cases I've used it in, that's not what the user is doing. I imagine it more that the action is submitContactForm, and the action just happens to play a "whoosh" sound effect. Philosophically I like to treat Redux actions as objects describing the user's interactions, whenever possible.

(I realized, after thinking about it for a bit, that adding a "mute" button to your app would make it so that yes, the user IS trying to mute the sound and it isn't a side effect of some other action. Because this isn't always the case, though, I'd still like to avoid explicit app-provided actions)

It doesn't bother me as much in redux-undo because in redux undo the user IS trying to undo some stuff, so the user behaviour aligns with the application logic.

For adding mute functionality, my first thought was something like this:

const muteSounds = {
  type: MUTE_SOUNDS,
  meta: {
    // booleans would mute all sounds:
    mute: true, 
    // Alternatively, maybe you could specify one or more keys to stop specific sounds?
    // mute: 'jump',
    // mute: ['game.jump', 'game.jingle'],
  }
}

I realized that the meta object might be used by other middlewares, so I should be doing my best to reduce how much I pollute the object. I also do like the idea of adding things like stop and setVolume, which would worsen the issue. So maybe we should rename sound to reduxSound, and pass it an object of the things you'd like to trigger:

const playJumpSoundAction = {
  type: PLAY_JUMP_SOUND,
  meta: {
    reduxSound: {
      play: 'jump',
      stop: 'jump',
      mute: 'jingle',
    },
  },
}

In terms of actually implementing this stuff, the goal was to delegate as much to Howler as possible. Howler should natively support all of this, so it should be pretty straightforward (for example, I wouldn't worry about validating whether a sound is currently playing when you try to stop it; I would just invoke the right howler method and let it throw any errors if it doesn't like what the user is trying to do)

tikhonbelousko commented 7 years ago

Yeah, I see your point about meta. While thinking more about the problem I came across few difficulties. How do I know the state of all sounds like:

It led me to the thought that I will have to copy a little of Howler state to the store. Right now I'm continuing to experiment with the Howler and do everything inside of thunks and reducers, also replicate howler's sub-state inside of the store (quite verbose but works). However, I liked the way you did it with meta, although I was not able to decide how to answer questions above.

If explaining the problem by example then the question is how do I implement a checkbox represents if the sound is muted or not and how do I persist its state between page reloads.

🤔

joshwcomeau commented 7 years ago

Yeah, it feels kind of unfortunate to need to duplicate that stuff into state, but I think you'd need to for your usecase.

I do worry about out-of-sync issues, but I don't really see a way around it.

The good news is, I think a middleware like ReduxSounds is still the best way to solve this problem; you can create a sounds reducer that tracks which ones are playing at which volume, and the actions have the requisite meta properties to trigger the side effects.

I don't know if you've had a chance to peek at the source yet, but it's really tiny, it's really just a thin layer over Howler. So if I were you, I'd copy/paste the source into your repo so you can fiddle with it and test the functionality you need. If it winds up working out, then I'd certainly appreciate a PR :) but it's not required.

tikhonbelousko commented 7 years ago

Thanks for your advice. 👍 I was basically thinking the same. I will close the issue then. If I find myself a good solution I will definitely make a PR.