feat: leafwing action for grab

[luan]

Allow using leafwing-input-manager for action binding. See examples/leafwing.rs.

[johanhelsing]

Thanks for the PR!

Adding input logic like combos to this crate feels slightly like scope creep. Maybe it could be doable by having an optional dependency on leafwing-input-manager instead? And allow arbitrary actions. Or have some other more flexible way of triggering moves? Perhaps events?

I'm not saying it's a definitive no, just hoping we could avoid having this sort of general purpose input logic here, since it's difficult to get right and cover every use case.

For instance, I believe the PR as it currently is interprets

1. Mouse down
2. Modifier down

As a valid key combo, which is probably not what the user would expect.

[luan]

Thanks for the PR!

Adding input logic like combos to this crate feels slightly like scope creep. Maybe it could be doable by having an optional dependency on leafwing-input-manager instead? And allow arbitrary actions. Or have some other more flexible way of triggering moves? Perhaps events?

I'm not saying it's a definitive no, just hoping we could avoid having this sort of general purpose input logic here, since it's difficult to get right and cover every use case.

For instance, I believe the PR as it currently is interprets

1. Mouse down
2. Modifier down

As a valid key combo, which is probably not what the user would expect.

Thanks for that feedback! Adding leafwing_input_manager as an optional dependency behind a feature sounds great! I was on the fence about adding another dependency so I slapped this together but just the edge case you described is enough to convince me this is going to be too much logic for this crate.

I'll update the PR with the optional leafwing actions shortly

[luan]

I've added a Action::Grab -- we could do the same for zoom I think but I didn't want to go overboard until you got a chance to give me some more feedback here.

[luan]

FWIW the example I committed using leafwing's chords also has that weird ordering behavior you described, but now the issue is no longer on this crate, which I think is a positive change.

[johanhelsing]

Looks great!

I think we should probably settle on one default value for grab_buttons, but otherwise this is fine.

[johanhelsing]

Hmm... I'm not sure how I feel about changing the defaults based on a feature flag... could lead to subtle surprising behavior/bugs if you have multiple crates that depend on pancam?

[luan]

I didn't want to have the default be using standard input if one requested leafwing. I made a big change around this now (will detail it in the main thread).

[johanhelsing]

Action feels maybe a bit too broad and it will probably have to be aliased when used to make intention clear? Maybe prefixing it PanCamAction is better?

(Honest question) :)

[luan]

Yeah I think I agree. Initially I assumed Action was "fine" because we're inside the bevy_pancam module, but it's pretty common to use import the type directly (or sometimes even via ::*) which would surely dirty up the namespace.

[johanhelsing]

FWIW the example I committed using leafwing's chords also has that weird ordering behavior you described, but now the issue is no longer on this crate, which I think is a positive change.

Yeah, it's quite a subtle thing. If I understand correctly, this is the issue to follow Leafwing-Studios/leafwing-input-manager#49

[luan]

@johanhelsing I pushed a refactor around defaults that consists of a breaking change. The reason for it is I needed to move grab_buttons out of the PanCam component so that we could carry that over to a component that is added via a bundle optionally, this makes it possible to keep leafwing related defaults and regular inputs separate.

Like I said, it does make it breaking, because existing code won't work without some minor corrections (see what I did on the examples).

[johanhelsing]

Hey sorry for the late response, but i think i prefer it in a single component. I generally follow the approach of making the common use case dead simple, while making more advanced use cases possible.

I think it's acceptable if leafwing users have to set buttons: vec![]

[johanhelsing]

I mean have the features be truly additive. Do you think it's possible? Maybe I'm asking too much, but just thought a not entirely thought through response is better than no response

[luan]

I can certainly do that, but I think it makes for a messier API. The idea that choosing leafwing uses leafwing to setup the default made more sense to me. Unfortunately, it did make it so I had to create a bundle type initializer instead of using the original.

I'll sit down to work on game stuff later tonight, until then if you have any thoughts let me know. If I don't hear back I'll just do what you asked and revert back to the buttons vector default.

[johanhelsing]

Thanks, perhaps you're right. I can have another look Thursday or Friday alternatively

[luan]

I've update it against main although I didn't change it back to default to using the regular input modes. Given your last message I'm not 100% which way you'd prefer to go, but happy to come back to this and get it done either way, just give me a ping.