Skip to content

Conversation

Troy-Ferrell
Copy link
Contributor

@Troy-Ferrell Troy-Ferrell commented May 31, 2019

Overview

There are two key dependencies between profiles/inputhandlers: input actions and speech keywords (i.e speechinputhandler). The inspector logic for these impacted components has been updated to be simpler, more consistent, and accurate with useful error messages to the user informing them of the issue (i.e no MRTK in scene, etc etc.).

Rendering of speech commands when rendered as sub-profile view (i.e within input profile) would not work properly for layout. Switched model to be vertical components to preserve useful formatting

Added forced height on scrollview for input actions inspector when rendered as sub-profile

Fixed edge case where inspector logic does not account for an active profile assigned to the scene MRTK instance (an unlikely edge case)

Fixed DefaultMouseProfile to correctly set IsCustomProfile to false

Fixed DefaultHololens2..profiles to correctly set IsCustomProfile to false

Fixed "Back to.." buttons for profile up the hierarchy to actually render

Changes

Verification

Removed input actions profile
Removed active profile
Removed MRTK instance
Switched hierarchy with multiple gesture profiles (but only one active in instance)
Deleted MRTK instance & re-added with SpeechInputHandler button when error message shows

@Troy-Ferrell Troy-Ferrell requested a review from Railboy May 31, 2019 15:03
@Troy-Ferrell Troy-Ferrell self-assigned this May 31, 2019
@Troy-Ferrell Troy-Ferrell requested a review from wiwei May 31, 2019 15:04
@Troy-Ferrell
Copy link
Contributor Author

@luis-valverde-ms and @provencher FYI

Copy link

@david-c-kline david-c-kline left a comment

Choose a reason for hiding this comment

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

Please reach out if you have any questions re: feedback. Thanks!

@Troy-Ferrell
Copy link
Contributor Author

After syncing with @davidkline-ms , the use of MRTK object references are going to remain in the PR. This is because

  1. These are only editor code additions, not runtime use additions
  2. The TryGetService & related paradigm needs to be rationalized for editor use since MRTK is not actually running

@Troy-Ferrell
Copy link
Contributor Author

/azp run mrtk_PR

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link

@david-c-kline david-c-kline left a comment

Choose a reason for hiding this comment

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

Thanks for taking the time to discuss this change @Troy-Ferrell!

@wiwei wiwei merged commit 9dc46c0 into microsoft:mrtk_development Jun 5, 2019
@wiwei
Copy link
Contributor

wiwei commented Jun 5, 2019

Thanks!

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