Added EvaluateMagnitude override to Vector3Composite to fix issues with callbacks
#1763
Conversation
| @@ -124,7 +124,7 @@ public class Vector2Composite : InputBindingComposite<Vector2> | |||
| /// </code> | |||
| /// </example> | |||
| /// </remarks> | |||
| public Mode mode; | |||
| public Mode mode = Mode.DigitalNormalized; | |||
There was a problem hiding this comment.
Is this a API/behaviorial change (may not be a valid patch) or is it just a case of explicit initialisation? (fine)
There was a problem hiding this comment.
I answer my own question, it seems like its not a change since Mode.DigitalNormalized was assigned value zero which would be equivalent to default(Mode), hence this is better in my opinion since explicit.
There was a problem hiding this comment.
I must admit, I'm a little sad that Vector2Composite and Vector3Composite both have a Mode enum with the same values (although they have been re-ordered for Vector2) and that the mode defaults are different.
But yes, this is just a cosmetic change for explicitness and consistency.
There was a problem hiding this comment.
Yes that inconsistency is indeed sad, something to address when a breaking change can be done.
| @@ -8057,6 +8057,39 @@ public void Actions_Vector2Composite_RespectsButtonPressurePoint() | |||
| Assert.That(value.Value, Is.EqualTo(Vector2.left)); | |||
| } | |||
|
|
|||
| [Test] | |||
| [Category("Actions")] | |||
| public void Actions_Vector2Composite_WithKeyboardKeys_CancelOnRelease() | |||
There was a problem hiding this comment.
Excellent to see you've added tests to cover this functionality which seem to have been missing.
| @@ -46,7 +46,7 @@ public class Vector3Composite : InputBindingComposite<Vector3> | |||
| /// </remarks> | |||
| // ReSharper disable once MemberCanBePrivate.Global | |||
| // ReSharper disable once FieldCanBeMadeReadOnly.Global | |||
| [InputControl(layout = "Axis")] public int up; | |||
| [InputControl(layout = "Axis")] public int up = 0; | |||
There was a problem hiding this comment.
Not sure these need explicit initialisation since default(int) is known to be zero in C#?
There was a problem hiding this comment.
That makes sense, in that case I'll change the Vector2Composite to remove the explicit initialisation - just noticed some discrepancies in consistency and wanted to clean them up!
There was a problem hiding this comment.
Resolved in f15b4fe.
There was a problem hiding this comment.
Ah, you decide what is best, consistency wins, not a big deal either way.
There was a problem hiding this comment.
This change broke the test API_MinorVersionsHaveNoBreakingChanges on my branch.
It took me a while to trace it here because CI on develop and on this branch both seemed to have passed... 😸
Secticide
force-pushed
the
ISXB-251-Action-Vector3-Cancel-Fix
branch
from
f15b4fe to
d5eadf9
Compare
…r tweaks to bring consistency between Vector2 and Vector3 composites
Secticide
force-pushed
the
ISXB-251-Action-Vector3-Cancel-Fix
branch
from
d5eadf9 to
ce8785f
Compare
Description
Issue tracker: ISXB-251.
When an action is set to
Vector3Compositeusing keyboard keys would result in started and performed callbacks firing but a cancel callback would only fire when losing focus. This doesn't map to what is explained in the documentation.The issue was caused by InputActionState.IsActuated returning true consistently for
Vector3Compositeactions. The result when callingtrigger.magnitudewas always -1 due toVector3Compositenot overridingEvaluateMagnitude(this was already resolved inVector2Composite).Changes made
EvaluateMagnitudeoverride toVector3Composite.Vector3Composite&Vector2CompositeVector3Composite&Vector2Compositeto be more consistentNotes
While changes have been made to
Vector3Composite&Vector2Compositefor consistency, they are entirely cosmetic.