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 Volume Controller #13

Merged
merged 3 commits into from
Jun 14, 2023
Merged

Add Volume Controller #13

merged 3 commits into from
Jun 14, 2023

Conversation

DiegoxK
Copy link
Contributor

@DiegoxK DiegoxK commented Jun 6, 2023

This pull request addresses issue #4 by adding a volume control feature to the extension. Users can now adjust the volume of the voice lines played by the extension.

@89netraM
Copy link
Owner

89netraM commented Jun 8, 2023

First off, thank you for these PRs! This comment addresses both #13 and #14.

The code updates seem fine at first glance, however before taking a closer look I would like to ask you to fix the formating. Notice that a lot of extra lines has been changed (see the diff) due to whitespace changes.
I would also like you to change the defaults to align with the current behaviour, set default volume to 1.0, and default delay to exactly 0 ms.

@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

Got it, i forgot to deactivate the default usage of prettier and that formatted all the files by default, sorry.
Let me work on it

@DiegoxK DiegoxK reopened this Jun 14, 2023
@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

Okay, I believe this branch now only includes the necessary changes made to Player.ts and package.json, along with a default volume of 1.

Copy link
Owner

@89netraM 89netraM left a comment

Choose a reason for hiding this comment

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

Have you tested these changes?

Can you also commit the package-lock.json file. This will help when building in the future.

src/Player.ts Outdated Show resolved Hide resolved
src/Player.ts Show resolved Hide resolved
@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

I have tested the changes, and setting the volume to values above 15 produces very loud sounds, while setting a volume of 0.01 produces very low sounds, which is working as expected.

Copy link
Owner

@89netraM 89netraM left a comment

Choose a reason for hiding this comment

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

Great! Because I can't get it to install on my PC right now (some npm issue), but I'll trust you that it works 👍

Great work! Thank you!

@89netraM 89netraM merged commit ec4acef into 89netraM:master Jun 14, 2023
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.

2 participants