Search Unity

corrected GUI.Button code (works properly with layered controls)

Discussion in 'Scripting' started by JoeStrout, Jul 13, 2011.

  1. JoeStrout

    JoeStrout

    Joined:
    Jan 14, 2011
    Posts:
    8,138
    As discussed in this thread, and also on Unity Answers, there is a major problem with things like pop-up menus when they happen to overlap GUI buttons.

    After some investigating, we've found it to be a flaw in the GUI.Button method: it's not properly honoring the GUIUtility.hotControl indicator. When that's not set to either 0 or the button's own ID, it should ignore mouse events completely. Instead, while it does refrain from drawing in mouse-over style, it nonetheless handles a mouseDown event that was intended for some other control.

    We banged out a replacement function that appears to work correctly in our tests. Here it is -- just put it someplace convenient in your code, and replace your calls to GUI.Button with calls to goodButton:

    Code (csharp):
    1.     bool goodButton(Rect bounds, string caption) {
    2.         GUIStyle btnStyle = GUI.skin.FindStyle("button");
    3.         int controlID = GUIUtility.GetControlID(bounds.GetHashCode(), FocusType.Passive);
    4.        
    5.         bool isMouseOver = bounds.Contains(Event.current.mousePosition);
    6.         bool isDown = GUIUtility.hotControl == controlID;
    7.  
    8.         if (GUIUtility.hotControl != 0  !isDown) {
    9.             // ignore mouse while some other control has it
    10.             // (this is the key bit that GUI.Button appears to be missing)
    11.             isMouseOver = false;
    12.         }
    13.        
    14.         if (Event.current.type == EventType.Repaint) {
    15.             btnStyle.Draw(bounds, new GUIContent(caption), isMouseOver, isDown, false, false);
    16.         }
    17.         switch (Event.current.GetTypeForControl(controlID)) {
    18.             case EventType.mouseDown:
    19.                 if (isMouseOver) {  // (note: isMouseOver will be false when another control is hot)
    20.                     GUIUtility.hotControl = controlID;
    21.                 }
    22.                 break;
    23.                
    24.             case EventType.mouseUp:
    25.                 if (GUIUtility.hotControl == controlID) GUIUtility.hotControl = 0;
    26.                 if (isMouseOver  bounds.Contains(Event.current.mousePosition)) return true;
    27.                 break;
    28.         }
    29.  
    30.         return false;
    31.     }
    32.  
    Note that, contrary to claims elsewhere, you do not need to put your overlapping controls into separate MonoBehaviours. You just need a button function that's not broken. ;)

    Enjoy,
    - Joe
     
    Last edited: Jul 13, 2011
  2. Overunity3d

    Overunity3d

    Joined:
    Sep 5, 2010
    Posts:
    14
    Is it possible to get a JavaScript version?
    The Bounds parameter in the
    is a little difficult to work out.

    TIA.
     
  3. r618

    r618

    Joined:
    Jan 19, 2009
    Posts:
    795
    the JS version / not thoroughly tested , just replaced declaration at the beginning.... /:

    Code (csharp):
    1.  
    2. function goodButton(bounds: Rect, caption: String): boolean
    3.     {
    4.         var btnStyle : GUIStyle = GUI.skin.FindStyle("button");
    5.         var controlID : int = GUIUtility.GetControlID(bounds.GetHashCode(), FocusType.Passive);
    6.        
    7.         var isMouseOver : boolean = bounds.Contains(Event.current.mousePosition);
    8.         var isDown : boolean = GUIUtility.hotControl == controlID;
    9.  
    10.         if (GUIUtility.hotControl != 0  !isDown) {
    11.             // ignore mouse while some other control has it
    12.             // (this is the key bit that GUI.Button appears to be missing)
    13.             isMouseOver = false;
    14.         }
    15.        
    16.         if (Event.current.type == EventType.Repaint) {
    17.             btnStyle.Draw(bounds, new GUIContent(caption), isMouseOver, isDown, false, false);
    18.         }
    19.         switch (Event.current.GetTypeForControl(controlID)) {
    20.             case EventType.mouseDown:
    21.                 if (isMouseOver) {  // (note: isMouseOver will be false when another control is hot)
    22.                     GUIUtility.hotControl = controlID;
    23.                 }
    24.                 break;
    25.                
    26.             case EventType.mouseUp:
    27.                 if (GUIUtility.hotControl == controlID) GUIUtility.hotControl = 0;
    28.                 if (isMouseOver  bounds.Contains(Event.current.mousePosition)) return true;
    29.                 break;
    30.         }
    31.  
    32.         return false;
    33.     }
    34.  
    btw thanks JoeStrout !

    I've been layouting my controls for a while until I realized that the button behavior is probably not exactly as intended :)
     
  4. royaldex

    royaldex

    Joined:
    Feb 6, 2012
    Posts:
    1
    i've been annoyed by this button overlapping problem until i see your post, thank you so much for sharing this solution!
     
  5. AlexisVaillant

    AlexisVaillant

    Joined:
    May 17, 2011
    Posts:
    15
    I don't understand. For me it does the exact same problem. I used the Javascript version translated by r618. Did I missed something ?
     
  6. Whyser

    Whyser

    Joined:
    Jun 3, 2011
    Posts:
    7
    It didn't work for me either at first, my problem was that i was using GUI.Button() for one of the buttons and the code mentioned above for the other. Make sure you use the code above for all buttons actually overlapping.

    Thank you OP for the solution, was really helpful!
     
  7. AlexisVaillant

    AlexisVaillant

    Joined:
    May 17, 2011
    Posts:
    15
    Wow, that's a strange one, because I solved the problem in using both methods : The JoeStrout Button under the GUIButton. When I used the JoeStrout for all, it didn't work.
     
  8. stridervan

    stridervan

    Joined:
    Nov 11, 2011
    Posts:
    141
    i dont think the problem has anything to do with the hotControl, the solution you presented does not call Event.current.Use() which the normal button calls after setting the hotControl to its control Id. Other controls ignores EventType.Used, so you wont get any events processed if the one control sets the "MouseDown" event to "Used".
     
  9. franky303

    franky303

    Joined:
    Aug 27, 2012
    Posts:
    10
    Great work, Joe. - Thanks for sharing this! You've made my day!
     
  10. franky303

    franky303

    Joined:
    Aug 27, 2012
    Posts:
    10
    Okay guys, even though the code is great, i've still had some problems with it: when using overlapping buttons, it would fire button events for EVERY button. Obviously, i only want it to fire an event for the topmost button. Therefore i have come up with the following solution. It is based on Joe's script and solves the problem of overlapping buttons (both: inside the same GUI.depth or with diferent GUI.depths). It also works with scrollviews (e.g. i have a main menu button, it's description can be expanded and then is overlapping a scrollview, which also has buttons). It will fire the event only to the topmost button.

    My first approach was to collect all button rect's and their rollover states during the Event.current.Layout process, and then i will know which button is really the topmost. (This is possible because the button routine is always called TWICE per frame as probably everyone of you know ...)
    With this i had a solution for button clicks, but when there were overlapping buttons inside the same GUI.depth level, still all my buttons were hovering, including the ones "behind" another button. - Therefore i've added a "highest control ID" for rollovers, and this finally simplified the whole script and doesn't need any hashtables anymore. The "highest control ID" is defined by using GUI.depth in combination with the controlID, and only hovered buttons will update it. At the end of the whole Event.current.Layout process, you will have processed all buttons and therefore know the definitive topmost hovered button. When the process status changes (i.e. the Event.current.type changes from EventType.Repaint back to EventType.Layout, a new frame has started and the highest control ID is reset to zero. - This will make sure that the routine starts collecting all button IDs again, so it will also by working for dynamic user interfaces where buttons are added or removed over time ...

    If you are using GUI.depth, make sure you're only using values between 0 and 1000. (or, adjust the values in the script).

    I have also added code for mobile devices, so the button doesn't fire when he's being dragged over. This way it is working nicely for me around and inside scrollViews etc.

    This is what i came up with:

    Code (csharp):
    1.  
    2. /*
    3.  *
    4.  * **** GUIButton CLASS ****
    5.  *
    6.  * this versions sends only events to the topmost button ...
    7.  *
    8.  *
    9.  * Fixes the bugs from the original GUI.Button function
    10.  * Based on the script from Joe Strout:
    11.  * http://forum.unity3d.com/threads/96563-corrected-GUI.Button-code-%28works-properly-with-layered-controls%29?p=629284#post629284
    12.  *
    13.  *
    14.  * The difference in this script is that it will only fire events (click and rollover!)
    15.  * for the topmost button when using overlapping buttons inside the same GUI.depth!
    16.  * Therefore the script finds the topmost button during the layout process, so it
    17.  * can decide which button REALLY has been clicked.
    18.  *
    19.  * Benefits:
    20.  * 1. The script will only hover the topmost button!
    21.  *    (doesn't matter wheter the topmost button is defined via GUI.depth or via drawing order!)
    22.  * 2. The script will only send events to the topmost button (as opposed to Joe's original script)
    23.  * 3. The script works for overlapping buttons inside same GUI.depth levels,
    24.  *    as well as for overlapping buttons using different GUI.depth values
    25.  * 4. The script also works when overlapping buttons over buttons inside scrollviews, etc.
    26.  *
    27.  * Usage:  just like GUI.Button() ... for example:
    28.  *
    29.  *  if ( GUIButton.Button(new Rect(0,0,100,100), "button_action", GUI.skin.customStyles[0]) )
    30.  *  {
    31.  *      Debug.Log( "Button clicked ..." );
    32.  *  }
    33.  *
    34.  *
    35.  *
    36.  * Original script (c) by Joe Strout!
    37.  *
    38.  * Code changes:
    39.  * Copyright (c) 2012 by Frank Baumgartner, Baumgartner New Media GmbH, fb@b-nm.at
    40.  *
    41.  *
    42.  * */
    43.  
    44.  
    45. using UnityEngine;
    46. using System.Collections;
    47.  
    48. public class GUIButton
    49. {
    50.     private static int highestDepthID = 0;
    51.     private static Vector2 touchBeganPosition = Vector2.zero;
    52.     private static EventType lastEventType = EventType.Layout;
    53.  
    54.     private static bool wasDragging = false;
    55.    
    56.     private static int frame = 0;
    57.     private static int lastEventFrame = 0;
    58.    
    59.    
    60.     public static bool Button(Rect bounds, string caption, GUIStyle btnStyle = null )
    61.     {
    62.         int controlID = GUIUtility.GetControlID(bounds.GetHashCode(), FocusType.Passive);
    63.         bool isMouseOver = bounds.Contains(Event.current.mousePosition);
    64.         int depth = (1000 - GUI.depth) * 1000 + controlID;
    65.         if ( isMouseOver  depth > highestDepthID ) highestDepthID = depth;
    66.         bool isTopmostMouseOver = (highestDepthID == depth);
    67. #if (UNITY_IPHONE || UNITY_ANDROID)   !UNITY_EDITOR
    68.         bool paintMouseOver = isTopmostMouseOver  (Input.touchCount > 0);
    69. #else
    70.         bool paintMouseOver = isTopmostMouseOver;
    71. #endif
    72.  
    73.         if ( btnStyle == null )
    74.         {
    75.             btnStyle = GUI.skin.FindStyle("button");
    76.         }
    77.            
    78.         if ( Event.current.type == EventType.Layout  lastEventType != EventType.Layout )
    79.         {
    80.             highestDepthID = 0;
    81.             frame++;
    82.         }
    83.         lastEventType = Event.current.type;
    84.    
    85.         if ( Event.current.type == EventType.Repaint )
    86.         {
    87.             bool isDown = (GUIUtility.hotControl == controlID);
    88.             btnStyle.Draw(bounds, new GUIContent(caption), paintMouseOver, isDown, false, false);          
    89.            
    90.         }
    91.        
    92. #if (UNITY_IPHONE || UNITY_ANDROID)
    93.        
    94.         if ( Input.touchCount > 0 )
    95.         {
    96.             Touch touch = Input.GetTouch(0);
    97.             if ( touch.phase == TouchPhase.Began )
    98.             {
    99.                 touchBeganPosition = touch.position;
    100.                 wasDragging = true;
    101.             }
    102.             else if ( touch.phase == TouchPhase.Ended  
    103.                             (   (Mathf.Abs(touch.position.x - touchBeganPosition.x) > 15) ||
    104.                                 (Mathf.Abs(touch.position.y - touchBeganPosition.y) > 15)       )
    105.                     )
    106.             {
    107.                 wasDragging = true;
    108.             }
    109.             else
    110.             {
    111.                 wasDragging = false;
    112.             }
    113.         }
    114.         else if ( Event.current.type == EventType.Repaint )
    115.         {
    116.             wasDragging = false;
    117.         }
    118.        
    119. #endif
    120.        
    121.         // Workaround:
    122.         // ignore duplicate mouseUp events. These can occur when running
    123.         // unity editor with unity remote on iOS ... (anybody knows WHY?)
    124.         if ( frame <= (1+lastEventFrame) ) return false;
    125.  
    126.         switch ( Event.current.GetTypeForControl(controlID) )
    127.         {
    128.             case EventType.mouseDown:
    129.             {
    130.                 if ( isTopmostMouseOver  !wasDragging )
    131.                 {
    132.                     GUIUtility.hotControl = controlID;
    133.                 }
    134.                 break;
    135.             }
    136.  
    137.             case EventType.mouseUp:
    138.             {
    139.                 if ( isTopmostMouseOver  !wasDragging )
    140.                 {
    141.                     GUIUtility.hotControl = 0;
    142.                     lastEventFrame = frame;
    143.                     return true;
    144.                 }
    145.                 break;
    146.             }
    147.         }
    148.         return false;
    149.     }
    150. }
    151.  
    152.  
    153.  
    154.  
     
    Last edited: Sep 23, 2012
  11. bedford

    bedford

    Joined:
    Jan 29, 2012
    Posts:
    33
    Hello,

    I need this for a GUI.TextField control. I've spent many hours trying to get it to work but couldn't make it to work. It's possible to do make it to work for a TextField ? If yes, could someone point me on the right direction please ?
     
  12. franky303

    franky303

    Joined:
    Aug 27, 2012
    Posts:
    10
    bedford: try using GUI.depth and place the textfield into a separate GameObject (you need different gameobjects in order to make GUI.depth work)
     
  13. allanolivei

    allanolivei

    Joined:
    Oct 31, 2012
    Posts:
    6
    This is Solution.


    Code (csharp):
    1.  
    2. using UnityEngine;
    3. using System.Collections;
    4.  
    5. public class GUIButton {
    6.    
    7.     private static int highestDepthID = 0;
    8.     private static int targetID;
    9.    
    10.    
    11.     static public bool Button(Rect bounds, string caption, GUIStyle btnStyle)
    12.     {
    13.         int controlID = GUIUtility.GetControlID(bounds.GetHashCode(), FocusType.Passive);
    14.         int depth = (1000 - GUI.depth) * 1000 + controlID;
    15.        
    16.        
    17.         bool result = false;
    18.        
    19.         if(Event.current.type == EventType.Layout)
    20.         {
    21.             bool isMouseOver = bounds.Contains(Event.current.mousePosition);
    22.             if ( isMouseOver  depth > highestDepthID ) { highestDepthID = depth;  targetID = controlID; }
    23.         }
    24.         else if(Event.current.type == EventType.Repaint)
    25.         {
    26.             result = (GUIUtility.hotControl == controlID);
    27.            
    28.             btnStyle.Draw(bounds, new GUIContent(caption), (targetID == controlID), result, false, false);
    29.             if(targetID == controlID) { highestDepthID = 0; targetID = 0; }
    30.         }
    31.        
    32.         switch (Event.current.GetTypeForControl(controlID)) {
    33.             case EventType.mouseDown:
    34.                 if (targetID == controlID)  GUIUtility.hotControl = controlID;
    35.  
    36.                 break;
    37.             case EventType.mouseUp:
    38.  
    39.                 if (GUIUtility.hotControl == controlID) GUIUtility.hotControl = 0;
    40.                 return (targetID == controlID);
    41.                 break;
    42.         }
    43.  
    44.         return false;
    45.     }
    46. }
    47.  
     
  14. CrackTop

    CrackTop

    Joined:
    Aug 5, 2014
    Posts:
    1
    Nice Useful Stuff , but i still do have some issue....... if a create a button and another inside it. the isMouseOver boolean returns true for both the buttons.....how do i stop it...and how do i chnage the texture if isMouseOver is true and vice versa.....thanks in advance
     
  15. philb1010

    philb1010

    Joined:
    Nov 12, 2013
    Posts:
    1
    I know this is an old thread, but in case it helps anyone... The overlap issue was only a problem when using my custom-made pop-up menus, i.e., they're the only buttons that might overlap other buttons, so my solution was to use a custom Button method, shown here, that checked whether the popup menu was open, and if so, just draw the button using its style instead of actually calling GUI.Button.

    I use this custom method for buttons everywhere except in my menu drawing routines, which make the regular GUI.Button calls because they should never draw the "fake" buttons. (Note: Menu.IsAnyOpen is a static method on my Menu class, not shown, that indicates whether a popup menu is open.)

    Code (CSharp):
    1.         /// <summary>
    2.         /// Draws a button, or just the appearance of a button if a menu is open.
    3.         /// </summary>
    4.         /// <param name="bounds">bounds</param>
    5.         /// <param name="content">content</param>
    6.         /// <param name="style">style</param>
    7.         /// <returns>whether the user clicked the button</returns>
    8.         public static bool Button(Rect bounds, GUIContent content, GUIStyle style = null)
    9.         {
    10.             if (style == null) style = Skin.button;
    11.             if (Menu.IsAnyOpen)
    12.             {
    13.                 if (Event.current.type == EventType.repaint) style.Draw(bounds, content, false, false, false, false);
    14.                 return false;
    15.             }
    16.             else return GUI.Button(bounds, content, style);
    17.         }
    18.  
     
    Last edited: Oct 21, 2014
  16. Zogg

    Zogg

    Joined:
    Mar 28, 2009
    Posts:
    157
    Great, thanks! Works like a charm!

    (I renamed the class "GUIButton" into "CitizensForABetterHandlingOfOverlappingButtons" though. Sounds better, methinks.)
     
  17. bakno

    bakno

    Joined:
    Mar 18, 2007
    Posts:
    561
    Thank you for sharing this solution. Here, it works when using the GoodButton on both buttons.

    Now I am having the same overlapping problem when having a button over a SelectionGrid. Even when using the GoodButton.

    Is there a way to have the same solution but for SelectionGrid? A replacement GoodSelectionGrid, please.

    Thank you
     
  18. tcz8

    tcz8

    Joined:
    Aug 20, 2015
    Posts:
    212