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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - add documentation on Input #1781

Closed
wants to merge 3 commits into from
Closed

[Merged by Bors] - add documentation on Input #1781

wants to merge 3 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Mar 28, 2021 •
edited
Loading

related to #1700

This PR:

  • documents all methods on Input<T>
  • adds documentation on the struct about how to use it, and how to implement it for a new input type
  • renames method update to a easier to understand clear
  • adds two methods to check for state and clear it after, allowing easier use in the case of triggering state transition from player input locks the game #1700

@mockersf mockersf mentioned this pull request Mar 28, 2021
Closed
@Ixentus
Copy link
Contributor

Ixentus commented Mar 29, 2021

Looks good. I like the _and_clear() methods. Makes the solution to #1700 obvious even when skipping the documentation.

@alice-i-cecile alice-i-cecile added C-Docs An addition or correction to our documentation C-Feature A new feature, making something new possible A-Input Player input via keyboard, mouse, gamepad, and more C-Code-Quality A section of code that is hard to understand or change labels Mar 29, 2021
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Apr 11, 2021
cart
cart reviewed Apr 13, 2021
View reviewed changes
crates/bevy_input/src/input.rs Outdated
/// Check if `input` has been just pressed, and clear it so that it won't be processed further.
pub fn just_pressed_and_clear(&mut self, input: T) -> bool {
self.just_pressed.remove(&input);
self.just_pressed.contains(&input)
Copy link
Member

Choose a reason for hiding this comment

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

This will always return false because we just removed input in the previous line. I'll just fix this now because its easy.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sometime I really miss the 🤦‍♂️ emoji reaction on GitHub... sorry about that one

Copy link
Member

Choose a reason for hiding this comment

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

Haha no worries. This is a class of mistake that we all make 😄

@cart
Copy link
Member

cart commented Apr 13, 2021

I renamed just_x_and_clear to clear_just_x. This makes it more clear that these are subsets of the global clear method, removes the need for _and_, and is closer to std set operation naming conventions like remove(x) -> bool

@cart
Copy link
Member

cart commented Apr 13, 2021

bors r+

bors bot pushed a commit that referenced this pull request Apr 13, 2021
@mockersf @cart
add documentation on Input (#1781)
4c1099a 
related to #1700 

This PR:
* documents all methods on `Input<T>`
* adds documentation on the struct about how to use it, and how to implement it for a new input type
* renames method `update` to a easier to understand `clear`
* adds two methods to check for state and clear it after, allowing easier use in the case of #1700 

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
@bors
Copy link
Contributor

bors bot commented Apr 13, 2021

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title add documentation on Input [Merged by Bors] - add documentation on Input Apr 13, 2021
@bors bors bot closed this Apr 13, 2021
@mockersf mockersf mentioned this pull request Apr 15, 2021
Closed
jihiggins pushed a commit to jihiggins/bevy that referenced this pull request Apr 18, 2021
@mockersf @cart @jihiggins
add documentation on Input (bevyengine#1781)
5e360e0 
related to bevyengine#1700 

This PR:
* documents all methods on `Input<T>`
* adds documentation on the struct about how to use it, and how to implement it for a new input type
* renames method `update` to a easier to understand `clear`
* adds two methods to check for state and clear it after, allowing easier use in the case of bevyengine#1700 

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
JeanMertz
JeanMertz reviewed Apr 25, 2021
View reviewed changes
crates/bevy_input/src/input.rs
/// but only one should react, for example in the case of triggering
/// [`State`] change, you should consider clearing the input state, either by:
///
/// * Using [`Input::just_pressed_and_clear`] or [`Input::just_released_and_clear`] instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wanted to note that these no longer exist with the changes that @cart pushed to this PR (just_pressed_and_clear -> clear_just_pressed).

It might be a good idea to enable rustdoc linting on the CI to catch issues like there?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed those two and another one in #2007

I checked on how to enable rustdoc linting cleanly for our CI, cargo is missing a feature for that, will try to get it. We're also trying to get global dead links check with #1590 but too many errors for now

@mockersf mockersf mentioned this pull request Apr 25, 2021
Closed
bors bot pushed a commit that referenced this pull request Apr 25, 2021
@mockersf
fix dead intra links in doc on Input and Reflect (#2007)
fcf8faf 
fix a few dead links

* Links in `Input` missed a refactor
* `Reflect::downcast` can't use the intra doc link format, as it's not a link to a trait function, but to a function implemented on `dyn Reflect`

noticed in #1781 (comment)
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
@mockersf @cart @ostwilkens
add documentation on Input (bevyengine#1781)
ba6da1e 
related to bevyengine#1700 

This PR:
* documents all methods on `Input<T>`
* adds documentation on the struct about how to use it, and how to implement it for a new input type
* renames method `update` to a easier to understand `clear`
* adds two methods to check for state and clear it after, allowing easier use in the case of bevyengine#1700 

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
@mockersf @ostwilkens
fix dead intra links in doc on Input and Reflect (bevyengine#2007)
9fcef61 
fix a few dead links

* Links in `Input` missed a refactor
* `Reflect::downcast` can't use the intra doc link format, as it's not a link to a trait function, but to a function implemented on `dyn Reflect`

noticed in bevyengine#1781 (comment)
@MinerSebas MinerSebas mentioned this pull request Dec 5, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cart cart cart left review comments

@JeanMertz JeanMertz JeanMertz left review comments

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-Input Player input via keyboard, mouse, gamepad, and more C-Code-Quality A section of code that is hard to understand or change C-Docs An addition or correction to our documentation C-Feature A new feature, making something new possible S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mockersf @Ixentus @cart @JeanMertz @alice-i-cecile