-
Notifications
You must be signed in to change notification settings - Fork 121
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
Replace axis-like input configuration with new input processors #494
Replace axis-like input configuration with new input processors #494
Conversation
fa0242d
to
f4468cc
Compare
the rounded square deadzone seems to be a bit... dead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally quite pleased with this abstraction, and the builder pattern used! It makes a lot of sense to me. Code quality and docs are exceptional, you've done an awesome job there.
However:
- The pipeline work is super cool, but not needed for an MVP. I'm opposed to merging it in our initial implementation, and will require benchmarks to merge it in a followup. Simply storing a
Vec<Box<dyn AxisProcessor>>
is a fine design to start with. - Some cleanup to do: the
region
comments should all be gone. - Fewer macros. The
Reflect
for trait objects one can stay, but the others should be cut. They're relatively hard to maintain, and will be a serious barrier to upstreaming into Bevy. typetag
cannot be used, flat out. It's incompatible with WASM builds, so we need to redesign to strip it out. See Support static constructors (ctors) rustwasm/wasm-bindgen#1216.
The processors still need to be removed from this PR. I also don't quite understand why all of the typetag stuff is needed in the first place. Is this just for serialization of processors? I would really like to avoid introducing the associated complexity and dependencies, and would frankly rather sacrifice the ability to store arbitrary preprocessor functions than add it. I understand if that's not the path you'd prefer to take: I'm happy to adopt this excellent work and finish it up on my own if you'd like. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'm satisfied now. To rehash from our conversation on Discord:
- Pipelines are much simpler now, and just store a
Vec<Boxy dyn Processor>
. - We need typetag-style functionality for serialization of boxed trait objects. While we could move away from trait objects here, in favor of a finite set of operations, it won't actually trim complexity / dependencies in the end.
The case for using trait objects for input types is dramatically stronger, as is the need for serialization.
Merging!
Objective
Solution
Added input processors for
SingleAxis
,DualAxis
,VirtualAxis
, andVirtualDpad
to refine input values:AxisProcessor
: Handles single-axis values.DualAxisProcessor
: Handles dual-axis values.AxisProcessingPipeline
: Chain processors for single-axis values.DualAxisProcessingPipeline
: Chain processors for dual-axis values.AxisInverted
: Single-axis inversion.DualAxisInverted
: Dual-axis inversion.AxisSensitivity
: Single-axis scaling.DualAxisSensitivity
: Dual-axis scaling.AxisBounds
: Restricts single-axis values to a range.DualAxisBounds
: Restricts single-axis values to a range along each axis.CircleBounds
: Limits dual-axis values to a maximum magnitude.AxisExclusion
: Excludes small single-axis values.DualAxisExclusion
: Excludes small dual-axis values along each axis.CircleExclusion
: Excludes dual-axis values below a specified magnitude threshold.AxisDeadZone
: Normalizes single-axis values based onAxisExclusion
andAxisBounds::default
.DualAxisDeadZone
: Normalizes dual-axis values based onDualAxisExclusion
andDualAxisBounds::default
.CircleDeadZone
: Normalizes dual-axis values based onCircleExclusion
andCircleBounds::default
.register_axis_processor
andregister_dual_axis_processor
for registering processors in thebevy::prelude::App
.serde_typetag
for adding type tags for processors.TODO
Known Issues
Changelog
DualAxisShape
with new deadzones.Internal changes
typetag
,dyn_clone
,dyn_eq
,dyn_hash
for boxed dyn trait objects, ensuring object-safety.InputStreams::input_value
to use these processors.