-
Notifications
You must be signed in to change notification settings - Fork 127
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 Reflect and FromReflect to UserInput #331
Conversation
…etitSet; so, newtyped as InputChord.
Definitely in favor of having these traits implemented! I'm actually planning on dropping petitset as a dependency, in favor of a simple hash or btree map in #321. As a result, I'm a bit reluctant to merge this in the current form, but I'm open to suggestions on the best way to get to where we're hoping to be. |
Hi @alice-i-cecile ! Thanks for looking. I think the same primary issue arises regardless of what form In the suggested link, if Would it be better to walk back the chord implementation here and just grab the easy derives on everything else for now? |
Good analysis: I was thinking of using one of the blessed types here :) If you're interested in moving this forward faster, I'd be happy to merge a PR migrating to hash maps, and then we can rebase and merge this? |
I can do that! |
Fixing as part of #400. |
Yep, not a problem at all :) That PR has been really tough. |
* Fix typo * Update release notes * Straightforward migration * Remove useless test for from impl * Just use a Vec for chords :( * Clone because our API is jank * Hash all the types * Repair tests after swapping to a Vec * Simple fixes for binding_map * Remove binding_menu egui example due to maintenance burden * Clippy * Fix idempotency of insertion * Remove brittle serde test * Simple doc test fixes * Remove outdated references to 16 max input maps * Implement Reflect for InputMap Closes #386. * Reflect UserInput Closes #331 * Simple fix for doc test * Fix mismatched HashMap types in doc test * Remove example using removed API * Switch `A::variants()` to `self.iter()` (#411) * Update for Bevy 0.12 (#408) * Update to git * forgot a configure_set * Update examples * Update Bevy to 0.12 release * Bump versions * Disable failing test * Remove bevy_egui so we can ship * Update release notes --------- Co-authored-by: Tomato <[email protected]> Co-authored-by: Alice Cecile <[email protected]> * Add back `bevy_egui` to LWIM 0.11 (#409) * Update to `bevy_egui` 0.23 for Bevy 0.12 * Bump to 0.11.1 * Update RELEASES.md * Switch `A::variants()` to `self.iter()` --------- Co-authored-by: Alice Cecile <[email protected]> Co-authored-by: Alice Cecile <[email protected]> --------- Co-authored-by: Tomato <[email protected]> Co-authored-by: Alice Cecile <[email protected]>
Related to the contributing guidelines: I note that the
dev
branch is very stale and I couldn't find any recent PRs against it, so I made this againstmain
. I can adjust this if necessary.General
This PR adds
Reflect
andFromReflect
toUserInput
via adding it to the entire chain of types under its variants that also needed them. It was mentioned in a few places that this was desired, notably in #308, and I also would like to see this. (It kinda feels like any type that is intended to be a bevy component should have them by default and not having them be opt-out, but I digress.)PetitSet ...
The big hang up here was that
PetitSet
does not implement either of said traits. I naively newtype-d it and delegated a majority of its features, but I would imagine there could be a need to have more careful thought put into this. It currently is just a poor veneer overPetitSet
PetitSet
is currently exposed as API surface in a couple of places; so, this is kind of a big change. I can imagine a world where its not, though, and that the functionality of an input chord is much more pared down, or perhaps looks entirely different than a front forPetitSet
, and this would perhaps force that thought into the light earlier than expected.A perhaps less obtrusive approach would be to only do this behind the scenes and maintain
PetitSet
as the forward API. You all own both crates so I defer all decisions!Last thing on the newtype: it's added to a separate module, but reexported through
user_input
-- this was entirely a "idk lmao" decision.