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.

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

Button.OnButtonClicked is triggered twice on clicks #1879

Closed
thiemok opened this issue Mar 26, 2018 · 16 comments
Closed

Button.OnButtonClicked is triggered twice on clicks #1879

thiemok opened this issue Mar 26, 2018 · 16 comments
Assignees
Projects

Comments

@thiemok
Copy link

thiemok commented Mar 26, 2018

Overview

OnButtonClicked is triggered twice on input clicks. Once from the InputClickedEvent and once from InputDown.
This happens spread over some frames and thus causes unintended side effects with dialogs and button actions that spawn gameObjects infront of the user. For example the first InpudClicked event will open a Dialog and the second will imediately dismiss it.

buttonclicktrace

Expected Behavior

OnInputClicked is only fired once

Actual Behavior

OnInputClicked is fired twice

Steps to reproduce

  • Create empty scene
  • Add default MRTK stuff
  • Add Button
  • Trace OnButtonClicked

Unity Editor Version

2017.3.1f1

Mixed Reality Toolkit Release Version

dev branch @ d46126f

Edit: Added commit hash

@braden-smith
Copy link

Similar with the HolographicButton prefab in dev working branch. Unity 2017.2.1p4

https://forums.hololens.com/discussion/6999/ui-button-on-click-event-firing-twice#latest

@thiemok
Copy link
Author

thiemok commented Mar 26, 2018

I'm currently solving this by simply debouncing the invocation of OnButtonClicked a few hundered milliseconds, but haven't had the time to test if this has any unintended side effects.

@StephenHodgson
Copy link
Contributor

eventData.Use();

@thiemok
Copy link
Author

thiemok commented Mar 26, 2018

Button.OnButtonClicked recieves a GameObject as payload, no eventData. the trace i attached also shows that the calls originate from two different event sources (InputDown and InputClicked).

@StephenHodgson
Copy link
Contributor

You'll have to show me how you've got it setup in the inspector and in code.

@thiemok
Copy link
Author

thiemok commented Mar 26, 2018

Unfortunately i just left my windows machine at work, but i will try to flesh out my description a bit.

Reproduction

This seems to be not reproducible in the Unity Editor, it only fails on the HoloLens itself.

  • Create an empty scene
  • Add MRTK defaults (InputManager, Cursor, Camera, ...)
  • Add an instance of the HolographicButton prefab (or any other button that uses the Button script)
  • Add the following script to the button (Please forgive smaller errors, since i'm typing this of the top of my head)
using UnityEngine;
using MixedRealityToolkit.UX.Buttons;

class Test : MonoBehaviour {
  int frame = 0;

  private void Start() {
    Button button = this.GetComponent<Button>();
    button.OnButtonClicked += (target) => {
      Debug.Log(frame);
      Debug.Log(System.Environment.StackTrace);
    }
  }

  private void Update() {
    frame++;
  }
}

Run.

Issue

The Issue seems to be in the handling of input events in Button.cs itself. I'll try to outline it here.
Input events were triggered for me in the following order: InputDown, InputUp, InputClicked

public void OnInputDown(InputEventData eventData)
        {
            if (enabled)
            {
                if(ButtonPressFilter == InteractionSourcePressInfo.None || ButtonPressFilter == eventData.PressType)
                {
                    DoButtonPressed(); // This calls button pressed on 

                    // Set state to Pressed
                    ButtonStateEnum newState = ButtonStateEnum.Pressed;
                    this.OnStateChange(newState);
                }
            }
        }

public void OnInputClicked(InputClickedEventData eventData)
        {
            if (enabled)
            {
                if (ButtonPressFilter == InteractionSourcePressInfo.None || ButtonPressFilter == eventData.PressType)
                {
                    DoButtonPressed(true);
                }
            }
        }

// As you can see both events result in DuButtonPressed() being called.
// This then triggers OnButtonClicked
        protected void DoButtonPressed(bool bRelease = false)
        {
            ButtonStateEnum newState = ButtonStateEnum.Pressed;
            this.OnStateChange(newState);

            if (OnButtonPressed != null)
            {
                OnButtonPressed(gameObject);
            }

            if(OnButtonClicked != null)
            {
                OnButtonClicked(gameObject);
            }

            if (bRelease)
            {
                StartCoroutine(DelayedRelease(0.2f));
            }
        }

@braden-smith
Copy link

here's a basic project where i am trying to use additional button in AppBar

http://bradensmith.com/vrar/_testHoloButtons.7z

Click on button 'test' and it prints debug line twice

@StephenHodgson
Copy link
Contributor

Not a fan of just downloading binaries. Could you make a public repo here on Github?

@StephenHodgson
Copy link
Contributor

public void OnInputDown(InputEventData eventData)
        {
            if (enabled)
            {
                if(ButtonPressFilter == InteractionSourcePressInfo.None || ButtonPressFilter == eventData.PressType)
                {
                    DoButtonPressed(); // This calls button pressed on 

                    // Set state to Pressed
                    ButtonStateEnum newState = ButtonStateEnum.Pressed;
                    this.OnStateChange(newState);
                    eventData.Use(); // <-- Call this to prevent input from falling to the next handler.
                }
            }
        }

public void OnInputClicked(InputClickedEventData eventData)
        {
            if (enabled)
            {
                if (ButtonPressFilter == InteractionSourcePressInfo.None || ButtonPressFilter == eventData.PressType)
                {
                    DoButtonPressed(true);
                    eventData.Use(); // <-- Call this to prevent input from falling to the next handler.
                }
            }
        }

@thiemok
Copy link
Author

thiemok commented Mar 27, 2018

I created a test scene for you in here:
https://github.com/thiemok/MixedRealityToolkit-Unity/tree/feature/button_test
You can find it in Assets/buttonTest.unity
I also went ahead and and added eventData.Use() to Button.cs, to show that the problems is not created by rehandeled events.
thiemok@ff2c4e3

Disclaimer: i could not base the test of the current dev branch, because of some exception, so i used d46126f

@MSDNAndi
Copy link

MSDNAndi commented Apr 9, 2018

I have the same or a similar problem it seems. Added eventData.Use() ... still both OnInputDown and OnInputClicked are called.

Seems in my case at least one event is from GestureRecognizer_Tapped, the other the other
InteractionManager_InteractionSourcePressed.

both coming from InteractionInputSource
In my case I temporarily set GestureRecognizer to manual start in my scene until I find/decide on a better way to handle it.

In the editor/player the call stack is:
1)

>	Void MixedRealityToolkit.UX.Buttons.Button:OnInputDown (InputEventData)+0x34 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\UX\Scripts\Buttons\Button.cs:126	C#
 	Void MixedRealityToolkit.InputModule.InputManager:<OnSourceDownEventHandler>m__6 (IInputHandler, BaseEventData)+0xa at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputManager.cs:449	C#
 	Boolean UnityEngine.EventSystems.ExecuteEvents:Execute (GameObject, BaseEventData, EventFunction`1)+0x73 at C:\buildslave\unity\build\Extensions\guisystem\UnityEngine.UI\EventSystem\ExecuteEvents.cs:261	C#
 	GameObject UnityEngine.EventSystems.ExecuteEvents:ExecuteHierarchy (GameObject, BaseEventData, EventFunction`1)+0x28 at C:\buildslave\unity\build\Extensions\guisystem\UnityEngine.UI\EventSystem\ExecuteEvents.cs:286	C#
 	Void MixedRealityToolkit.InputModule.InputManager:HandleEvent (BaseEventData, EventFunction`1)+0x147 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputManager.cs:295	C#
 	Void MixedRealityToolkit.InputModule.InputManager:RaiseSourceDown (IInputSource, UInt32, InteractionSourcePressInfo, Object[])+0x1d at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputManager.cs:458	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:SendControllerStateEvents (Single)+0x31 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:408	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:UpdateControllerState (DebugInteractionSourceState)+0x1a6 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:395	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:UpdateControllerData ()+0x6f at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:338	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:Update ()+0x12 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:275	C#

2)

>	Void MixedRealityToolkit.UX.Buttons.Button:OnInputClicked (InputClickedEventData)+0x34 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\UX\Scripts\Buttons\Button.cs:163	C#
 	Void MixedRealityToolkit.InputModule.InputManager:<OnInputClickedEventHandler>m__4 (IInputClickHandler, BaseEventData)+0xa at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputManager.cs:401	C#
 	Boolean UnityEngine.EventSystems.ExecuteEvents:Execute (GameObject, BaseEventData, EventFunction`1)+0x73 at C:\buildslave\unity\build\Extensions\guisystem\UnityEngine.UI\EventSystem\ExecuteEvents.cs:261	C#
 	GameObject UnityEngine.EventSystems.ExecuteEvents:ExecuteHierarchy (GameObject, BaseEventData, EventFunction`1)+0x28 at C:\buildslave\unity\build\Extensions\guisystem\UnityEngine.UI\EventSystem\ExecuteEvents.cs:286	C#
 	Void MixedRealityToolkit.InputModule.InputManager:HandleEvent (BaseEventData, EventFunction`1)+0x147 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputManager.cs:295	C#
 	Void MixedRealityToolkit.InputModule.InputManager:RaiseInputClicked (IInputSource, UInt32, InteractionSourcePressInfo, Int32, Object[])+0x1f at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputManager.cs:410	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:SendControllerStateEvents (Single)+0xc2 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:428	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:UpdateControllerState (DebugInteractionSourceState)+0x1a6 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:395	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:UpdateControllerData ()+0x6f at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:338	C#
 	Void MixedRealityToolkit.InputModule.InputSources.CustomInputSource:Update ()+0x12 at D:\Dev\Unity3D\Customer\CustomerProcess\Assets\MixedRealityToolkit\InputModule\Scripts\InputSources\CustomInputSource.cs:275	C#

While debugging on HoloLens, the callstack is
1)

>	Assembly-CSharp.dll!MixedRealityToolkit.UX.Buttons.Button.OnInputDown(MixedRealityToolkit.InputModule.EventData.InputEventData eventData) Line 126	Unknown
 	Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputManager..cctor.AnonymousMethod__114_6(MixedRealityToolkit.InputModule.InputHandlers.IInputHandler handler, UnityEngine.EventSystems.BaseEventData eventData) Line 449	Unknown
 	[External Code]	
 	Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputManager.HandleEvent<MixedRealityToolkit.InputModule.InputHandlers.IInputHandler>(UnityEngine.EventSystems.BaseEventData eventData, UnityEngine.EventSystems.ExecuteEvents.EventFunction<MixedRealityToolkit.InputModule.InputHandlers.IInputHandler> eventHandler) Line 295	Unknown
 	Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputManager.RaiseSourceDown(MixedRealityToolkit.InputModule.InputSources.IInputSource source, uint sourceId, MixedRealityToolkit.InputModule.InputSources.InteractionSourcePressInfo pressType, object[] tags) Line 458	Unknown
 	**Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputSources.InteractionInputSource.InteractionManager_InteractionSourcePressed(UnityEngine.XR.WSA.Input.InteractionSourcePressedEventArgs args)** Line 930	Unknown
 	[External Code]	

2)

>	Assembly-CSharp.dll!MixedRealityToolkit.UX.Buttons.Button.OnInputClicked(MixedRealityToolkit.InputModule.EventData.InputClickedEventData eventData) Line 163	Unknown
 	Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputManager..cctor.AnonymousMethod__114_4(MixedRealityToolkit.InputModule.InputHandlers.IInputClickHandler handler, UnityEngine.EventSystems.BaseEventData eventData) Line 401	Unknown
 	[External Code]	
 	Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputManager.HandleEvent<MixedRealityToolkit.InputModule.InputHandlers.IInputClickHandler>(UnityEngine.EventSystems.BaseEventData eventData, UnityEngine.EventSystems.ExecuteEvents.EventFunction<MixedRealityToolkit.InputModule.InputHandlers.IInputClickHandler> eventHandler) Line 295	Unknown
 	Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputManager.RaiseInputClicked(MixedRealityToolkit.InputModule.InputSources.IInputSource source, uint sourceId, MixedRealityToolkit.InputModule.InputSources.InteractionSourcePressInfo pressType, int tapCount, object[] tags) Line 410	Unknown
 	**Assembly-CSharp.dll!MixedRealityToolkit.InputModule.InputSources.InteractionInputSource.GestureRecognizer_Tapped(UnityEngine.XR.WSA.Input.TappedEventArgs args) Line 957**	Unknown
 	[External Code]	

@Alexees
Copy link
Contributor

Alexees commented Jun 27, 2018

DoButtonPressed is the problem here. It contains both, OnButtonPressed and OnButtonClicked, while it's been called from both, OnInputDown and OnInputClicked.

@ivan2007
Copy link

Maybe this problem is related with #2040

@keveleigh
Copy link
Contributor

I have a rework in progress, so I'll be sure to double check #2040 as well.

@keveleigh
Copy link
Contributor

The PopupMenu issue ended up needing a different fix, since it uses a different button class than this one. I've noted that, so we can clean up the various overlapping UX components.

@keveleigh
Copy link
Contributor

keveleigh commented Jun 29, 2018

Just opened #2367 to remove the duplicate events firing.

@keveleigh keveleigh moved this from In progress to Done in 2017.4.1.0 Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
2017.4.1.0
  
Done
Development

No branches or pull requests

8 participants