-
Notifications
You must be signed in to change notification settings - Fork 435
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
Make analog sticks emit "fake" digital actions for left/right/up/down… #1746
Changes from 2 commits
7c77870
4d1dd74
3ded9b2
f72cace
ffe6838
1c9febd
dc9a674
08aa03d
1c9ca2e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -184,9 +184,17 @@ class FlxGamepad implements IFlxDestroyable | |
control = _device.getControlAt(i); | ||
|
||
//quick absolute value for analog sticks | ||
var value = Math.abs(control.value); | ||
button = getButton(i); | ||
|
||
if (isAxisForAnalogStick(i)) | ||
{ | ||
handleAxisMove(i, control.value, button.value); | ||
} | ||
|
||
button.value = control.value; | ||
|
||
var value = Math.abs(control.value); | ||
|
||
if (value < deadZone) | ||
{ | ||
button.release(); | ||
|
@@ -261,10 +269,10 @@ class FlxGamepad implements IFlxDestroyable | |
manager = null; | ||
|
||
#if FLX_JOYSTICK_API | ||
hat = FlxDestroyUtil.put(hat); | ||
hat = FlxDestroyUtil.put(hat); | ||
ball = FlxDestroyUtil.put(ball); | ||
|
||
hat = null; | ||
hat = null; | ||
ball = null; | ||
#end | ||
} | ||
|
@@ -606,6 +614,61 @@ class FlxGamepad implements IFlxDestroyable | |
return getAnalogYAxisValue(Stick); | ||
} | ||
|
||
@:allow(flixel.input.gamepad.FlxGamepadManager) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is unnecessary, the class already has |
||
private function handleAxisMove(axis:Int, newVal:Float, oldVal:Float) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Abbreviating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. result of a refactor, those were the old variable names, updating now. |
||
{ | ||
newVal = applyAxisFlip(newVal, axis); | ||
oldVal = applyAxisFlip(oldVal, axis); | ||
|
||
//check to see if we should send digital inputs as well as analog | ||
var stick:FlxGamepadAnalogStick = this.getAnalogStickByAxis(axis); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary |
||
if (stick.mode == ONLY_DIGITAL || stick.mode == BOTH) | ||
{ | ||
var neg = stick.digitalThreshold * -1; | ||
var pos = stick.digitalThreshold; | ||
var digitalButton = -1; | ||
|
||
trace("axis(" + axis + ") val = " + newVal); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leftover trace. |
||
|
||
//pressed/released for digital LEFT/UP | ||
if (newVal < neg && oldVal >= neg) | ||
{ | ||
if (axis == stick.x) digitalButton = stick.rawLeft; | ||
else if (axis == stick.y) digitalButton = stick.rawUp; | ||
var btn = getButton(digitalButton); | ||
if (btn != null) btn.press(); | ||
} | ||
else if (newVal >= neg && oldVal < neg) | ||
{ | ||
if (axis == stick.x) digitalButton = stick.rawLeft; | ||
else if (axis == stick.y) digitalButton = stick.rawUp; | ||
var btn = getButton(digitalButton); | ||
if (btn != null) btn.release(); | ||
} | ||
|
||
//pressed/released for digital RIGHT/DOWN | ||
if (newVal > pos && oldVal <= pos) | ||
{ | ||
if (axis == stick.x) digitalButton = stick.rawRight; | ||
else if (axis == stick.y) digitalButton = stick.rawDown; | ||
var btn = getButton(digitalButton); | ||
if (btn != null) btn.press(); | ||
} | ||
else if (newVal <= pos && oldVal > pos) | ||
{ | ||
if (axis == stick.x) digitalButton = stick.rawRight; | ||
else if (axis == stick.y) digitalButton = stick.rawDown; | ||
var btn = getButton(digitalButton); | ||
if (btn != null) btn.release(); | ||
} | ||
|
||
if (stick.mode == ONLY_DIGITAL) | ||
{ | ||
//still haven't figured out how to suppress the analog inputs properly. Oh well. | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Whether any buttons have the specified input state. | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ class FlxGamepadAnalogStick | |
/** | ||
* when analog inputs are received, how to process them digitally | ||
*/ | ||
public var mode(default, null):FlxAnalogToDigitalMode = ONLY_ANALOG; | ||
public var mode(default, null):FlxAnalogToDigitalMode = BOTH; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing the default here seems like a breaking change. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not strictly speaking breaking; previously it was reporting only analog actions, this would make it report both analog and digital. As far as I can tell, the analog-to-digital actions weren't even mapped up correctly and thus weren't being reported. The only difference users should see is new digital actions being reported, and will only affect things if they choose to poll for them. |
||
|
||
public function new(x:Int, y:Int, ?settings:FlxGamepadAnalogStickSettings) | ||
{ | ||
|
@@ -42,12 +42,12 @@ class FlxGamepadAnalogStick | |
if (settings == null) | ||
return; | ||
|
||
mode = (settings.mode != null) ? settings.mode : ONLY_ANALOG; | ||
mode = (settings.mode != null) ? settings.mode : BOTH; | ||
rawUp = (settings.up != null) ? settings.up : -1; | ||
rawDown = (settings.down != null) ? settings.down : -1; | ||
rawLeft = (settings.left != null) ? settings.left : -1; | ||
rawRight = (settings.right != null) ? settings.right : -1; | ||
digitalThreshold = (settings.threshold != null) ? settings.threshold : -1; | ||
digitalThreshold = (settings.threshold != null) ? settings.threshold : 0.5; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing the default here seems like a breaking change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above note. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify in this situation: since the default setting everywhere was to only report Analog, I'm pretty sure nobody was actually accessing this behavior. Accessing analog-to-digital virtual stick directions was buried in the API, essentially undocumented, and only worked on legacy. This PR is basically the first time the feature's actually been exposed to daylight in any practical sense. |
||
} | ||
|
||
public function toString():String | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,4 +31,9 @@ class FlxGamepadButton extends FlxInput<Int> | |
super.press(); | ||
} | ||
#end | ||
|
||
/** | ||
* Optional analog value, so we can check when the value has changed from the last frame | ||
*/ | ||
public var value:Float = 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Variables are declared at the top of the class. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -81,14 +81,24 @@ abstract FlxGamepadInputID(Int) from Int to Int | |
/**for a mouse-like input such as touch or IR camera. Vertical axis.**/ | ||
var POINTER_Y = 29; | ||
|
||
var LEFT_STICK_DIGITAL_UP = 30; | ||
var LEFT_STICK_DIGITAL_RIGHT = 31; | ||
var LEFT_STICK_DIGITAL_DOWN = 32; | ||
var LEFT_STICK_DIGITAL_LEFT = 33; | ||
|
||
var RIGHT_STICK_DIGITAL_UP = 34; | ||
var RIGHT_STICK_DIGITAL_RIGHT = 35; | ||
var RIGHT_STICK_DIGITAL_DOWN = 36; | ||
var RIGHT_STICK_DIGITAL_LEFT = 37; | ||
|
||
/**an extra digital button that doesn't fit cleanly into the universal template**/ | ||
var EXTRA_0 = 30; | ||
var EXTRA_0 = 46; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing these IDs is a breaking change because this abstract allows There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, so just move the new ids to the end of the value list? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems sensible. |
||
/**an extra digital button that doesn't fit cleanly into the universal template**/ | ||
var EXTRA_1 = 31; | ||
var EXTRA_1 = 47; | ||
/**an extra digital button that doesn't fit cleanly into the universal template**/ | ||
var EXTRA_2 = 32; | ||
var EXTRA_2 = 48; | ||
/**an extra digital button that doesn't fit cleanly into the universal template**/ | ||
var EXTRA_3 = 33; | ||
var EXTRA_3 = 49; | ||
|
||
@:from | ||
public static inline function fromString(s:String) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -453,58 +453,18 @@ class FlxGamepadManager implements IFlxInputManager | |
{ | ||
var isForStick = gamepad.isAxisForAnalogStick(i); | ||
var isForMotion = gamepad.mapping.isAxisForMotion(i); | ||
|
||
if (!isForStick && !isForMotion) | ||
{ | ||
// in legacy this returns a (-1, 1) range, but in flash/next it | ||
// returns (0,1) so we normalize to (0, 1) for legacy target only | ||
newAxis[i] = (newAxis[i] + 1) / 2; | ||
} | ||
else if (isForStick) | ||
else if(isForStick) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flixel's code style always has a space after |
||
{ | ||
//check to see if we should send digital inputs as well as analog | ||
var stick:FlxGamepadAnalogStick = gamepad.getAnalogStickByAxis(i); | ||
if (stick.mode == ONLY_DIGITAL || stick.mode == BOTH) | ||
{ | ||
var newVal = newAxis[i]; | ||
var oldVal = oldAxis[i]; | ||
|
||
var neg = stick.digitalThreshold * -1; | ||
var pos = stick.digitalThreshold; | ||
var digitalButton = -1; | ||
|
||
//pressed/released for digital LEFT/UP | ||
if (newVal < neg && oldVal >= neg) | ||
{ | ||
if (i == stick.x) digitalButton = stick.rawLeft; | ||
else if (i == stick.y) digitalButton = stick.rawUp; | ||
handleButtonDown(device, digitalButton); | ||
} | ||
else if (newVal >= neg && oldVal < neg) | ||
{ | ||
if (i == stick.x) digitalButton = stick.rawLeft; | ||
else if (i == stick.y) digitalButton = stick.rawUp; | ||
handleButtonUp(device, digitalButton); | ||
} | ||
|
||
//pressed/released for digital RIGHT/DOWN | ||
if (newVal > pos && oldVal <= pos) | ||
{ | ||
if (i == stick.x) digitalButton = stick.rawRight; | ||
else if (i == stick.y) digitalButton = stick.rawDown; | ||
handleButtonDown(device, digitalButton); | ||
} | ||
else if (newVal <= pos && oldVal > pos) | ||
{ | ||
if (i == stick.x) digitalButton = stick.rawRight; | ||
else if (i == stick.y) digitalButton = stick.rawDown; | ||
handleButtonUp(device, digitalButton); | ||
} | ||
|
||
if (stick.mode == ONLY_DIGITAL) | ||
{ | ||
//still haven't figured out how to suppress the analog inputs properly. Oh well. | ||
} | ||
} | ||
var newVal = newAxis[i]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These temp variables seem a little unnecessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cruft from a refactor, removing... |
||
var oldVal = oldAxis[i]; | ||
gamepad.handleAxisMove(i, newVal, oldVal); | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,9 +29,6 @@ class LogitechID | |
//TODO: Someone needs to look this up and define it! (NOTE: not all logitech controllers have this) | ||
public static inline var LOGITECH:Int = -1; | ||
|
||
public static var LEFT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(0, 1); | ||
public static var RIGHT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(2, 3); | ||
|
||
#else // native and html5 | ||
public static inline var ONE:Int = 0; | ||
public static inline var TWO:Int = 1; | ||
|
@@ -55,7 +52,9 @@ class LogitechID | |
//TODO: Someone needs to look this up and define it! (NOTE: not all logitech controllers have this) | ||
public static inline var LOGITECH:Int = -5; | ||
|
||
public static var LEFT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(0, 1); | ||
public static var RIGHT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(2, 3); | ||
#end | ||
|
||
public static var LEFT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(0, 1, {up:24, down:25, left:26, right:27}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is there no indent here? |
||
public static var RIGHT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(2, 3, {up:28, down:29, left:30, right:31}); | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,8 +25,8 @@ class MFiID | |
public static inline var DPAD_LEFT:Int = 19; | ||
public static inline var DPAD_RIGHT:Int = 20; | ||
|
||
public static var LEFT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(0, 1); | ||
public static var RIGHT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(2, 3); | ||
public static var LEFT_ANALOG_STICK (default, null) = new FlxGamepadAnalogStick(0, 1, {up:21, down:22, left:23, right:24}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where do these up / down / left / right IDs come from? Just arbitrary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are "arbitrary" in that they don't match any physical inputs, but are specifically chosen to not conflict with any existing button ID's. They are basically new virtual digital button inputs that represent the analog stick being pushed past the digital threshold in each direction. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, wouldn't it be better if they weren't so close to the actual IDs? Maybe have them all start at 100-something an an indicator that they aren't real IDs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, that's totally fine, as long as you're okay with these other two considerations:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The array indices thing is a good argument against my suggestion. I think we might even have had the exact same conversation about those other fake IDs a while ago.. :D |
||
public static var RIGHT_ANALOG_STICK(default, null) = new FlxGamepadAnalogStick(2, 3, {up:25, down:26, left:27, right:28}); | ||
|
||
public static inline var LEFT_TRIGGER:Int = 4; | ||
public static inline var RIGHT_TRIGGER:Int = 5; | ||
|
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.
Fyi, I'm not a fan of this formatting for vertical alignment. That's just a lot of extra maintenance effort with little benefit.