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

Add window entity to TouchInput events #11128

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Add window entity to TouchInput events #11128

merged 1 commit into from
Jan 2, 2024

Conversation

NthTensor
Copy link
Contributor

@NthTensor NthTensor commented Dec 29, 2023 •
edited by alice-i-cecile
Loading

Objective

If you have multiple windows, there is no way to determine which window a TouchInput event applies to. This fixes that.

Solution

Migration Guide

  • Add a window field when constructing or destructuring a TouchInput struct.

@github-actions GitHub Actions
Copy link
Contributor

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Input Player input via keyboard, mouse, gamepad, and more labels Dec 29, 2023
@alice-i-cecile alice-i-cecile added this to the 0.13 milestone Dec 29, 2023
@alice-i-cecile alice-i-cecile added the C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide label Dec 29, 2023
@github-actions GitHub Actions
Copy link
Contributor

It looks like your PR is a breaking change, but you didn't provide a migration guide.

Could you add some context on what users should update when this change get released in a new version of Bevy?
It will be used to help writing the migration guide for the version. Putting it after a ## Migration Guide will help it get automatically picked up by our tooling.

matiqo15
matiqo15 reviewed Dec 29, 2023
View reviewed changes
Copy link
Member

@matiqo15 matiqo15 left a comment

Choose a reason for hiding this comment

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

Other than fixing compile errors, it looks good!

@NthTensor
Copy link
Contributor Author

Fixed those tests, sorry about that.

I wonder if it would also be a good idea to make Touches a component of the window entity rather than a resource. Any guidance on including that change in this PR?

@matiqo15
Copy link
Member

I wonder if it would also be a good idea to make Touches a component of the window entity rather than a resource. Any guidance on including that change in this PR?

Not sure about that one, but I believe this would deserve another PR.

@matiqo15 matiqo15 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 29, 2023
@alice-i-cecile
Copy link
Member

Definitely another PR. I kind of like the idea though 🤔

@SecretPocketCat
Copy link
Contributor

I believe this would fix #6011 .

@alice-i-cecile
Copy link
Member

Merging. If this fails, please merge main into your branch, which should pick up the fixes to CI.

auto-merge was automatically disabled January 2, 2024 01:33

Head branch was pushed to by a user without write access

@NthTensor
Copy link
Contributor Author

NthTensor commented Jan 2, 2024 •
edited
Loading

Rebased onto main to fix CI issues and force pushed. Looks like you might need to add this back to the queue.

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 2, 2024
@alice-i-cecile
Copy link
Member

Done, thank you :)

Merged via the queue into bevyengine:main with commit 4034740 Jan 2, 2024
22 checks passed
@NthTensor NthTensor mentioned this pull request Jan 2, 2024
Merged
@matiqo15 matiqo15 mentioned this pull request Jan 3, 2024
Merged
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

@matiqo15 matiqo15 matiqo15 approved these changes

Assignees
No one assigned
Labels
A-Input Player input via keyboard, mouse, gamepad, and more C-Breaking-Change A breaking change to Bevy's public API that needs to be noted in a migration guide C-Bug An unexpected or incorrect behavior 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
0.13
Development

Successfully merging this pull request may close these issues.

Touch input does not report window
4 participants
@NthTensor @matiqo15 @alice-i-cecile @SecretPocketCat