Skip to content

Rename Input to ButtonInput#10859

Merged
alice-i-cecile merged 2 commits intobevyengine:mainfrom
matiqo15:rename-input-struct
Dec 6, 2023
Merged

Rename Input to ButtonInput#10859
alice-i-cecile merged 2 commits intobevyengine:mainfrom
matiqo15:rename-input-struct

Conversation

@matiqo15
Copy link
Copy Markdown
Member

@matiqo15 matiqo15 commented Dec 3, 2023 •
edited
Loading

Objective

Solution

  • Changed the name of Input struct to PressableInput.
  • Changed the name of Input struct to ButtonInput.

Migration Guide

  • Breaking Change: Users need to rename Input to ButtonInput in their projects.

@ItsDoot
Copy link
Copy Markdown
Contributor

ItsDoot commented Dec 3, 2023

What about DigitalInput instead? Slightly more succinct.

@ItsDoot ItsDoot added 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 Dec 4, 2023
@Nilirad
Copy link
Copy Markdown
Contributor

Nilirad commented Dec 4, 2023

What about DigitalInput instead? Slightly more succinct.

Would BinaryInput be more immediate? 🤔

@Nilirad Nilirad added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Dec 4, 2023
@matiqo15
Copy link
Copy Markdown
Member Author

matiqo15 commented Dec 4, 2023

Would BinaryInput be more immediate? 🤔

I think DigitalInput would be less misleading.

@alice-i-cecile
Copy link
Copy Markdown
Member

alice-i-cecile commented Dec 4, 2023

I like Pressable much better: it more clearly captures the intent and is more approachable for beginners.

@alice-i-cecile
Copy link
Copy Markdown
Member

alice-i-cecile commented Dec 4, 2023

I would like a Pressable marker trait to go with this eventually, but that can wait until it's better motivated. See Leafwing-Studios/leafwing-input-manager#321 for the broader context there.

@stepancheg
Copy link
Copy Markdown
Contributor

stepancheg commented Dec 4, 2023

Maybe ButtonInput. Not precisely correct, but conveys the meaning. My 2 cents.

@Nilirad
Copy link
Copy Markdown
Contributor

Nilirad commented Dec 5, 2023

I would like a Pressable marker trait to go with this eventually, but that can wait until it's better motivated. See Leafwing-Studios/leafwing-input-manager#321 for the broader context there.

Do you mean something like #10855?

@ickshonpe
Copy link
Copy Markdown
Contributor

ickshonpe commented Dec 6, 2023

Maybe ButtonInput. Not precisely correct, but conveys the meaning. My 2 cents.

Much prefer this, PressableInput is a bit ugly and DigitalInput is too obscure.

@alice-i-cecile
Copy link
Copy Markdown
Member

alice-i-cecile commented Dec 6, 2023

Let's go with ButtonInput then and get this merged :)

@matiqo15 matiqo15 force-pushed the rename-input-struct branch from 7d43744 to accff09 Compare December 6, 2023 15:45
@alice-i-cecile alice-i-cecile changed the title Rename Input to PressableInput Rename Input to ButtonInput Dec 6, 2023
Nilirad
Nilirad suggested changes Dec 6, 2023
View reviewed changes
Copy link
Copy Markdown
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.

input.rs should be renamed. For the rest, it looks ok to me.

@matiqo15 matiqo15 requested a review from Nilirad December 6, 2023 18:24
@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 Dec 6, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 6, 2023
Merged via the queue into bevyengine:main with commit 1f97717 Dec 6, 2023
@Shute052 Shute052 mentioned this pull request Feb 3, 2024
Closed
3 tasks
@Shute052 Shute052 mentioned this pull request Feb 19, 2024
Merged
1 task
@ChristopherBiscardi ChristopherBiscardi mentioned this pull request Feb 22, 2024
Closed
@ppoitier ppoitier mentioned this pull request Nov 26, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

@ItsDoot ItsDoot Awaiting requested review from ItsDoot

@ickshonpe ickshonpe Awaiting requested review from ickshonpe

+1 more reviewer

@Nilirad Nilirad Nilirad approved these changes

Reviewers whose approvals may not affect merge requirements

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 M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide 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.

Input name is too generic

6 participants

@matiqo15 @ItsDoot @Nilirad @alice-i-cecile @stepancheg @ickshonpe