Skip to content

fix blink locomotion when changing player scale - #323

Merged
AndrewTHEManeri merged 2 commits into
stagingfrom
blink-during-scale
Sep 28, 2017
Merged

fix blink locomotion when changing player scale#323
AndrewTHEManeri merged 2 commits into
stagingfrom
blink-during-scale

Conversation

@stella3d

@stella3d stella3d commented Sep 28, 2017

Copy link
Copy Markdown
Contributor

fixes #300

tested that everything else in locomotion still works as expected.
now, if you release the blink button while in the middle of changing player scale, the blink is cancelled.

I'm not wild about having to do it like this - with the if(DoThing) return; pattern, but as far as the scope of this bugfix goes, i think it's the right solution.
Perhaps there is a better way ? it seems like this pattern in general will lead to more instances of control being consumed in a way that prevents another system from working.

@stella3d stella3d self-assigned this Sep 28, 2017
@AndrewTHEManeri

Copy link
Copy Markdown
Contributor

I think the 'proper' solution to this is instead of returning when a locomotion method is used, recording that fact and passing it down to each function in the line, so that they can handle the situation themselves. In that scenario, activating two-handed scale could just cancel the blink out. I'll make an issue for this.

But, in terms of a fix for this release I'm fine with this solution

@AndrewTHEManeri AndrewTHEManeri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should cancel a blink if active here, rather than just performing it. Right now you can't do a locomotion reset (scale+translation) without immediately blinking afterwards

@stella3d

Copy link
Copy Markdown
Contributor Author

ok, changing it to cancel blink

@stella3d

Copy link
Copy Markdown
Contributor Author

blink cancels now !

@mtschoen-unity

Copy link
Copy Markdown
Collaborator

Yeah... the if (didlocomote) return pattern was a little more sensible when there wasn't anything that needed to happen when transitioning between states. Blink visuals and feedback really complicated things.

I'm open to restructuring this tool, but I'm not sure of the best solution. Maybe something more like a state machine?

@stella3d

Copy link
Copy Markdown
Contributor Author

@mtschoen-unity something like that is probably sensible, but that's a bigger discussion. for the purposes of this release i think just fixing this one bug instance is what we want

@AndrewTHEManeri

Copy link
Copy Markdown
Contributor

Excellent

@AndrewTHEManeri
AndrewTHEManeri merged commit e6c1c0e into staging Sep 28, 2017
@AndrewTHEManeri
AndrewTHEManeri deleted the blink-during-scale branch September 28, 2017 23:57
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

Successfully merging this pull request may close these issues.

3 participants