Add Random Interval #62
Conversation
Added a new GroupBox to hold the Randomized Click Interval UI components NOTE: this commit causes some issues with the horizontal spacing for some reason. Will attempt to fix these issues in a later commit
Bumped the max window dimensions to hold the new UI components
Created necessary UI components to set the maximum and minimum intervals NOTE: I didn't update the bindings yet so all three inputs are currently bound to the same variable
Added new properties to the AutoClickerSettings class for use with the new Randomized Interval functionality Also added the binds to the UI components and updated SetApplicationSettings
Implemented necessary interval logic to get a working randomized interval NOTE: the new implementation assumes that the standard interval will be 0 (null) otherwise the standard interval will be used instead. i.e. the standard interval has a higher precedence than the randomized interval so the standard interval must be 0's to use the randomized interval
|
Just discovered a bug. The interval is not reset after the mouse click action in the current implementation. I need to move the interval update logic into the OnClickTimerElapsed function |
I like this idea. It should possibly be part of this PR because without an indication in the UI, it's a bit more confusing to get expected results from this application. |
I agree. I also think that the horizontal spacing issue should be fixed before this PR is merged. Unforunately I have no experience with XAML so I htink someone else would be better equiped to add these features |
|
@bnn678 First of all, thanks for taking the time of making a PR and putting effort into this, it's really appreciated. 🙏🏽
Regarding all other things discussed, take your time. That's why drafts are for :) |
|
May I suggest a simpler implementation? |
|
@KronkIV I considered using the existing Click Interval as a portion of the input, but I ultimately decided against because it is more ambiguous. Without additional UI information, it's not inherently obvious to a user what each of the components would be for. I believe that the current UI implementation makes it instantly obvious how the Randomized Click Interval should be used. @oriash93 Thanks! I plan to update this PR over the time I have off around Christmas. |
I replaced the previous CalculateInterval with 3 functions (CalculateInterval, CalculateFixedInterval, and CalculateRandomizedInterval) as suggested in the PR comments.
Added an overload of IsIntervalValid that allows users to pass in the interval so that they can store the result of CalculateInterval if desired.
I added the code that should disable the Maximum Interval input whenever the Click Interval has a non-zero value however the disabled state of the input will not update. As I lack experience with XAML I'm going to pass this off to other collaborators on the PR
|
@IdiosApps @oriash93 I have attempted to disable the inputs at an appropriate time, but unforunately I can't figure out why the disabled state of the input won't update after the program is stared. Does anyone have suggestions? |
| @@ -122,16 +122,24 @@ | |||
| </Grid.ColumnDefinitions> | |||
|
|
|||
| <TextBox Grid.Column="0" Width="45" | |||
| Text="{Binding AutoClickerSettings.MaximumHours, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}"/> | |||
| Text="{Binding AutoClickerSettings.MaximumHours, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}" | |||
| IsEnabled="{Binding AutoClickerSettings.IsRandomizedIntervalEnabled, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}" | |||
There was a problem hiding this comment.
@bnn678 Also consider using "IsReadOnly" instead of "IsEnabled"
There was a problem hiding this comment.
What is the difference between the two?
There was a problem hiding this comment.
In the context of a TextBox, IsReadOnly allows the user to set focus to and select and copy the text, but not modify it.
A disabled TextBox does not allow any interaction whatsoever.
As a general rule, use IsReadOnly when you have data that you want the user to see and copy, but not modify.
Use a disabled textbox when the data you are displaying is not applicable for the current state of a dialog or window.
There was a problem hiding this comment.
I'm leaning toward rescoping this ticket and removing the textbox disabling since I can't get the binding to update. Then a later PR can disable the textboxes when appropriate.
Gotcha. Yea, this seems like a good idea. I have made this update.
Actually, I made this update and the IsReadOnly property apparently doesn't grey out the textbox (for some reason I thought it would). I think we want to make it very obvious that using the standard interval means that the randomized interval is disregarded so I believe the IsEnabled property is still the better choice. If you are satisfied with this justification, please resolve the conversation as approval.
|
@bnn678 Are you still working on this? |
I'm not sure why the PropertyChanged trigger was not behaving correctly but by explicitly calling \OnPropertyChanged, I have was able to get the textboxes to respond appropriately.
The previous commit actually depends on these files which I forgot to include in the commit. Here they are now :)
|
Here is what the interface looks like when the Click Interval is set to 0
And here is the appearance when there is a non-zero Click Interval
In the current implementation, the IsEnabled property is used instead of the IsReadOnly property because the "grey-out" effect makes it more obvious to users that the randomized interval is disregarded if a standard interval is provided. |
|
Probably should have checked existing PRs as I just opened one for this. My only gripe is that a real human clicking so fast, produces something closer to a normal distribution, rather than a uniformly distributed random range. If I was reviewing someone's click action/statistics and saw a perfectly uniform random distribution for some duration of time that is neatly windowed, it'd be a huge red flag. A lot of human action/response times tend to be closer to normal, skewed, and multimodal distributions. Also curious, have you noticed whether the new logic adds considerable delays, possibly requiring compensation, to achieve same CPS w/o the added logic? |
Thats a very intereting point and something I hadn't considered.
I haven't noticed any delays, but I also ddn't do any actual performance testing. I can make no guarenteed promises, but it appears to perform reasonably. |
|
Great work! |
|
Updated the UI to include a new section that allows users to specify a maximum and minimum interval.
To use the randomized interval, simply set a value in the maximum and minimum input boxes where the sum of the maximum values is greater than than the minimum values.
NOTE: The randomized interval will only be used if the Click Interval is set to 0. If any value is entered in the Click Interval, the Click Interval will be used instead
Desired Future Updates
Add some indication in the UI that only the Click Interval or Randomized Click Interval can be used (not both). Possibly disable the input box when either has a non-zero value?This was added using the IsRandomizedIntervalEnabled AutoClickerSetting, an explicit call to OnPropertyChanged, and the IsEnabled property.