OnClick and private methods

[JosephFerano]

I noticed that the reflection for generating methods in the OnClick handler for UIButton only has the flags BindingFlags.Instance | BindingFlags.Public  in them. I think I understand why this was done; in order to make things easier for the user. However, I usually find myself making my events private, to properly hide away such things from other classes. Sure, it's not a big deal, but I take my encapsulation in Unity very seriously, going as far as to using SerializeField all the time so I can hide away all those inspector fields from other classes.

So what I came up with was a simple empty attribute. For those who don't care, they can continue making them public. For those who care, they can use the NGUIEvent I created;

1. [System.AttributeUsageAttribute(System.AttributeTargets.Method)]
2. public class NGUIEvent : System.Attribute
3. {
4.         // Used to allow private methods in the Methods dropdown for OnClicks in the inspector
5. }
6.  

I added the BindingFlags.NonPublic flag, then I added this code on line 40 or something;

1. if (!mi.IsPublic) {
2.         System.Object[] nguiEventMethods = mi.GetCustomAttributes(typeof(NGUIEvent), false);
3.         bool hasAttribute = false;
4.         if (nguiEventMethods.Length > 0) {
5.                 foreach (var nguiEventMethod in nguiEventMethods) {
6.                         if (nguiEventMethod.GetType() == typeof(NGUIEvent)) {
7.                                 hasAttribute = true;
8.                         }
9.                 }
10.         }
11.         if (!hasAttribute) continue;
12. }
13.  

That would probably look prettier with some link which can be processed on the original mi collection. What do you guys think?

I especially like this because it makes the code even more legible, because now [NGUIEvent] will clearly tell you what a method is being used for, generally.

[JosephFerano]

I've been using it thus far and it seems to be working out pretty nicely. I'll probably use some linq to make it prettier though.

[ArenMook]

It's an interesting thought.

[ShinyMark]

I agree that in general Unity encourages making WAY too much information public in components. I also use the SerializedField attribute on all of my components when exposing data to the inspector so that the fields can be kept private if they are truly private data to the class.

I like this idea, @JosephFerano, but I wanted to mention another approach that you can follow to hide this information from the inspector. Simply, don't use the OnClick notification in UIButton and instead setup your event listeners programmatically using the UIEventListener class.

Example:

1. UIEventListener.Get(_targetButton.gameObject).onClick += gameObjectClicked =>
2. {
3.   // do something to respond to the event
4. };
5.  

On our team the guy who makes the UI prefabs doesn't write any code so he's not going to be setting up OnClick handlers in the inspector anyways. We find the programmatic approach works best for us. As another bonus for larger teams, it makes it easier for other programmers to see what is going on when looking at a code file they didn't write without having to instantiate a UI prefab and dig around in the inspector.

[JosephFerano]

I do like your approach ShinyMark. I actually think it makes more sense because your approach is more in line with the Observer pattern. There would still be inspector work to be done as the easiest way to find "_targetButton" would be to create an inspector field that you can drag and drop. The one pro I can see from my system is that anyone can find the button and look at its OnClick handlers in the inspector and instantly know what it's calling, whereas your method would require someone to find the class that receives the has the listener, which may not always be clear (it should be if you set up a consistent scene hierarchy). Your technique though has the advantage of not needing to modify NGUI, so it's not a pain in the butt to upgrade NGUI each time. I say it's a pain in the butt, but it's pretty easy with git, however, it would be nice if it were streamlined anyway.