Minor Refactoring and Cleanup #827

StephenHodgson · 2017-08-03T16:25:59Z

Moved some classes that were not part of a proper namespace into one.
Cleaned up formatting and fixed misspellings.
Added LCA headers.
Updated UNITY_METRO to UNITY_WSA
Fixed support for UnityEngine.Windows.Speech

… for public properties)

…own file.

Added LCA Headers. Fixed misspellings and formatting.

Minor formatting and cleanup.

keveleigh

I was starting work on a PR like this, so I'm happy to see it!

keveleigh · 2017-08-03T19:30:33Z

Assets/HoloToolkit/Input/Scripts/InputSources/GesturesInput.cs

@@ -3,7 +3,7 @@

 using UnityEngine;

-#if UNITY_EDITOR || UNITY_WSA


Why did all the UNITY_EDITOR directives get removed? It's useful to use these APIs in the editor when using Unity remoting.

UNITY_EDITOR is redundant even when remoting

Ahh, so this code will still run in the editor with the UNITY_WSA flag? Sweet, I didn't know that!

Really it shouldn't matter even when remoting because the build should already be WSA

Sounds good

keveleigh · 2017-08-03T19:31:26Z

Assets/HoloToolkit-Examples/InteractiveElements/Scripts/GestureInteractive.cs


-#if UNITY_EDITOR || UNITY_WSA
+#if UNITY_WSA || UNITY_STANDALONE


UNITY_STANDALONE should probably be UNITY_STANDALONE_WIN, since I doubt the Windows Speech APIs will work on Mac or Linux :)

Thanks. That's exactly what I was attempting to do lol

keveleigh · 2017-08-03T19:37:46Z

Assets/HoloToolkit/SpatialMapping/Scripts/SpatialProcessing/SurfaceMeshesToPlanes.cs

-    /// Editor extension class to enable multi-selection of the 'Draw Planes' and 'Destroy Planes' options in the Inspector.
-    /// </summary>
-    [CustomEditor(typeof(SurfaceMeshesToPlanes))]
-    public class PlaneTypesEnumEditor : Editor


What happened to all this?

It was moved into it's own class.

I wasn't able to find it anywhere in this PR. The only added file I see is SupportedInputInfo.cs (and its meta).

I'll double check. Thanks!

Fixed, and added.

keveleigh · 2017-08-03T19:57:25Z

Assets/HoloToolkit/SpatialSound/Scripts/UAudioManager/UAudioManager.cs

            {
-                if (currentEvent.instanceBehavior == AudioEventInstanceBehavior.KillNewest)
+                if (currentEvent.AudioEventInstance == AudioEventInstanceBehavior.KillNewest)


nit: this name change removes the word "Behavior", causing it to be less accurate and less clear what it's referring to.

Sounds good. I'll update.

keveleigh · 2017-08-03T19:59:57Z

Assets/HoloToolkit/UI/Scripts/KeyboardInputField.cs

-            Keyboard.Instance.onTextUpdated += this.Keyboard_onTextUpdated;
-            Keyboard.Instance.onClosed += this.Keyboard_onClosed;
+            // Subscribe to keyboard delegates
+            Keyboard.Instance.OnTextUpdated += Keyboard_onTextUpdated;


Should probably update Keyboard_onTextUpdated -> Keyboard_OnTextUpdated (and Keyboard_onClosed) as well.

…Left UNITY_STANDALONE for Input only events. Added missing meta file.

StephenHodgson · 2017-08-03T20:59:54Z

Thanks for the review @keveleigh, fixes are in.

ForrestTrepte

Thanks for tidying things up, Stephen!

ForrestTrepte · 2017-08-07T17:14:37Z

Out of curiosity, is there a utility that you use for finding things such as spelling errors in comments?

StephenHodgson · 2017-08-07T22:22:57Z

@ForrestTrepte Resharper. But I don't use it all the time cause it's a resource hog.

Zod- · 2017-08-08T15:41:45Z

Assets/HoloToolkit/Input/Scripts/InputManager.cs

@@ -727,6 +726,6 @@ public void RaiseDictationError(IInputSource source, uint sourceId, string dicta
        }

        #endregion // Dictation Events
-
+#endif


Usage of RaiseDictationError, RaiseDictationHypothesis, RaiseDictationComplete and RaiseDictationResult also has to be wrapped like this.
Since I didn't wrap these before I also didn't realize they have to be wrapped where they are used in the DictationInputManager. Result and Complete are already wrapped so it just has to be extended a little bit to include Hypothesis and Error.

Getting 2 errors for those now in the Editor:

Assets/HoloToolkit/Input/Scripts/Microphone/DictationInputManager.cs(248,35): error CS1061: Type `HoloToolkit.Unity.InputModule.InputManager' does not contain a definition for `RaiseDictationError' and no extension method `RaiseDictationError' of type `HoloToolkit.Unity.InputModule.InputManager' could be found. Are you missing an assembly reference? Assets/HoloToolkit/Input/Scripts/Microphone/DictationInputManager.cs(200,35): error CS1061: Type `HoloToolkit.Unity.InputModule.InputManager' does not contain a definition for `RaiseDictationHypothesis' and no extension method `RaiseDictationHypothesis' of type `HoloToolkit.Unity.InputModule.InputManager' could be found. Are you missing an assembly reference?

I'll look into it right now. Was this in standalone pc build, or wsa?

I just opened the project in Unity. Are those flags added when you change the target platform? If they are then probably neither. I'll get Unity on this machine and take a look.

What's your build target platform?

Sorry took a bit longer, had to install unity on my machine at home. I tested it earlier on my workstation. I get the errors with android as the target platform.

No worries, I figured. Should be addressed in my latest PR

Minor Refactoring and Cleanup

StephenHodgson added 7 commits April 19, 2017 11:22

Updated uAudio classes to proper property naming formats. (PascalCase…

a473ee2

… for public properties)

Merge branch 'HTK-Master' into HTK-Audio

57d8ae4

Just updated a few namespaces, and broke out an enum class into it's …

bdd4df9

…own file.

Merge branch 'HTK-local' into HTK-Audio

6e9994a

Merge branch 'HTK-Master' into HTK-Refactor

c8b68b4

Merge branch 'HTK-Audio' into HTK-Refactor

881f5a3

Added AppViewManagerEditButton to a namespace.

c4a59fd

Added LCA Headers. Fixed misspellings and formatting.

msftclas added the cla-already-signed label Aug 3, 2017

Replaced UNITY_METRO with UNITY_WSA.

ad146a3

Minor formatting and cleanup.

keveleigh suggested changes Aug 3, 2017

View reviewed changes

StephenHodgson added 5 commits August 3, 2017 13:42

Updated UNITY_STANDALONE to UNITY_STANDALONE_WIN for Windows.Speech. …

ce839f9

…Left UNITY_STANDALONE for Input only events. Added missing meta file.

Added missing Editor script for SurfaceMeshesToPlanes

2f6945c

Updated callback method names per CR

20ae18d

updated some field names per CR

ca745cd

missing change from last push

0278cf9

keveleigh approved these changes Aug 3, 2017

View reviewed changes

ForrestTrepte approved these changes Aug 7, 2017

View reviewed changes

NeerajW approved these changes Aug 7, 2017

View reviewed changes

NeerajW merged commit 500522b into microsoft:master Aug 7, 2017

Zod- reviewed Aug 8, 2017

View reviewed changes

StephenHodgson mentioned this pull request Aug 8, 2017

fixed cross platform support from last refactor PR #841

Merged

keveleigh pushed a commit to keveleigh/HoloToolkit-Unity that referenced this pull request Aug 11, 2017

Merge pull request microsoft#827 from StephenHodgson/HTK-Refactor

327b007

Minor Refactoring and Cleanup

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor Refactoring and Cleanup #827

Minor Refactoring and Cleanup #827

StephenHodgson commented Aug 3, 2017 •

edited

Loading

keveleigh left a comment

keveleigh Aug 3, 2017

StephenHodgson Aug 3, 2017

keveleigh Aug 3, 2017 •

edited

Loading

StephenHodgson Aug 3, 2017

keveleigh Aug 3, 2017

keveleigh Aug 3, 2017

StephenHodgson Aug 3, 2017

keveleigh Aug 3, 2017

StephenHodgson Aug 3, 2017

keveleigh Aug 3, 2017 •

edited

Loading

StephenHodgson Aug 3, 2017

StephenHodgson Aug 3, 2017

keveleigh Aug 3, 2017

StephenHodgson Aug 3, 2017

keveleigh Aug 3, 2017

StephenHodgson commented Aug 3, 2017

ForrestTrepte left a comment

ForrestTrepte commented Aug 7, 2017

StephenHodgson commented Aug 7, 2017 •

edited

Loading

Zod- Aug 8, 2017 •

edited

Loading

StephenHodgson Aug 8, 2017

Zod- Aug 8, 2017 •

edited

Loading

StephenHodgson Aug 8, 2017 •

edited

Loading

Zod- Aug 8, 2017 •

edited

Loading

StephenHodgson Aug 8, 2017

		@@ -3,7 +3,7 @@

		using UnityEngine;

		#if UNITY_EDITOR \|\| UNITY_WSA


		#if UNITY_EDITOR \|\| UNITY_WSA
		#if UNITY_WSA \|\| UNITY_STANDALONE

Minor Refactoring and Cleanup #827

Minor Refactoring and Cleanup #827

Conversation

StephenHodgson commented Aug 3, 2017 • edited Loading

keveleigh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keveleigh Aug 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keveleigh Aug 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenHodgson commented Aug 3, 2017

ForrestTrepte left a comment

Choose a reason for hiding this comment

ForrestTrepte commented Aug 7, 2017

StephenHodgson commented Aug 7, 2017 • edited Loading

Zod- Aug 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zod- Aug 8, 2017 • edited Loading

Choose a reason for hiding this comment

StephenHodgson Aug 8, 2017 • edited Loading

Choose a reason for hiding this comment

Zod- Aug 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StephenHodgson commented Aug 3, 2017 •

edited

Loading

keveleigh Aug 3, 2017 •

edited

Loading

keveleigh Aug 3, 2017 •

edited

Loading

StephenHodgson commented Aug 7, 2017 •

edited

Loading

Zod- Aug 8, 2017 •

edited

Loading

Zod- Aug 8, 2017 •

edited

Loading

StephenHodgson Aug 8, 2017 •

edited

Loading

Zod- Aug 8, 2017 •

edited

Loading