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

API Proposal #1

Open
jice-nospam opened this issue Apr 24, 2018 · 17 comments
Open

API Proposal #1

jice-nospam opened this issue Apr 24, 2018 · 17 comments

Comments

@jice-nospam
Copy link
Collaborator

jice-nospam commented Apr 24, 2018

So this library should provide low level access to gamepad/joystick input, and we have to keep in mind that we have to create a unified API for web and native. I'd rather go for a pragmatic API like this :

This might not be doable on every platform so we'll probably have to iterate a bit. it's a starting point.

Also, this is not compilable rust but you get the idea.

This also helps to define the perimeter of what we want to support with this library as this can go pretty wild (gamepad only or joystick/motion sensor? vibrations ? headsets ?) keeping in mind that high level features like key-repeat, key-mapping or dead zone should not be part of this low level library but rather on top of uni-pad.

struct ControllerInfo {
    name: String,
    digital_count : usize, // number of ON/OFF buttons
    analog_count: usize, // number of -1.0-1.0 axis
}

struct ControllerState {
    status: enum to be defined,
    sequence: usize, // incremented every time state is changed
    digital_state: Vec<bool>,
    analog_state: Vec<f32>,
}

struct ControllerContext {
fn get_controller_num() -> usize;
fn get_controller_info(controller_num: usize) -> ControllerInfo;
fn borrow_controller_state(controller_num: usize) -> &ControllerState;
}

In case a controller is not available anymore because it was unplugged, the API should return some default values so that end user doesn't have to deal with Option.

If you have 2 controllers plugged : 0 and 1 and you unplug 0, 1 is still available at index 1. 0 returns a default value with a status indicating it's unpluged.

[edit] status should be obviously in ControllerState. Also added sequence field

@edwin0cheng
Copy link
Member

Look good to me, and as you said, it is an very abstract representation for all input devices.

Just a little questions for clarify, how uni-pad mapping these values ? For examples, when a pressed X in my xbox controller, would digital_state[0] == true in all platform ?

@jice-nospam
Copy link
Collaborator Author

jice-nospam commented Apr 24, 2018

That's probably one of the trickiest part. So far i've no idea if all platforms use the same button ids but there probably need to be some mapping done at some point, similar to the keyboard mapping in uni-app. We can probably settle for a convention so that at least standard gamepads (with XYAB buttons) have a consistent behaviour, but there are a lot of different controllers with different buttons. In the end, the unrust user (and probably the player) should be able to define his own mapping.

@edwin0cheng
Copy link
Member

Sound reasonable, would you mind to start the Windows implementation first ? I would like to follow your code styles and implement back the MacOS and Linux part.

@jice-nospam
Copy link
Collaborator Author

ok, i'll have a look at current gilrs linux implementation too to avoid being too influenced by windows API (there are 3 one on windows btw... winapi, directinput and xinput...)

@edwin0cheng
Copy link
Member

edwin0cheng commented Apr 24, 2018

Thanks, and i think XInput is more reliable, right ? (I don't know about gamepad api a lot)

@jice-nospam
Copy link
Collaborator Author

XInput is the latest and bestest API but it only supports XBOX360 controllers. DirectInput is older, but more generic and plays more gently with exotic controllers.

@edwin0cheng
Copy link
Member

edwin0cheng commented Apr 24, 2018

It is up to you to use which one, i have no idea (I have a XBox360 controller anyway )

@jice-nospam
Copy link
Collaborator Author

ok I've got a working XInput implementation at https://github.com/jice-nospam/gamepad-rs
As expected, it detects my XBOX360 controller, but not some exotic controller. I also started a Linux implementation but it's unfinished.

@edwin0cheng
Copy link
Member

edwin0cheng commented Apr 26, 2018

So fast~~~, I am looking on your code now, Here are some ideas on ControllerContextInterface (mainly about rust idioms) :

  1. Seem like ControllerContextInterface trait can be ControllerContext, as we did not want to expose the actual implemetion, right ?
  2. Maybe get_controller_count could be count as https://doc.rust-lang.org/1.0.0/style/style/naming/README.html#getter/setter-methods-[rfc-344] stated ?
  3. borrow_controller_info and borrow_controller_state can be simplified to info and state, as it must be controller.
  4. controller_num maybe should renamed to index as it is just an index, normally num represents it is a total number.

@edwin0cheng
Copy link
Member

And i just tested it (Windows/Xbox 360 controller), it works perfectly even hot plugged, great !!

@jice-nospam
Copy link
Collaborator Author

Nice :)
On my PC, everything works but the "shoulder buttons" (RB and LB) that are not detected. They work on your system ?

About your ideas :

  1. Currently, the user uses ControllerContext::new(). The interface is only here so that the compiler can check whether your platform implementation is correct, but maybe I can just drop the interface (uni-app doesn't use such an interface and simply expect all implementations to be compatible)

  2. Actually get_controller_count is not a simple getter. It scans available controllers and sets the Connected/Disconnected states (that's why it's a &mut self function). Maybe I should rather call it scan_controllers

  3. In the same idea, the borrow_* function and not getters. They update the context's state. This way, there's no thread constantly polling the input driver. State is only updated when the user needs it. So maybe we should call them update_and_borrow_* ?...

  4. right!

@edwin0cheng
Copy link
Member

RB and LB buttons works in my system.

  1. If this trait should be only used internal only, then maybe remove the pub.
  2. scan_controllers would be good.
  3. How about something like these :

trait ControllerContextInterface {
    /// Scan all device and return number of valid controllers 
    fn scan_controllers(&mut self) -> usize;
    /// Update controller state by index
    fn update(&mut self, index: usize);    
    /// Get current information of Controller
    fn info(&self, index: usize) -> &ControllerInfo;
    /// Get current state of Controller
    fn state(&self, index: usize) -> &ControllerState;    
}

And we can lazy update the ControllerInfo and poll updating ControllerState when update is called.

@edwin0cheng
Copy link
Member

edwin0cheng commented Apr 26, 2018

There are some warnings when i compiled, and i think this session :
https://github.com/jice-nospam/gamepad-rs/blob/master/src/platform/windows/mod.rs#L107-L119

    fn update_info(&mut self, id: usize, capabilities: &XCapabilities) {
        let mut name = String::from("XBOX360");
        match capabilities.SubType {
            XINPUT_DEVSUBTYPE_GAMEPAD => name.push_str(" gamepad"),
            XINPUT_DEVSUBTYPE_WHEEL => name.push_str(" wheel"),
            XINPUT_DEVSUBTYPE_ARCADE_STICK => name.push_str(" arcade stick"),
            XINPUT_DEVSUBTYPE_FLIGHT_STICK => name.push_str(" flight stick"),
            XINPUT_DEVSUBTYPE_DANCE_PAD => name.push_str(" dance pad"),
            XINPUT_DEVSUBTYPE_GUITAR => name.push_str(" guitar"),
            XINPUT_DEVSUBTYPE_GUITAR_ALTERNATE => name.push_str(" guitar"),
            XINPUT_DEVSUBTYPE_GUITAR_BASS => name.push_str(" bass guitar"),
            XINPUT_DEVSUBTYPE_DRUM_KIT => name.push_str(" drum"),
            XINPUT_DEVSUBTYPE_ARCADE_PAD => name.push_str(" arcade pad"),

Are all these missing xinput:: in front of each enum ??

Because rust cannot find the symbol, it treat all symbols as variables. And it equal to :

    fn update_info(&mut self, id: usize, capabilities: &XCapabilities) {
        let mut name = String::from("XBOX360");
        match capabilities.SubType {
            a=> name.push_str(" gamepad"),
            b=> name.push_str(" wheel"),
            c=> name.push_str(" arcade stick"),
            c=> name.push_str(" flight stick"),
            d=> name.push_str(" dance pad"),
            e=> name.push_str(" guitar"),
            f=> name.push_str(" guitar"),
            q=> name.push_str(" bass guitar"),
            g=> name.push_str(" drum"),
            f=> name.push_str(" arcade pad"),

So only the first arm will be matched.

@jice-nospam
Copy link
Collaborator Author

jice-nospam commented Apr 26, 2018

Ah thanks, I didn't understand where these warnings were from. Since the symbols were not used, I wonder why this compiled...

If this trait should be only used internal only, then maybe remove the pub.

If I remove the pub, the end user can't use the functions from the trait... bah I'll just remove the trait, no?

/// Update controller state by index
fn update(&mut self, index: usize);

Ok, but the game should call update() then, not the engine (unrust) else you'll have to update all controllers whereas the game might only need one (if it's a single player game)

@edwin0cheng
Copy link
Member

edwin0cheng commented Apr 26, 2018

Ah thanks, I didn't understand where these warnings were from. Since the symbols were not used, I wonder why this compiled...

For example , the following code :

match 2 {
  XINPUT_DEVSUBTYPE_GAMEPAD => println("{}", XINPUT_DEVSUBTYPE_GAMEPAD);
}

will print 2, Rust just treat XINPUT_DEVSUBTYPE_GAMEPAD as variable.

If I remove the pub, the end user can't use the functions from the trait... bah I'll just remove the trait, no?

I think remove the trait is fine. ;)

Ok, but the game should call update() then, not the engine (unrust) else you'll have to update all controller whereas the game might only need one (if it's a single player game)

That's fine. :)

@jice-nospam
Copy link
Collaborator Author

btw it's done, but the linux port is still wip. Do I create a PR on main repo to be able to integrate this into uni-pad or do you prefer to wait for the linux port to be finished ?

@edwin0cheng
Copy link
Member

edwin0cheng commented Apr 30, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants