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

Allow Setting Detection Delay #14

Merged
merged 9 commits into from
Jun 14, 2023
Merged

Conversation

DiegoxK
Copy link
Contributor

@DiegoxK DiegoxK commented Jun 6, 2023

This pull request addresses issue #12 and #10 by introducing a configurable detection delay for the extension. Users can now set a delay between a range to control the frequency at which the extension reacts to errors and voice lines are played.

I recommend a minimum cooldown duration of 600000 milliseconds (10 minutes) and a maximum of 3600000 (1 hour) for really unexpected remainders of your failures c:

@89netraM 89netraM mentioned this pull request Jun 8, 2023
src/ErrorTracker.ts Outdated Show resolved Hide resolved
@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

I have tested the changes, and setting both the maximum and minimum values to 0 produces the default behavior where the voices can be heard for each error

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.

Looks good to me. I've got the same issue with running it on my PC, but if it sounds good to you then 👍

@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

Thank you! I believe I encountered an error with npm while compiling the speaker module, which was related to the installation of Visual Studio Code's C++ via the Visual Studio Installer

@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

Tested Volume and Delay changes after merged and working fine

@89netraM
Copy link
Owner

I'm terrible sorry, I feel so negative, but the package-lock.json file was lost in the merge.

@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

Yeah, I can see. Don't worry, you're not being negative at all. Let me check and upload the package.json uwu

@DiegoxK
Copy link
Contributor Author

DiegoxK commented Jun 14, 2023

it should be uploaded now, I'm so sorry as well. I'm kinda new to collaborating to open source

@89netraM
Copy link
Owner

No worries, all's good! I'm kinda new too.

@89netraM 89netraM merged commit a5977ef into 89netraM:master Jun 14, 2023
@NinjaZMY
Copy link

@89netraM still not working for me, I'm using windows by the way
this might be due to the code becoming obsolete out-dated

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.

3 participants