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

Create and implement sounds for the Vegas Buttons/ Level Selection Buttons which also may be used as defaults in other sims #89

Closed
Ashton-Morris opened this issue May 12, 2021 · 31 comments
Assignees

Comments

@Ashton-Morris
Copy link

I have created 10 level selection/Vegas buttons based on the solute selection options used in Molarity. Since 10 is the highest number of buttons for "games" I have seen I created 10. They are all different from one and another but are still in the same sound family and sound like they consistently increase as the number gets higher.

Please assign anyone to this issue that you feel is needed.

All of the sounds can be found here.

Related to these issues - #88 (comment)
phetsims/fourier-making-waves#54

  • level-selection-buttons-010.mp3
  • level-selection-buttons-009.mp3
  • level-selection-buttons-008.mp3
  • level-selection-buttons-007.mp3
  • level-selection-buttons-006.mp3
  • level-selection-buttons-005.mp3
  • level-selection-buttons-004.mp3
  • level-selection-buttons-003.mp3
  • level-selection-buttons-002.mp3
  • level-selection-buttons-001.mp3
@pixelzoom
Copy link
Contributor

pixelzoom commented May 12, 2021

While these sounds are needed by Fourier, they are not specific to Fourier. This is a general issue that belongs in vegas. So transferring this issue from fourier-making-waves to vegas.

@pixelzoom pixelzoom transferred this issue from phetsims/fourier-making-waves May 12, 2021
@pixelzoom
Copy link
Contributor

pixelzoom commented May 12, 2021

To evaluate, I need information about the sound design. How did we get here? What were the requirements?

@arouinfar @Ashton-Morris were there meetings or discussions that I missed while I was on vacation? Can you fill me in?

The last time that I was involved in a discussion about sound for Level Selection buttons was Fourier design meeting on 4/22/21. From the meeting notes at #88 (comment):

LevelSelectionButton might be thematically like radio buttons, where each one has a different pitch (increasing by level number). But note that there is currently no "button group" for LevelSelectionButton in vegas, so no way to programmatically assign/increment different pitches.

There was also a question at the time as to whether we needed different sounds for different levels, or whether 1 sound would suffice for all level-selection buttons. So I'm not clear on how we ended up here, where the papertrail of decisions lives, what the design is, or how I'm supposed to evaluate. For example... What is the design for these sounds? Does each button need a different sound? Is it up to the sim to choose which sounds, or is there a standard for each level? Are the sounds supposed to be different enough to be distinguishable? Are higher levels supposed to have higher pitches?

Feedback on these specific sounds:

  • I'm not a fan of approaches that limit us to a specific number of buttons. Reminder that the Java version of Fourier was originally 11 levels. If we need different sounds for each level, an approach that uses generated sounds (based on number of levels) would be more robust and flexible.
  • I like the general theme of the sounds. I prefer the "major chord" sounds, vs the "minor chord" ones (level-selection-buttons-005.mp3, level-selection-buttons-008.mp3).
  • Some of the sounds are too similar to differentiate. For example, level-selection-buttons-001.mp3 and level-selection-buttons-002.mp3 sound almost identical on my MacBook Pro 16". The only way I can confirm that they are different is via an MD5 checksum for the files. If we need different sounds for each level, shouldn't they be easy to distinguish?

Other things I'm not clear on...

Are these 10 sounds that can be assigned to any level, as desired? Or is level-selection-buttons-001.mp3 intended to be the standard sound for Level 1, level-selection-buttons-002.mp3 for Level 2, etc.?

Did implementation considerations inform this sound design? This approach of having 10 mp3 files has some significant implementation consequences. LevelSelectionButton is the common-code button, and (currently) clients create the number of these that they need. If we have 10 different sounds, then clients will need to assign the sounds to each button. That means duplicated code in clients, and the potential to end up with different sound assignments in different sims. Or we'll need to create a "level-selection button group" in vegas, to handle creation of LevelSelectionButtons and sound assignment - a non-trivial piece of work. Using a generated sound assigned that is created by the LevelSelectionButton, based on the level assigned to the button, would be less work, and easier to standardize.

Happy to discuss all of the above, either here or on Slack/Zoom.

@pixelzoom pixelzoom assigned Ashton-Morris and unassigned pixelzoom May 12, 2021
@Ashton-Morris
Copy link
Author

These sounds are intended to be played in order. If a sim has 5 options then they should be played in the numbered order.

@pixelzoom
Copy link
Contributor

pixelzoom commented May 12, 2021

9 of the sounds are 32 kHz. One is different: level-selection-buttons-010.mp3 is 44.1 kHz. I don't know if that's significant, just mentioning in case it was not intentional. If it was intentional, it might be good to mention why (even if it's just in this issue).

@Ashton-Morris
Copy link
Author

@pixelzoom That was not intentional. I have replaced it with this file.

@pixelzoom
Copy link
Contributor

@jbphet pointed me to the documentation for PhET encoding standards, https://github.com/phetsims/tambo/blob/master/doc/encoding-audio.md. It says:

The .wav file should use a sample rate of 44.1 kHz, since it is the most commonly supported frequency for audio contexts, so re-sampling will not be required in most cases.

So it looks like the 32 kHZ files are the ones that need to be changed.

@pixelzoom
Copy link
Contributor

pixelzoom commented May 13, 2021

Questions for 5/13/21 design meeting:

Sound Design:

  • Different sounds for each level? (If the answer is "no", then most of the other questions here are irrelevant, and implementation is trivial.)
    YES

  • Standard sound for each level?
    YES, based on level (and maybe number of levels?)

  • Ability to override the standard sound for a level?
    YES

  • Should each sound be distinguishable from the others?
    YES. Some of the above sounds are too similar.

  • Is it OK to impose an upper limit on the number of levels? If so, what is that limit?
    NO. (Not a hard NO, but not really necessary to have a limit. Similar to radio buttons.)

Implementation:

  • mp3 vs generated sounds - limited number of levels, "range" of sound, etc. (I recommend generated sounds, because there's no inherent limit, and the range of tones can be tailored to the number of levels.)
    Take one recorded sound and change playback rate, like radio buttons.

  • How to integrate sounds? Options:
    (a) Have every sim explicitly add sounds to LevelSelectionButton. (I do not recommend this option. It duplicates code, and is will be difficult to ensure standardization.)
    (b) Add a level number to LevelSelectionButton. Have LevelSelectionButton assign it's own sound based on level number, with the ability to override that via an option. (I recommend this option.)
    (c) Create something analagous to RadioButtonGroup that creates a group of LevelSelectionButton, and handles sound assignment. This will be significantly more expensive, because we'll also have to deal with layout for the group, which varies from sim to sim (1 row, grid, etc.) (I do not recommend this option; it has little advantage over option b.)
    We talked about these, and (b) is probably what we'll do.

@pixelzoom
Copy link
Contributor

pixelzoom commented May 13, 2021

5/13/21 design meeting: @jbphet @arouinfar @kathy-phet @KatieWoe @pixelzoom

We discussed the bullet items in #89 (comment). Answers/conclusions are inlined in #89 (comment) in bold font.

Consensus was that the approach should be similar to radio buttons. Use one base sound, and change the playback rate based on level number.

@jbphet suggested having @Ashton-Morris create some candidates for the base sound, then choose one.

@jbphet @arouinfar @Ashton-Morris will follow up with design meetings.

@Ashton-Morris
Copy link
Author

Putting this on hold until we can discuss it further and clarify during our status meeting on the 27th.

@jbphet
Copy link
Contributor

jbphet commented May 18, 2021

This was discussed in today's sound design meeting and @Ashton-Morris is going to come up with a sound or two that we can then play at different rates for the various level selection buttons. Once he provides this, I'll hook it up and we can review it in a larger design meeting.

@jbphet jbphet removed their assignment May 18, 2021
@Ashton-Morris
Copy link
Author

I have 4 candidates to try.

game-button-001.mp3
game-button-002.mp3
game-button-004.mp3
game-button-003.mp3

@jbphet Can you share this with me once implemented so I can listen to them before you bring it up in the larger design meeting?

@jbphet
Copy link
Contributor

jbphet commented Jun 2, 2021

I've marked this as blocking publication. This is for any sim that uses the level selection buttons, since there will be some untested code in them for a while.

jbphet added a commit to phetsims/scenery-phet that referenced this issue Jun 2, 2021
jbphet added a commit to phetsims/tambo that referenced this issue Jun 2, 2021
@jbphet
Copy link
Contributor

jbphet commented Jun 2, 2021

@Ashton-Morris, @emily-phet, and @arouinfar - I've published a dev version of the vegas demo that includes an "Options" dialog that allows us to choose between different base sounds for the level selection buttons, and I've integrated the four initial candidate sounds that @Ashton-Morris provided (see comments above). Please give it a try and log your feedback in this ticket. The dev version can be found at https://phet-dev.colorado.edu/html/vegas/1.0.0-dev.5/phet/vegas_en_phet.html. The level selection buttons are shown on the first screen of the demo.

@KatieWoe
Copy link

KatieWoe commented Jun 3, 2021

A number of sims are failing with these types of errors on CT:

balancing-act : fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/balancing-act/balancing-act_en.html?continuousTest=%7B%22test%22%3A%5B%22balancing-act%22%2C%22fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1622723211956%22%2C%22timestamp%22%3A1622725839138%7D&brand=phet&ea&fuzz&memoryLimit=1000
Query: brand=phet&ea&fuzz&memoryLimit=1000
Uncaught TypeError: Cannot read property 'value' of undefined
TypeError: Cannot read property 'value' of undefined
at Object.play (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/vegas/js/LevelSelectionButton.js:148:95)
at playSound (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/sun/js/buttons/RectangularPushButton.js:54:51)
at TinyEmitter.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/axon/js/TinyEmitter.js:86:18)
at https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/axon/js/Emitter.js:39:29
at Emitter.execute (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/axon/js/Action.js:227:18)
at Emitter.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/axon/js/Emitter.js:64:19)
at downPropertyObserver (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/sun/js/buttons/PushButtonModel.js:93:36)
at TinyProperty.emit (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/axon/js/TinyEmitter.js:86:18)
at BooleanProperty._notifyListeners (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/axon/js/Property.js:271:23)
at BooleanProperty.set (https://bayes.colorado.edu/continuous-testing/ct-snapshots/1622723211956/axon/js/Property.js:186:14)
id: Bayes Chrome
Snapshot from 6/3/2021, 6:26:51 AM

@samreid suggested it is connected to this issue.

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 3, 2021

Recommended to revert this work in master, and move it to a branch. This blocks publication of any sim that has a game, @jbphet is liable to be unavailable for awhile, and sounds could certainly be evaluated in a branch.

@samreid
Copy link
Member

samreid commented Jun 3, 2021

It is unclear what to move to a branch--instead I put a bandaid on the global access which appears to fix things (tested in balancing act).

@pixelzoom
Copy link
Contributor

pixelzoom commented Jun 14, 2021

For option soundPlayerIndex, it would also be good to note that this index is 0-based. If the client is using 1-based indexing for their levels, they'll need to compensate. Nothing is going to break if they don't compensate, but their levels will all be 1 pitch higher than "standard". This was the case in Fourier, see WaveGameLevelSelectionNode.js:

    // {WaveGameLevelSelectionButton[]} a level-selection button for each level
    const levelSelectionButtons = _.map( model.levels,
      level => new WaveGameLevelSelectionButton( level, model.levels.length, model.levelProperty, {
        soundPlayerIndex: level.levelNumber - 1,
        tandem: options.tandem.createTandem( `level${level.levelNumber}SelectionButton` )
      } )
    );

@jbphet
Copy link
Contributor

jbphet commented Jun 14, 2021

I've added the documentation that @pixelzoom suggested in the previous comment.

@jbphet
Copy link
Contributor

jbphet commented Jun 15, 2021

We discussed the sounds at today's sound design meeting and decided to move forward with game-button-003.mp3. I'll delete the other sounds from the repo and make this one official, and will also remove the ability to select the sound from the options dialog.

@jbphet
Copy link
Contributor

jbphet commented Jun 29, 2021

I have removed the unused sounds and renamed the one that we decided to keep to level-selection-button.mp3.

@Ashton-Morris - Two things:

  • I'll need a .wav version of this sound for the assets directory
  • Please note this name change for your records in case we ever need to manipulate the sound

@jbphet jbphet assigned Ashton-Morris and unassigned jbphet Jun 29, 2021
@Ashton-Morris
Copy link
Author

I have changed the filename in my system. Here is the .wav

level-selection-button.wav

jbphet added a commit that referenced this issue Jun 29, 2021
jbphet added a commit that referenced this issue Jun 29, 2021
@jbphet
Copy link
Contributor

jbphet commented Jun 29, 2021

I've added the asset and we did some level adjustment during today's sound design meeting. I think we're done here. Closing.

@jbphet jbphet closed this as completed Jun 29, 2021
jbphet added a commit to phetsims/expression-exchange that referenced this issue Jul 16, 2021
jbphet added a commit to phetsims/balancing-act that referenced this issue Aug 2, 2021
jbphet added a commit to phetsims/balancing-chemical-equations that referenced this issue Aug 31, 2021
jbphet added a commit to phetsims/area-model-common that referenced this issue Aug 31, 2021
jbphet added a commit to phetsims/build-an-atom that referenced this issue Aug 31, 2021
jbphet added a commit to phetsims/area-builder that referenced this issue Aug 31, 2021
jbphet added a commit to phetsims/equality-explorer that referenced this issue Sep 7, 2021
jbphet added a commit to phetsims/fractions-common that referenced this issue Sep 7, 2021
jbphet added a commit to phetsims/graphing-lines that referenced this issue Sep 7, 2021
jbphet added a commit to phetsims/make-a-ten that referenced this issue Sep 7, 2021
jbphet added a commit to phetsims/reactants-products-and-leftovers that referenced this issue Sep 7, 2021
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

No branches or pull requests

7 participants