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

feat: movemouse-speed #490

Merged
merged 19 commits into from
Jul 21, 2023
Merged

feat: movemouse-speed #490

merged 19 commits into from
Jul 21, 2023

Conversation

ArijanJ
Copy link
Contributor

@ArijanJ ArijanJ commented Jul 18, 2023

Closes #488
Adds a movemouse-speed <percentage> action which somewhat hackily makes the mouse go at a different speed, at runtime.
I did some dogfooding and I think the math checks out.

@ArijanJ
Copy link
Contributor Author

ArijanJ commented Jul 18, 2023

I tried modifying distance instead, and while it works perfectly for movemouse, it's very wonky for movemouse-accel:

distance = *distance * (self.move_mouse_speed / 10) / 10,

Unfortunately copy-pasting this line for min and max_distance yields pretty weird results.

parser/src/cfg/mod.rs Outdated Show resolved Hide resolved
src/kanata/mod.rs Outdated Show resolved Hide resolved
@rszyma
Copy link
Contributor

rszyma commented Jul 18, 2023

This case that is not handled correctly:

  1. press any movemouse-* action
  2. press 1st movemouse-speed action
  3. press 2st movemouse-speed action
  4. release both movemouse-speed actions
  5. the original movemouse-* action interval is different from the starting one and, until the key is released.

@rszyma
Copy link
Contributor

rszyma commented Jul 18, 2023

movemouse-speed can increase movemouse-* interval above 65535 (u16 limit) which will cause panic

thread '<unnamed>' panicked at 'attempt to multiply with overflow', src/kanata/mod.rs:973:51

however it would require having absurdly high movemouse-* interval like 660 in and 1% in movemouse-speed

@ArijanJ
Copy link
Contributor Author

ArijanJ commented Jul 18, 2023

Alright, I think the only thing left to do is to restore self.move_mouse_speed correctly on the MoveMouseAccel CustomEvent::Release event, which I'm not too sure can be done with the current math (right now it's self.move_mouse_speed = 100;, which is not correct in your case of pressing two movemouse-speed actions. A solution could be inserting the previous speed into a hashmap on the Press event, and then reverting that from the Release event with the speed argument as the key.

src/kanata/mod.rs Outdated Show resolved Hide resolved
@rszyma
Copy link
Contributor

rszyma commented Jul 18, 2023

A solution could be inserting the previous speed into a hashmap on the Press event, and then reverting that from the Release event with the speed argument as the key.

This is a decent idea, however I'd suggest to use a list instead of a hashmap. Hashmaps/sets can't have duplicate keys (assuming you wanted to use the hashmap as set) while we need to handle a case where 2 same values are added.

@ArijanJ ArijanJ marked this pull request as draft July 18, 2023 22:14
@ArijanJ
Copy link
Contributor Author

ArijanJ commented Jul 19, 2023

This way makes tons more sense and gets rid of all the previous issues we had for free. I couldn't get it to panic at all through my limited testing, plus it isn't choppy anymore due to high interval values.

src/kanata/mod.rs Outdated Show resolved Hide resolved
@rszyma
Copy link
Contributor

rszyma commented Jul 19, 2023

Besides the comment above, this PR looks good to me now.

@ArijanJ ArijanJ marked this pull request as ready for review July 19, 2023 13:11
@ArijanJ
Copy link
Contributor Author

ArijanJ commented Jul 19, 2023

I wonder if this should also apply to the scroll events?

@rszyma
Copy link
Contributor

rszyma commented Jul 19, 2023

I agree with the idea of having a speed modifier for scroll, but I think it should be a separate action, because someone might have use case where they want only scroll or only mouse movement speed affected at a time.

@ArijanJ
Copy link
Contributor Author

ArijanJ commented Jul 19, 2023

Yeah, although I don’t think that’s in the scope of this particular PR.

docs/config.adoc Outdated Show resolved Hide resolved
src/kanata/mod.rs Outdated Show resolved Hide resolved
src/kanata/mod.rs Outdated Show resolved Hide resolved
src/kanata/mod.rs Outdated Show resolved Hide resolved
Co-authored-by: jtroo <j.andreitabs@gmail.com>
Copy link
Owner

@jtroo jtroo left a comment

Choose a reason for hiding this comment

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

Nearly there! Thanks for doing all of the revisions so far.

Only some minor textual suggestions for clarity/accuracy- can take them, leave them, or use different text for clarity.

Other than that, please fix the formatting with cargo fmt --all and fix the clippy errors.

docs/config.adoc Outdated Show resolved Hide resolved
parser/src/cfg/mod.rs Outdated Show resolved Hide resolved
parser/src/cfg/mod.rs Outdated Show resolved Hide resolved
ArijanJ and others added 4 commits July 21, 2023 10:26
Co-authored-by: jtroo <j.andreitabs@gmail.com>
Co-authored-by: jtroo <j.andreitabs@gmail.com>
Co-authored-by: jtroo <j.andreitabs@gmail.com>
@jtroo jtroo merged commit d5ce74e into jtroo:main Jul 21, 2023
3 checks passed
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.

Feature request: Variable movemouse speed
3 participants