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 extra mouse buttons #178

Merged
merged 1 commit into from
Dec 19, 2022

Conversation

rpaladin
Copy link
Contributor

No description provided.

@@ -175,7 +175,7 @@ class Mouse extends VirtualInput {
}

function buttonIndex(button: String): Int {
return button == "left" ? 0 : (button == "right" ? 1 : 2);
return button == "left" ? 0 : button == "right" ? 1 : button == "middle" ? 2 : button == "side1" ? 3 : (button == "side2" ? 4 : 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

A switch statement is probably more readable here. It's not a big deal though, at least not for me :)

Also, should 2 still be the default value if the button name is not in the list of known names? I think we should either return 5 or -1 here (irrespective of introducing a breaking change, which would be pretty minor).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Funny story, I actually used switch/case originally to test my buttons as I enjoy them more as well, then back-ported it to the question-mark & colon combination. To answer your questions:

  1. Lubos implemented the code so I didn't want to change anything that is preferable, nor did I know if using queston-mark/colon versus switch/case had any particular performance boost. It does however have better white space i.e. 1 line vs 14 lines.
  2. 5 would return an extra mouse button, which might raise an error if a user doesn't have that extra mouse button. Better to keep it at 2 (middle mouse) - or 0 (left mouse) if you'd prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, good point 👍

@rpaladin
Copy link
Contributor Author

Hi @ Lubos. Regarding the comments above, do you prefer the current question-mark/colon condition combination or do you prefer a more readable (but also larger whitespace) condition combination of switch/case? If you prefer to keep the condition combination as is, I believe this PR is ready, unless you prefer switch/case, in which case I'll commit the necessary changes to implement that instead.

@luboslenco luboslenco merged commit 68caee5 into armory3d:main Dec 19, 2022
@luboslenco
Copy link
Member

Happy with either one, thanks! Looks like for gamepad it's done in a loop https://github.com/armory3d/iron/blob/main/Sources/iron/system/Input.hx#L717.

@rpaladin rpaladin deleted the add-extra-mouse-btns-iron branch December 21, 2022 00:25
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