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 ad action delay option #263

Merged
merged 6 commits into from
Apr 26, 2021

Conversation

fdarveau
Copy link
Contributor

Enables customization of the delay before playing, pausing or skipping to next track in the ad detection feature.

Can fix an issue where Pause() is called twice in a row if the first one completes too early.

Replaces the hard coded 100ms delay with a default 100ms delay.

@fdarveau
Copy link
Contributor Author

fdarveau commented Jan 13, 2020

I am unsure about the name to use for the option, so feel free to suggest better ones.

I have not fully tested it yet, which is why I marked it as a draft.

@jwallet
Copy link
Collaborator

jwallet commented Jan 14, 2020

I have not fully tested it yet, which is why I marked it as a draft.

Ad Detection

  • Delay between macro steps
  • Macro action steps delay
  • Delay between automated keystrokes

It might be call twice because the app did not have the focus yet, SetForegroundWindowFromHandle() so it still detects sound and send another Pause(), maybe just increasing the Thread.Sleep() before SetForegroundWindowFromHandle() is enough and will be easier to name this option like Tabing delay or Auto app switcher delay

@fdarveau
Copy link
Contributor Author

It might work for Spotify, but if another music app is used (which is my use case), we also need to change the sleep time in the other methods where there is no tabbing involved.

@fdarveau
Copy link
Contributor Author

After a few tests, it seems to work pretty well on my end.

Would it be clearer if the option was named "Media control delay"/"Délai des contrôles de médias"? Or could this potentially be used for OBS/future plans for another ad detection plugin?

@fdarveau fdarveau marked this pull request as ready for review January 26, 2020 03:19
@jwallet
Copy link
Collaborator

jwallet commented Jan 26, 2020

no future plan for addons, this name fits the current state of the code, we will see if this option name still make sense if new addons are added

@fdarveau
Copy link
Contributor Author

Ok. I will push the updated labels/variable names tomorrow.

@jwallet
Copy link
Collaborator

jwallet commented Jan 26, 2020

this is ready for testing. once one of us approves it, this will be merged

@jwallet jwallet merged commit 8bf0f74 into MLBAMGames:master Apr 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants