RFC: Panel Events

[garrynewman]

So I'm looking at the panel event system we have. Right now we're doing things like this.

public void OnEvent( string eventName )
{
	if ( eventName == "onclick" )
	{
		Toggle();
	}

	base.OnEvent( eventName );
}

This is obviously a bit dumb. So what do you favour here?

Method A:

[PanelEvent( "onclick" )] // event name is dictated by the attribute
public void OnClickEvent()
{
	Toggle();
}

Method B:

public void OnClickEvent() // Don't use an attribute, event Methods have Event on the end
{
	Toggle();
}

Method C:

[PanelEvent] // Hybrid, names are optional in the attribute, if unset they're derived from the method name
public void OnClickEvent() 
{
	Toggle();
}

[notcat]

Method B looks the nicest and most concise, imo.

After careful rethinking, I think A is nicer for things like user created events, and makes more sense overall.

[garrynewman]

For me, the benefit of having the [PanelEvent] attribute is that in this example you could just add the attribute to the toggle method, skipping the need for a special event method completely.

[garrynewman]

I guess we could have it all here. Could create an event if it has Event on the end of the method name OR if it has a PanelEvent attribute

[Alf21]

I like method A as it matches the Event system etc. as well :) Anyways, C would help making code looking more uniformly.

[ZehMatt]

I would actually favor method A, the reason being I can document with the function name what the click event is about.

[PanelEvent( "onclick" )]
public void PlayerReadyClicked()
{
}

C is also a nice option since we can have both.

[Srlion]

Method C would fit perfectly, having the name option is good for everyone. (I just don't like typing the event name twice, one for the attribute and one for the method)

[Ceveos]

Method A I think is the most explicit and easily understood; that gets my vote

[garrynewman]

Alright here's what I decided.. for built in stuff, we'll do the event system as normal, "onclick" etc, but also have built in virtual methods.

This means you'll still be able to bind to other controls like

Error.Button.AddEvent( "onclick", () => { Error.Toggle(); OnFilter(); } );

but also means you don't have panels using the event system on themselves if they don't need to.

For custom binding events you'll have to explicitly define them with the attribute. This will stop you accidentally binding stuff because you added Event at the end of a method name.

[yoranmandema]

it would be nice to have the Button property on the event be some enum