-
-
Notifications
You must be signed in to change notification settings - Fork 89
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
Adds dual samples for many sounds #286
Conversation
key result: library size reduced by > 40%
Codecov Report
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
+ Coverage 85.37% 85.55% +0.17%
==========================================
Files 26 29 +3
Lines 1272 1440 +168
Branches 147 172 +25
==========================================
+ Hits 1086 1232 +146
- Misses 89 113 +24
+ Partials 97 95 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Here is an overview of what got changed by this pull request: Issues
======
+ Solved 13
- Added 4
See the complete overview on Codacy |
return; | ||
} | ||
handlePlayerEvent(event: PlayerControlEvent): void { | ||
event.src.forEach((soundKey: string) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codacy found an issue: Arrow function has a complexity of 10. Maximum allowed is 4.
*/ | ||
showIdleStatus(): void { | ||
if (this.isShowingIdleStatus() === true) { | ||
enableStatus(statusID: string, enabled: boolean): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codacy found an issue: Method 'enableStatus' has a complexity of 7. Maximum allowed is 4.
} | ||
|
||
private createIcon(id: string): HTMLElement { | ||
const template = document.createElement("template"); | ||
template.innerHTML = `<svg id="${id}" class="sound"><use xlink:href="${Icons[id]}" /><svg>`; | ||
template.innerHTML = `<svg id="${id}" class="icon"><use xlink:href="${Icons[id]}" /><svg>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codacy found an issue: Unsafe assignment to innerHTML
} | ||
|
||
private createIcon(id: string): HTMLElement { | ||
const template = document.createElement("template"); | ||
template.innerHTML = `<svg id="${id}" class="sound"><use xlink:href="${Icons[id]}" /><svg>`; | ||
template.innerHTML = `<svg id="${id}" class="icon"><use xlink:href="${Icons[id]}" /><svg>`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codacy found an issue: Generic Object Injection Sink
Hi @ashutoshgngwr, first off, thanks a lot for this great app! I noticed that this MR modified I was wondering if this change was deliberate, as according to my understanding this MR should only split effects into separate files to playback intertwinedly, not change the content of the sounds. In case it was an accident, I would be very happy if you could revert the change to this particular effect, as it was one of my favorite effects in the library and in my personal opinion the third tone quite ruins it. Thanks! |
@Lesik it was a deliberate change. Long story short, original sample had 3 beeps (current version) but I trimmed one beep in earlier versions (personal preference). Don't remember why I changed it though. Feel free to submit a PR reverting to old sample. :) |
@ashutoshgngwr I was going to write "if you could accept a MR" but then I realized that many samples (including the seatbelt beeps) have been equalized since (#313). Just reverting the original file would result in an unequalized sample, so it's not that easy for me to submit a MR. Could you document the process you used for equalization? |
Sorry I am caught up somewhere else. Maybe you could add the instructions from that comment in the adding sounds section of |
Changes
Testing
Others
TODO
PlayerManagerStatusEvent
toPlayerManagerStatus