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

Composing Input types #71

Open
DarkDrek opened this issue Jul 10, 2019 · 6 comments
Open

Composing Input types #71

DarkDrek opened this issue Jul 10, 2019 · 6 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@DarkDrek
Copy link
Contributor

When #65 and #69 are implemented there are three basic Input's and one combination of two Input's.
Do users want all possible combinations as predefined types?
Is there a elegant way to implement all Input combinations?

A macro like this gist may help.

@hecrj
Copy link
Owner

hecrj commented Jul 11, 2019

Do users want all possible combinations as predefined types?

The plan is to offer the most common ones, like KeyboardAndMouse.

Is there a elegant way to implement all Input combinations?

A macro like this gist may help.

I think a derive macro would be the best strategy here. However, as far as I know, it needs its own crate and it adds quite the amount of complexity just to get rid of very simple and easy to maintain code. I don't think creating macros is worth it when the sole purpose of them is to remove a small amount of boilerplate.

Additionally, most combinations of input types won't probably be as straightforward as KeyboardAndMouse. For instance, if we want to combine a Keyboard with a Gamepad (see #65), we will probably need to keep track of the last active device too, etc.

In conclusion, I think we should wait until we see a clearer pattern before deciding on an abstraction.

@hecrj hecrj added help wanted Extra attention is needed question Further information is requested labels Jul 11, 2019
@PvdBerg1998
Copy link
Contributor

I disagree, I think this is a perfect use case of a declarative macro. It avoids some boilerplate of creating a new structure with accessor fields. Procedural macro's are even more magic and indeed require a lot of extra complexity.

@hecrj
Copy link
Owner

hecrj commented Jul 11, 2019

I do not think it's a good idea to add 47 lines of macro code just to remove 44 simple lines of code.

I will quote myself:

In conclusion, I think we should wait until we see a clearer pattern before deciding on an abstraction.

You disagree with this, @PvdBerg1998?

To me, the derive macro sounds like a better approach because it feels more intuitive (it would work just like Debug, Clone, Copy, etc.), it has a clear goal, it would be less prone to change, and it can be exposed seamlessly with the trait as part of the public API, allowing users to combine input trackers themselves.

On the other hand, exposing a declarative macro that generates a struct, generates accessor methods, and also generates the trait implementation feels like a bit too much unnecessary magic. We could use it internally, but we would need to be able to justify it first. Right now, we only have one use case for it, and I have given an example showing why this abstraction won't be enough:

For instance, if we want to combine a Keyboard with a Gamepad (see #65), we will probably need to keep track of the last active device too, etc.

Each input combination will try to satisfy a specific use case. The fact that the KeyboardAndMouse type is a simple composition of two other types is a coincidence (a nice one). This could easily change in the future.

I think we would at least need 3 similar use cases before even thinking about this. And even then, I think it would probably still not be worth it. Abstracting low-maintenance code is a mistake.

@PvdBerg1998
Copy link
Contributor

PvdBerg1998 commented Jul 11, 2019

A macro is nice here when you want to have a handful of different inputs... But that will probably not exceed 3 if I'm to guess. In that case it's overkill. I was thinking about the case where one would have n input structs that all need accessors etc.

Edit: about the magic part: I find declarative macro's MUCH easier to read than procedural ones.

@hecrj
Copy link
Owner

hecrj commented Jul 12, 2019

In that case it's overkill. I was thinking about the case where one would have n input structs that all need accessors etc.

Right, but as you say this shouldn't happen. We should not encourage API users to deal with many input sources at the same time.

For instance, most games (singleplayer at least) that support both gamepads and KBM tend to only listen to the last active source. This is an interesting use case that we should probably satisfy. Instead of having an input tracker with three fields and three accessors, we could make an enum with two variants: Gamepad and KeyboardAndMouse. This way, we abstract the "source switch" in the trait implementation while the API becomes very apparent and easy to use! The user won't be able to listen to an inactive input source! It will be impossible to do so.

As I said, each use case will most likely need its own particular design.

I find declarative macro's MUCH easier to read than procedural ones.

They are both quite hard to read for me. I was mostly talking about the cognitive load for the end-user of our API. A derive macro is quite a simple concept to grasp if you compare it with a custom declarative macro:

#[derive(Input)]
struct KeyboardAndMouse {
    keyboard: Keyboard,
    mouse: Mouse,
}

// vs

create_input_tracker!(KeyboardAndMouse, mouse: Mouse; "mouse", keyboard: Keyboard; "keyboard");

To me, the first one feels quite straightforward if you are familiar with Rust. The second one needs a meaningful name, specific arguments in the correct order, and custom syntax.

@coolshaurya
Copy link

Maybe there could be Combination<A,B> type that works like KeyboardAndMouse ?

struct Combination<A, B> where 
    A: Input,
    B: Input,
{ 
    pub input_a: A,
    pub input_b: B,
}

impl Input for Combination { ... }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants