discordjs / voice

Implementation of the Discord Voice API for discord.js and other JS/TS libraries
Apache License 2.0
327 stars 111 forks source link

fix(Events): strengthen new state type for status events #132

Closed skick1234 closed 3 years ago

skick1234 commented 3 years ago

Please describe the changes this PR makes and why it should be merged:

Status and versioning classification:

codecov[bot] commented 3 years ago

Codecov Report

Merging #132 (e949177) into main (7b6ca77) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #132   +/-   ##
=======================================
  Coverage   67.63%   67.63%           
=======================================
  Files          19       19           
  Lines         893      893           
  Branches      218      212    -6     
=======================================
  Hits          604      604           
  Misses        287      287           
  Partials        2        2           
Impacted Files Coverage Δ
src/VoiceConnection.ts 85.49% <100.00%> (ø)
src/audio/AudioPlayer.ts 87.87% <100.00%> (ø)

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 7b6ca77...e949177. Read the comment docs.

kyranet commented 3 years ago

Please lint the code, see CI for more information. @skick1234

skick1234 commented 3 years ago

Could you also do the same thing for AudioPlayer, then lint the code? You can try npm run lint:fix to have them automatically fixed.

I didn't realize AudioPlayer has those kinds of events too :> And linted

amishshah commented 3 years ago

Hmm looks like there are some compilation errors...

skick1234 commented 3 years ago

Hmm looks like there are some compilation errors...

Yep, it just solves typings for listening, not for emitting those .-. I will close this and maybe re-open when I have better solution

skick1234 commented 3 years ago

My solution is

if (newState.status === VoiceConnectionStatus.Connecting) this.emit(newState.status, oldState, newState);
if (newState.status === VoiceConnectionStatus.Ready) this.emit(newState.status, oldState, newState);
...

But this is too long :> that why I simply add as any to the newState parameter. Not the best solution for TS but .-.