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

Document which button types are pressable #10825

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stepancheg
Copy link
Contributor

No description provided.

@Nilirad Nilirad added C-Docs An addition or correction to our documentation A-Input Player input via keyboard, mouse, gamepad, and more labels Dec 1, 2023
crates/bevy_input/src/input.rs Outdated Show resolved Hide resolved
@rlidwka
Copy link
Contributor

rlidwka commented Dec 2, 2023

KeyCode and ScanCode aren't buttons. "Input types" perhaps?

I suggest to do the same exact changes to Axis<T> struct (which has GamepadButton and GamepadAxis, and that's not documented anywhere either).

It is a bit unfortunate that Input is not self-documenting, ideally we'd have Input<T: InputType> instead of Input<T: Copy + Eq + And + Stuff>, and users could've just looked up which types implement InputType trait. But that's probably gonna break something, so better not.

@stepancheg
Copy link
Contributor Author

ideally we'd have Input<T: InputType>

I wanted to do it, it resulted in strange compilation errors, I suspect because of this dependency cycle:

[dev-dependencies]
bevy = { path = "../../", version = "0.12.0" }

@stepancheg
Copy link
Contributor Author

I suggest to do the same exact changes to Axis struct

Why stop at Axis why not document anything else? There's a lot of things can be documented in bevy.

I don't use Axis and after quick look I don't understand what it does, so I didn't document it. Also PRs are open for everyone, you know.

Copy link
Contributor

@Nilirad Nilirad left a comment

Choose a reason for hiding this comment

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

The generalization from "button types" to "input types" makes it less confusing, I like it.

I agree with @rlidwka that Input<T> and Axis<T> should be more self-documenting, so we don't have to manually maintain a list, with the added benefit of not allowing types that aren't meant to go there but they happen to have the right traits. But that's for another PR since it's more controversial and breaking.

Copy link
Member

@TrialDragon TrialDragon left a comment

Choose a reason for hiding this comment

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

This PR's branch should merge or rebase main since relevant changes, and conflicts, have occurred.

Comment on lines +23 to +27
/// ## Input types
///
/// This crate provides support for the following parameters of `Input`:
///
/// * [`KeyCode`](crate::KeyCode) and [`ScanCode`](crate::ScanCode) for keyboard input
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// ## Input types
///
/// This crate provides support for the following parameters of `Input`:
///
/// * [`KeyCode`](crate::KeyCode) and [`ScanCode`](crate::ScanCode) for keyboard input
/// ## ButtonInput types
///
/// This crate provides support for the following parameters of `ButtonInput`:
///
/// * [`KeyCode`](crate::KeyCode) for keyboard input

Input has since been renamed ButtonInput and ScanCode removed with KeyCode taking it's place as the physical key. (I do not remember if Key, the present logical key, is relevant to ButtonInput at all).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Input Player input via keyboard, mouse, gamepad, and more C-Docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants