Add initial axis input proposal draft #2415
Conversation
|
@dotnet-policy-service agree |
|
|
||
| public interface IAxisInputHandler | ||
| { | ||
| public void OnAxisChanged(IAxisDevice device, AxisDescription description, float value); |
There was a problem hiding this comment.
Should AxisDescription be in so we don't have to copy this every time?
There was a problem hiding this comment.
i don't feel strongly one way or another. AxisDescription is a relatively small struct, and idk what the cache-related consequences are.
note: I used to reflexively in structs > 8 bytes until you told me there was cache related consequences so if it sounds good to you it's def good to me
| public class DualsenseDevice : IAxisDevice | ||
| { | ||
| IReadOnlyList<AxisDescription> | ||
| private static readonly IReadOnlyList<AxisDescription> Axes = |
There was a problem hiding this comment.
I know this is INFORMATIVE TEXT but... triggers? Only reason I mention it is because it's quite pertinent for DualSense specifically. Feel free to ignore this comment though as it may open irrelevant discussions now I mention it (namely application-controlled axes like the haptic feedback)
There was a problem hiding this comment.
I haven't yet thought of any sort of API for axis output yet. my thoughts were outputs could be a non-breaking extension to this API as you mightve guessed - lots of things to consider there (and potentially lots of input-related expansions too e.g. microphone/audio out, LEDs, etc). i have no problem re-visiting this to properly complete the axis list though
| /// <summary> | ||
| /// An interface for handling deadzones in different ways | ||
| /// </summary> | ||
| public interface IDeadzoneHandler |
There was a problem hiding this comment.
Sceptical about calling this IDeadzoneHandler given that in our input API "handlers" usually refer to our actor-like model for handling input events.
Maybe IDeadzoneDeterminant, or maybe call this IDeadzone and rename Deadzone to DeadzoneBounds? Or something else?
There was a problem hiding this comment.
the naming was an attempt to keep the API consistent with how people will be used to handling input events, though if this doesn't actually follow the model of the others then I'm totally down to change the name. Though not crazy about those names in particular bc i think it's less clear
we can shop it with The Gang? ¯_(ツ)_/¯
| # Device Registration | ||
| Due to the complex set of constraints detailed above, it is important that a validation layer exists to ensure that higher-level code can trust the device is following important conventions and constraints. For each device that connects, it must first register to the `AxisDeviceRegistry` in order to be used as an `IAxisDevice`. | ||
|
|
||
| Any interested party can then reference the `AxisDeviceRegistry` to retrieve all available devices. Notably, this is missing a way to alert users of devices that have been newly connected or disconnected. Feedback is requested on this front: is this necessary or should it be implemented elsewhere? | ||
|
|
||
| ```csharp | ||
| public class AxisDeviceRegistry | ||
| { | ||
| /// <summary> | ||
| /// Required before usage of an IAxisDevice - this is essentially a "validation layer" where we can check if the device | ||
| /// configuration is valid, e.g. "do this AxisGroup's axes have definitions that satisfy the axis group?" | ||
| /// Things like checking for axes with a rotational definition, Left/Right handedness, groups' axis counts, etc | ||
| /// Will populate the proper AxisDeviceRegistry collections using runtime type checks | ||
| /// Registration is required to use a device each time it connects | ||
| /// | ||
| /// Should we develop a way to register these conditions at compile time as like a list of functions? | ||
| /// </summary> | ||
| public bool TryRegister(IAxisDevice device, [NotNullWhen(false)] out string? error); | ||
|
|
||
| /// <summary> | ||
| /// Unregisters a device | ||
| /// Will remove the device from the registry, required to be called when a device disconnects | ||
| /// </summary> | ||
| /// <param name="device"></param> | ||
| /// <returns>True if the device was previously registered and is now unregistered</returns> | ||
| public bool Unregister(IAxisDevice device); | ||
|
|
||
| /// <summary> | ||
| /// All currently registered devices | ||
| /// </summary> | ||
| public IReadOnlyList<IAxisDevice> Devices { get; } | ||
|
|
||
| /// <summary> | ||
| /// Runtime-generated virtual devices | ||
| /// </summary> | ||
| public IReadOnlyList<IAxisDevice> VirtualDevices { get; } | ||
|
|
||
| /// <summary> | ||
| /// Physical devices | ||
| /// </summary> | ||
| public IReadOnlyList<IAxisDevice> PhysicalDevices { get; } | ||
|
|
||
| // A future composability proposal can include additional properties to retrieve all available axis groups, irrespective | ||
| // of the source devices | ||
| // there may also be a case for making the above 3 collection types (All, Virtual, Physical) a model that can be followed by | ||
| // the standard input context, which would be the primary benefit to adding `IsVirtual` to `IInputDevice` - to be discussed in a future proposal | ||
| } | ||
| ``` |
There was a problem hiding this comment.
I don't see why we need this when we have InputContext which could be changed to implicitly validate when it encounters an IAxisDevice in a backends, and connections/disconnections are handled in the obvious way as already specified by the base proposal. If there is a desire to register/deregister individual devices then there should be an implementation of IInputBackend that accommodates this.
There was a problem hiding this comment.
I would much prefer that - just wanted to silo things so as not to disturb The Great Base Proposal. Though, if we were to do that, I would also request the addition of the VirtualDevices and Physical Devices collections to the InputContext
Co-authored-by: Dylan Perks <[email protected]>
Co-authored-by: Dylan Perks <[email protected]>
Co-authored-by: Dylan Perks <[email protected]>
…xisGroupType.Position2D and 3D
this feedback was more-or-less answered by myself in the axis group priority section. if we need or would like to provide anything more specific, it could be an extension method
Summary of the PR
Adds the axis-based input proposal's first draft
Related issues, Discord discussions, or proposals
Several I've lost track of and I am sorry to report I am feeling particularly lazy after spewing so many words from my fingertips in a single afternoon
Further Comments
Any and all feedback ahead of our discussion this sunday is welcome - even if just for clarity of writing, and especially re: the contents of the proposal