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

Add play sound message #370

Open
wants to merge 3 commits into
base: kotlin-experiments
Choose a base branch
from

Conversation

tlf30
Copy link
Contributor

@tlf30 tlf30 commented Nov 6, 2017

I know that kotin-experiments is closed to new features. I am opening this for reference until kotlin-experiments is merged into master.

~Trevor

@garyttierney garyttierney self-requested a review November 11, 2017 23:10
*
* @param id The id of the sound to play.
* @param type The type of the sound to play.
* @param delay The delay before the client plays the sound
Copy link
Contributor

Choose a reason for hiding this comment

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

What unit is the delay measured in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe ticks

* Creates a new send play sound message.
*
* @param id The id of the sound to play.
* @param type The type of the sound to play.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know what the different types are?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So far all the sounds I have used are using type 0

@garyttierney
Copy link
Contributor

Could you update the documentation about the two things mentioned (delay in ticks, type usually 0). Besides that LGTM.

@tlf30
Copy link
Contributor Author

tlf30 commented Nov 14, 2017

@garyttierney I looked at the client I have a little. The delay is in milliseconds. The type I am not sure of though. It is passed to a function in the SoundTrack class called mix(int loop). I played several sounds doing a test of type 0, the of type 1. I could not hear any difference.

@garyttierney
Copy link
Contributor

Ok. If the type is always 0 we should add a constructor that uses 0 as a default value.

@Major-
Copy link
Member

Major- commented Jan 27, 2018

Aren't there two different frames to play (different types?) of sound

@garyttierney
Copy link
Contributor

There's music and there's sound effects IIRC.

@tlf30
Copy link
Contributor Author

tlf30 commented Aug 31, 2018

If someone can confirm which of the two channels are which, i.e channel 1 is music and 2 is sound effects. That is an example, I have no evidence for that. Then, I will work on adding sounds once kotlin experiments is merged. I have a list of sound effects and what they go to.

@tlf30
Copy link
Contributor Author

tlf30 commented Sep 4, 2018

OK, I believe that channel 0 is sound effects and channel 1 is music.
No idea, music is on its own channel

@tlf30 tlf30 force-pushed the add-play-sound-message branch from 781c3eb to 828cf12 Compare September 4, 2018 22:00
@codecov-io
Copy link

Codecov Report

Merging #370 into kotlin-experiments will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@                   Coverage Diff                    @@
##             kotlin-experiments     #370      +/-   ##
========================================================
- Coverage                 23.16%   23.12%   -0.04%     
  Complexity                  807      807              
========================================================
  Files                       643      645       +2     
  Lines                     11131    11146      +15     
  Branches                   1637     1637              
========================================================
  Hits                       2578     2578              
- Misses                     8268     8283      +15     
  Partials                    285      285
Impacted Files Coverage Δ Complexity Δ
...game/release/r377/SendPlaySoundMessageEncoder.java 0% <0%> (ø) 0 <0> (?)
.../java/org/apollo/game/release/r377/Release377.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...apollo/game/message/impl/SendPlaySoundMessage.java 0% <0%> (ø) 0 <0> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0c78ce...828cf12. Read the comment docs.

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.

4 participants