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

Port volume booster to web/vesktop #2730

Merged
merged 42 commits into from
Sep 1, 2024
Merged

Conversation

sadan4
Copy link
Contributor

@sadan4 sadan4 commented Jul 27, 2024

resolves: #1944
ports volume booster to web/vesktop

TODO:

  • don't error on screenshare
  • adjust volume?
  • fix deafening

Copy link
Contributor

@Sqaaakoi Sqaaakoi left a comment

Choose a reason for hiding this comment

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

works but idk if it really is 200% volume sounds like the "fix" you told me you made where it was 2x the volume it was supposed to be is actually what is intended

tl;dr: i think your fix broke it more

@Covkie
Copy link
Contributor

Covkie commented Jul 28, 2024

It does raise the volume but it doesnt seem to be ex: 500% louder.

also it basically functions as a fake deafen at the moment.,...

@Sqaaakoi
Copy link
Contributor

It does raise the volume but it doesnt seem to be ex: 500% louder.

yeah this was my experience too

also it basically functions as a fake deafen at the moment.,...

we cannot ship that, theres a good reason why fake deafen is explicitly not a feature, regardless of that I think I'd still like to have a "I want my friends to be quiet for a bit" button

@sadan4
Copy link
Contributor Author

sadan4 commented Jul 29, 2024

also it basically functions as a fake deafen at the moment.,...

somehow I didn't notice this, thanks!

@sadan4 sadan4 marked this pull request as ready for review August 1, 2024 19:46
@Nuckyz
Copy link
Collaborator

Nuckyz commented Aug 2, 2024

You are doing two webpack searches yet only one is used, remove the other one

also the code style doesn't match the rest of project, mainly the spacing in if conditions

src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
@sadan4
Copy link
Contributor Author

sadan4 commented Aug 2, 2024

You are doing two webpack searches yet only one is used, remove the other one

also the code style doesn't match the rest of project, mainly the spacing in if conditions

I may be a tad stupid, but what are/were you referring to by the spacing in if conditions

Copy link
Owner

@Vendicated Vendicated left a comment

Choose a reason for hiding this comment

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

like nuckyz said, follow the code style

src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
src/plugins/volumeBooster/index.ts Outdated Show resolved Hide resolved
sadan4 and others added 2 commits August 3, 2024 16:12
Co-authored-by: v <vendicated@riseup.net>
Co-authored-by: v <vendicated@riseup.net>
@ZirixCZ ZirixCZ mentioned this pull request Aug 4, 2024
11 tasks
@Vendicated
Copy link
Owner

i think it would be a good idea to extract this into a separate plugin like WebExtraVolume and make it enabled by default. That way more users can profit from it and Vesktop users will get the expected stock equivalent behaviour

@sadan4
Copy link
Contributor Author

sadan4 commented Aug 15, 2024

i think it would be a good idea to extract this into a separate plugin like WebExtraVolume and make it enabled by default. That way more users can profit from it and Vesktop users will get the expected stock equivalent behaviour

It depends on the patches already introduced by Volume Booster.

@sadan4 sadan4 requested a review from Nuckyz August 23, 2024 06:02
@MaxBosse
Copy link

Is there anything I can help with to get this merged? :) Would love to have this feature on web/vesktop

@sadan4
Copy link
Contributor Author

sadan4 commented Aug 30, 2024

This should be ready as i fixed the stock discord issue that made it glitch out at low volumes

@Nuckyz Nuckyz enabled auto-merge (squash) September 1, 2024 03:07
@Nuckyz Nuckyz merged commit e07a4e1 into Vendicated:dev Sep 1, 2024
1 check passed
@Nuckyz
Copy link
Collaborator

Nuckyz commented Sep 1, 2024

Thank you!

@sadan4 sadan4 deleted the portVolumeBooster branch September 1, 2024 03:11
@sadan4
Copy link
Contributor Author

sadan4 commented Sep 1, 2024

Apparently this isn't working on the vesktop, which is weird because that's where I tested it. I have family over today, but I will work on it tomorrow if nobody else fixes it

@sadan4 sadan4 restored the portVolumeBooster branch September 1, 2024 15:47
josiauh pushed a commit to josiauh/Vencord that referenced this pull request Sep 28, 2024
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.

6 participants