Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Made NonNativeKeyboard touchable #7611

Merged
merged 4 commits into from Apr 14, 2020
Merged

Made NonNativeKeyboard touchable #7611

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
199ae59
Made NonNativeKeyboard touchable
LocalJoost Apr 3, 2020
9f5f831
Changed AddComponent to EnsureComponent as per reviewer's suggestion
LocalJoost Apr 3, 2020
5850336
Changed pragma def into null init
LocalJoost Apr 3, 2020
e97553f
Removed unneccesary empty line and added summary
LocalJoost Apr 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions Assets/MRTK/SDK/Experimental/NonNativeKeyboard/Prefabs/NonNativeKeyboard.prefab
View file
Open in desktop
Expand Up @@ -12617,6 +12617,7 @@ GameObject:
- component: {fileID: 114593871428788132}
- component: {fileID: 6050639456931552995}
- component: {fileID: 583446739}
- component: {fileID: 6363978764271269843}
m_Layer: 0
m_Name: NonNativeKeyboard
m_TagString: Untagged
Expand Down Expand Up @@ -12767,6 +12768,19 @@ MonoBehaviour:
additionalOffset: {x: 0, y: 0, z: 0}
additionalRotation: {x: 0, y: 0, z: 0}
updateSolvers: 1
--- !u!114 &6363978764271269843
MonoBehaviour:
m_ObjectHideFlags: 0
m_CorrespondingSourceObject: {fileID: 0}
m_PrefabInstance: {fileID: 0}
m_PrefabAsset: {fileID: 0}
m_GameObject: {fileID: 146120}
m_Enabled: 1
m_EditorHideFlags: 0
m_Script: {fileID: 11500000, guid: c72a967253428ca4896d1e2639256334, type: 3}
m_Name:
m_EditorClassIdentifier:
_clickSound: {fileID: 8300000, guid: 291bf9326e517b0489c2ee53d0a6a63f, type: 3}
--- !u!1 &146124
GameObject:
m_ObjectHideFlags: 0
Expand Down
51 changes: 51 additions & 0 deletions Assets/MRTK/SDK/Experimental/NonNativeKeyboard/Scripts/NonNativeKeyboardTouchAssistant.cs
View file
Open in desktop
@@ -0,0 +1,51 @@
using Microsoft.MixedReality.Toolkit.Input;
using UnityEngine;
using UnityEngine.UI;


keveleigh marked this conversation as resolved.
Show resolved Hide resolved
namespace Microsoft.MixedReality.Toolkit.Experimental.UI
{
public class NonNativeKeyboardTouchAssistant : MonoBehaviour
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to make this script run at edit time. No need to add these components at runtime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not like this a very heavy procedure and I am not quite a fan of modifying things at edit time. Messing with the internals of the keyboard changes the scene permanently, not sure if I would like that

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess my preference is to simply permanently modify the prefab, rather than having a modifier that adds it. I understand both perspectives though

Copy link
Contributor

@keveleigh keveleigh Apr 3, 2020 •
edited

Choose a reason for hiding this comment

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

Is there a downside to just updating the prefab to contain all the relevant NearInteractionTouchableUnityUI and audioclip components? And leaving them there regardless of if touch is supported? Especially if this script is already being added to the prefab by default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You might not always want this, and this way it's easy to disable it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a summary and the AddComponentMenu attribute here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Summary, okay, but it's already on the prefab, as suggested by @provencher in the Slack channel. You are not going to add this thing to anything - at most you are going to disable it. At least, that was my idea

{
#pragma warning disable 0649
[SerializeField]
private AudioClip _clickSound;
#pragma warning restore 0649
keveleigh marked this conversation as resolved.
Show resolved Hide resolved


private AudioSource _clickSoundPlayer;
keveleigh marked this conversation as resolved.
Show resolved Hide resolved

private void Start()
{
var capabilityChecker = CoreServices.InputSystem as IMixedRealityCapabilityCheck;

if(capabilityChecker != null && capabilityChecker.CheckCapability(MixedRealityCapability.ArticulatedHand))
{
EnableTouch();
}
}

private void EnableTouch()
{
_clickSoundPlayer = gameObject.AddComponent<AudioSource>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure component here too

Copy link
Contributor Author

@LocalJoost LocalJoost Apr 3, 2020 •
edited

Choose a reason for hiding this comment

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

Nope. That might mess up an existing AudioSource component and fill it with my sound file. This is intentional

_clickSoundPlayer.playOnAwake = false;
_clickSoundPlayer.spatialize = true;
_clickSoundPlayer.clip = _clickSound;
var buttons = GetComponentsInChildren<Button>();
foreach (var button in buttons)
{
var ni = button.gameObject.EnsureComponent<NearInteractionTouchableUnityUI>();
ni.EventsToReceive = TouchableEventType.Pointer;
button.onClick.AddListener(PlayClick);
}
}

private void PlayClick()
{
if (_clickSound != null)
{
_clickSoundPlayer.Play();
}
}
}
}
11 changes: 11 additions & 0 deletions ...s/MRTK/SDK/Experimental/NonNativeKeyboard/Scripts/NonNativeKeyboardTouchAssistant.cs.meta
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.