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

Implement Reflect for InputMap #386

Closed
wants to merge 5 commits into from

Conversation

barsoosayque
Copy link
Contributor

Currently it's not possible to directly use an InputMap<A> as an asset (e.g. Handle<InputMap<A>>), as trait Asset now requires TypePath.

@alice-i-cecile
Copy link
Contributor

alice-i-cecile commented Sep 15, 2023

Can you swap this to a Reflect impl instead? It will have the same effect, but is much more powerful.

Ideally we do the same thing for all of our component and resource types actually, but that doesn't have to be this PR.

@alice-i-cecile alice-i-cecile added the usability Reduce user friction label Sep 15, 2023
@barsoosayque
Copy link
Contributor Author

Sure thing. Didn't know Reflect also implements TypePath, but I'll try to derive it for other types too

@barsoosayque
Copy link
Contributor Author

So since PetitSet is a foreign type, I had to create a newtype for it just to have a reflectable PetitSet, and then manually implement Reflect (and other reflect related traits). It doesn't seem like a bad solution, but one major issue is that UserInput::Chord is not a Box anymore (since Box<dyn Reflect> is not reflectable, yet), therefore it drastically increased in size and I'm not really sure what to do about it.

@alice-i-cecile
Copy link
Contributor

Right, okay :) Let's merge this as is then.

@alice-i-cecile alice-i-cecile enabled auto-merge (squash) September 16, 2023 13:16
@barsoosayque barsoosayque changed the title Implement TypePath for InputMap Implement Reflect for InputMap Sep 16, 2023
auto-merge was automatically disabled September 16, 2023 14:24

Head branch was pushed to by a user without write access

@barsoosayque
Copy link
Contributor Author

Um. so this PR is pretty much the same as #331 (although it's slightly outdated). And I've seen that there is an ongoing work to remove a PetitSet as a dependency, so I'm not sure what's the best course of action here..

@alice-i-cecile
Copy link
Contributor

We should merge this and unblock this feature request, then remove PetitSet in another PR.

@alice-i-cecile alice-i-cecile enabled auto-merge (squash) September 16, 2023 14:53
auto-merge was automatically disabled September 21, 2023 07:46

Head branch was pushed to by a user without write access

alice-i-cecile added a commit that referenced this pull request Oct 26, 2023
@alice-i-cecile
Copy link
Contributor

I'm going to tackle this in #400, which should be done shortly :) This was super helpful to push me towards "remove petitset" as the right architectural decision, thank you.

alice-i-cecile added a commit that referenced this pull request Nov 11, 2023
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability Reduce user friction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants