New input proposal (and some other proposal fixes I need to do) #794
Conversation
… Enhanced Input Events.md
Perksey
added
area-Documentation
| public JoystickButtonState Buttons { get; init; } | ||
| public DualReadOnlyList<Vector2> Thumbsticks { get; init; } | ||
| public DualReadOnlyList<float> Triggers { get; init; } | ||
| public IReadOnlyList<IMotor> VibrationMotors { get; } // forwards to forwards Device.VibrationMotors for ease of use |
| ```cs | ||
| public struct GamepadState | ||
| { | ||
| public IGamepad Device { get; init; } |
Co-authored-by: Kai Jellinghaus <contact@kaij.tech>
|
I don't see the reason why devices expose only a I'm also not sold on the events getting separated from the object they originate from. What is the benefit of all these design decisions? |
| One final point to note is that throughout the rest of the proposal the following type will be used: | ||
|
|
||
| ```cs | ||
| public struct InputReadOnlyList<T> : IReadOnlyList<T> | ||
| { | ||
| public InputReadOnlyList(IReadOnlyList<T> other); | ||
| } | ||
| ``` | ||
|
|
||
| The Silk.NET team wishes to reserve the right to add more constructors to this type as it sees fit. | ||
|
|
||
| This exists so that, should the Silk.NET choose to, we can optimize the lookup of elements while ensuring things like indexers are inlined and don't result in a virtual call if our implementation allows us to do so. | ||
|
|
||
| **INFORMATIVE TEXT:** For example, for joystick and mouse buttons we could use a fixed-sized bit buffer where each bit represents an individual button: 1 for pressed, 0 for unpressed. But for something like keyboard input where there are a large amount of keys, we can't do that and will likely use `Memory<T>` instead. |
There was a problem hiding this comment.
Well this doesn't suggest anything as it's informative text, all the proposal cares about is that it's a list, it's a struct, and is constructible from another list.
We have reserved the right to make more constructors above, which will allow us to make fast paths available to external backends.
It is my intention for this type to be constructible from any IReadOnlyList<T>, hence why I have specified that constructor here.
INFORMATIVE TEXT: Yeah, we'll likely be making a size trade-off for the fast path as you're right we can't have different implementations/memory layout per T.
This is an implementation detail that shouldn't impact the proposal - as far as the proposal is concerned there is no fast path, slow path, etc.
| ```cs | ||
| public struct MouseState | ||
| { | ||
| public IMouse Device { get; init; } |
There was a problem hiding this comment.
I don't really want isolation though, as that will just annoy users who don't care about immutability or isolation, some users just want to be able to get something that represents a mouse and check whether that has a button pressed, or move a cursor.
| public IMouse Device { get; init; } | ||
| public MouseButtonState Buttons { get; init; } | ||
| public ICursorConfiguration Cursor { get; } // forwards to Device.Cursor | ||
| public Vector2 Position { get; set; } // all sets after the first set forward to Device.SetPosition |
There was a problem hiding this comment.
But the real question is, how to we expose the device then? The input context today doesn't expose the devices any other way.
We'd have to essentially have the lists iterate a (IMouse Device, MouseState State) which at that point they are just as tied as they would be if IMouse is in here.
No matter what we do, someone somewhere will complain when they set the position, either via Device.SetPosition, the original Position setter, whatever; that the state is not updated. I'm leaning more towards just implicitly updating the state if the user explicitly changes it to alleviate confusion.
|
@ThomasMiz @HurricanKai all comments have been addressed |
|
Are you two happy with this? If so approval would be appreciated! |
Co-authored-by: ThomasMiz <32400648+ThomasMiz@users.noreply.github.com>
Co-authored-by: ThomasMiz <32400648+ThomasMiz@users.noreply.github.com>
4 participants
No description provided.