-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
UI: Device emulation - Xbox Controller S #1802
base: master
Are you sure you want to change the base?
UI: Device emulation - Xbox Controller S #1802
Conversation
build: Update arm64 target, handle target/arch independent pkg names
nv2a: Scale line thickness by surface scale factor
…er S in addition to the Xbox Controller
…er S in addition to the Xbox Controller
1b01a48
to
9e05b66
Compare
…faha223/xemu into DeviceEmulation-XboxControllerS
ui/xemu-input.c
Outdated
{ | ||
assert(port >= 0 && port <= 3); | ||
const char *driver = NULL; | ||
switch (port) { |
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.
Can we use the port_index_to_driver_settings_key_map
lookup table here?
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.
Yeah, I'm not sure why I didn't do that here already
return DRIVER_S; | ||
|
||
// Shouldn't be possible | ||
return DRIVER_DUKE; |
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.
This can be reached if the config is manually edited, or if someone jumps between a feature branch that implements a new driver without changing it first.
(Picking the Duke driver is probably a good default here anyway, but the assumption that it's unreachable is not correct)
ui/xemu-input.h
Outdated
@@ -81,7 +87,7 @@ typedef struct ControllerState { | |||
// Input state | |||
uint16_t buttons; | |||
int16_t axis[CONTROLLER_AXIS__COUNT]; | |||
|
|||
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.
nit: Extra spacing here
ui/xui/gl-helpers.cc
Outdated
@@ -33,7 +34,7 @@ | |||
#include "ui/shader/xemu-logo-frag.h" | |||
|
|||
Fbo *controller_fbo, *xmu_fbo, *logo_fbo; | |||
GLuint g_controller_tex, g_logo_tex, g_icon_tex, g_xmu_tex; | |||
GLuint g_controller_tex, g_controller_s_tex, g_logo_tex, g_icon_tex, g_xmu_tex; |
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.
Should we rename g_controller_tex
to g_controller_duke_tex
?
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.
I think that's a good idea
ui/xui/gl-helpers.cc
Outdated
@@ -464,7 +467,7 @@ static void RenderMeter(DecalShader *s, float x, float y, float width, | |||
RenderDecal(s, x, y, width * p, height, 0, 0, 1, 1, 0, 0, color_fg); | |||
} | |||
|
|||
void RenderController(float frame_x, float frame_y, uint32_t primary_color, | |||
static void RenderController_Duke(float frame_x, float frame_y, uint32_t primary_color, |
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.
nit: This doesn't technically follow the naming convention, maybe something like RenderDukeController
or `RenderControllerS?
if (strcmp(driver, DRIVER_DUKE) == 0) | ||
driver = DRIVER_DUKE_DISPLAY_NAME; | ||
else if (strcmp(driver, DRIVER_S) == 0) | ||
driver = DRIVER_S_DISPLAY_NAME; |
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.
What if driver doesn't compare to either of these (e.g. in one of the cases I described above)?
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.
bound_drivers is initialized via get_bound_driver which defaults to DRIVER_DUKE when the value read from the config file is not recognized.
bound_drivers can also be changed via the dropdown, but at that point it should only be possible to assign known good values because they're pulled from available_drivers.
I probably should add an assert(false) here if it doesn't match a known good value
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.
If somehow the value isn't a known good value, then no value in the dropdown will be selected but the user should still be able to assign a value to it
Fixed some naming issues Changed get_bound_driver to use port_index_to_driver_settings_key_map lookup table. Changed comment in get_bound_driver to clarify when we default to DUKE
This PR adds the emulation of just the Xbox Controller S. It's the simplest of the devices I'm adding support for because it uses the same XID Input and Output report structures as the Xbox Controller
This PR exists mainly to establish the added UI control for picking which device the user intends to emulate on a given controller port
I also split out the Xbox Gamepad specific structures from the shared XID code in preparation for adding support for other XID devices. I'm going to create PRs for each of those individually in addition to this one. There will be some shared code between the PRs, mostly UI stuff.