Skip to content

Commit

Permalink
Reusable function for raising events (MonoGame#5713)
Browse files Browse the repository at this point in the history
* Reusable function for raising events

Before raising an event, you must first check the handler to see if it is null in order to check that an object is subscribed to handle the event.  This is best done if the event handler is stored in a local variable just in case the last subscriber unsubscribes in a different thread between the time the event handler is checked and the handler is actually invoked.  This reusable function makes it easy to raise events by storing a local copy of the handler and checking for null before invoking it.  Many classes already had a helper function for this.  This change refactors those into a single function.

There are some instances where more complex logic was being done after the event handler was checked.  In those scenarios, the code was mostly left alone except for the introduction of a local handler variable.

* Safely raising events through common function

- Updated events being raised throughout the framework with the new EventHelpers.Raise function
- Updated the name of EventHelper to be plural in case more helper methods are needed in the future.
  • Loading branch information
gsfreema authored and tomspilman committed May 28, 2017
1 parent d9603eb commit aa98ee6
Show file tree
Hide file tree
Showing 34 changed files with 180 additions and 266 deletions.
3 changes: 3 additions & 0 deletions Build/Projects/2MGFX.definition
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@
</Compile>
<Compile Include="..\..\MonoGame.Framework\ContainmentType.cs">
<Link>MonoGame.Framework\ContainmentType.cs</Link>
</Compile>
<Compile Include="..\..\MonoGame.Framework\EventHelpers.cs">
<Link>MonoGame.Framework\EventHelpers.cs</Link>
</Compile>
<Compile Include="..\..\MonoGame.Framework\Graphics\ColorWriteChannels.cs">
<Link>MonoGame.Framework\ColorWriteChannels.cs</Link>
Expand Down
1 change: 1 addition & 0 deletions Build/Projects/MonoGame.Framework.definition
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@
<Compile Include="CurveTangent.cs" />
<Compile Include="DisplayOrientation.cs" />
<Compile Include="DrawableGameComponent.cs" />
<Compile Include="EventHelpers.cs" />
<Compile Include="FrameworkDispatcher.cs" />
<Compile Include="FrameworkResources.cs" />
<Compile Include="GameComponentCollection.cs" />
Expand Down
6 changes: 2 additions & 4 deletions MonoGame.Framework/Android/AndroidGameActivity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ public override void OnConfigurationChanged (Android.Content.Res.Configuration n
protected override void OnPause()
{
base.OnPause();
if (Paused != null)
Paused(this, EventArgs.Empty);
EventHelpers.Raise(this, Paused, EventArgs.Empty);

if (_orientationListener.CanDetectOrientation())
_orientationListener.Disable();
Expand All @@ -68,8 +67,7 @@ protected override void OnPause()
protected override void OnResume()
{
base.OnResume();
if (Resumed != null)
Resumed(this, EventArgs.Empty);
EventHelpers.Raise(this, Resumed, EventArgs.Empty);

if (Game != null)
{
Expand Down
18 changes: 5 additions & 13 deletions MonoGame.Framework/Android/GamerServices/SignedInGamer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -306,22 +306,14 @@ public GamerPrivileges Privileges


protected virtual void OnSignedIn(SignedInEventArgs e)
{
if (SignedIn != null)
{
// Invokes the delegates.
SignedIn(this, e);
}
{
EventHelpers.Raise(this, SignedIn, e);
}

protected virtual void OnSignedOut(SignedOutEventArgs e)
{
if (SignedOut != null)
{
// Invokes the delegates.
SignedOut(this, e);
}
}
{
EventHelpers.Raise(this, SignedOut, e);
}


#region Events
Expand Down
6 changes: 4 additions & 2 deletions MonoGame.Framework/Audio/DynamicSoundEffectInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,14 @@ internal void UpdateQueue()
PlatformUpdateQueue();

// Raise the event
if (BufferNeeded != null)
var bufferNeededHandler = BufferNeeded;

if (bufferNeededHandler != null)
{
var eventCount = (_buffersNeeded < 3) ? _buffersNeeded : 3;
for (var i = 0; i < eventCount; i++)
{
BufferNeeded(this, EventArgs.Empty);
bufferNeededHandler(this, EventArgs.Empty);
}
}

Expand Down
4 changes: 2 additions & 2 deletions MonoGame.Framework/Audio/Xact/AudioEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ private void Dispose(bool disposing)
// TODO: Should we be forcing any active
// audio cues to stop here?

if (disposing && Disposing != null)
Disposing(this, EventArgs.Empty);
if (disposing)
EventHelpers.Raise(this, Disposing, EventArgs.Empty);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions MonoGame.Framework/Audio/Xact/Cue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,7 @@ private void Dispose(bool disposing)
{
IsCreated = false;
IsPrepared = false;

if (Disposing != null)
Disposing(this, EventArgs.Empty);
EventHelpers.Raise(this, Disposing, EventArgs.Empty);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions MonoGame.Framework/Audio/Xact/SoundBank.cs
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,7 @@ private void Dispose(bool disposing)
if (disposing)
{
IsInUse = false;

if (Disposing != null)
Disposing(this, EventArgs.Empty);
EventHelpers.Raise(this, Disposing, EventArgs.Empty);
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions MonoGame.Framework/Audio/Xact/WaveBank.cs
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,7 @@ private void Dispose(bool disposing)

IsPrepared = false;
IsInUse = false;

if (Disposing != null)
Disposing(this, EventArgs.Empty);
EventHelpers.Raise(this, Disposing, EventArgs.Empty);
}
}
}
Expand Down
14 changes: 3 additions & 11 deletions MonoGame.Framework/DesktopGL/GamerServices/SignedInGamer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -292,20 +292,12 @@ public GamerPrivileges Privileges

protected virtual void OnSignedIn(SignedInEventArgs e)
{
if (SignedIn != null)
{
// Invokes the delegates.
SignedIn(this, e);
}
}
EventHelpers.Raise(this, SignedIn, e);
}

protected virtual void OnSignedOut(SignedOutEventArgs e)
{
if (SignedOut != null)
{
// Invokes the delegates.
SignedOut(this, e);
}
EventHelpers.Raise(this, SignedOut, e);
}


Expand Down
16 changes: 9 additions & 7 deletions MonoGame.Framework/DrawableGameComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ public int DrawOrder
if (_drawOrder != value)
{
_drawOrder = value;
if (DrawOrderChanged != null)
DrawOrderChanged(this, null);
OnDrawOrderChanged(this, null);
OnDrawOrderChanged(this, EventArgs.Empty);
}
}
}
Expand All @@ -40,8 +38,6 @@ public bool Visible
if (_visible != value)
{
_visible = value;
if (VisibleChanged != null)
VisibleChanged(this, EventArgs.Empty);
OnVisibleChanged(this, EventArgs.Empty);
}
}
Expand Down Expand Up @@ -70,8 +66,14 @@ protected virtual void UnloadContent () { }

public virtual void Draw(GameTime gameTime) { }

protected virtual void OnVisibleChanged(object sender, EventArgs args) { }
protected virtual void OnVisibleChanged(object sender, EventArgs args)
{
EventHelpers.Raise(this, VisibleChanged, args);
}

protected virtual void OnDrawOrderChanged(object sender, EventArgs args) { }
protected virtual void OnDrawOrderChanged(object sender, EventArgs args)
{
EventHelpers.Raise(this, DrawOrderChanged, args);
}
}
}
45 changes: 45 additions & 0 deletions MonoGame.Framework/EventHelpers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// MonoGame - Copyright (C) The MonoGame Team
// This file is subject to the terms and conditions defined in
// file 'LICENSE.txt', which is part of this source code package.

using System;

namespace Microsoft.Xna.Framework
{
/// <summary>
/// Provides helper methods to make it easier
/// to safely raise events.
/// </summary>
internal static class EventHelpers
{
/// <summary>
/// Safely raises an event by storing a copy of the event's delegate
/// in the <paramref name="handler"/> parameter and checking it for
/// null before invoking it.
/// </summary>
/// <typeparam name="TEventArgs"></typeparam>
/// <param name="sender">The object raising the event.</param>
/// <param name="handler"><see cref="EventHandler{TEventArgs}"/> to be invoked</param>
/// <param name="e">The <typeparamref name="TEventArgs"/> passed to <see cref="EventHandler{TEventArgs}"/></param>
internal static void Raise<TEventArgs>(object sender, EventHandler<TEventArgs> handler, TEventArgs e)
where TEventArgs : EventArgs
{
if (handler != null)
handler(sender, e);
}

/// <summary>
/// Safely raises an event by storing a copy of the event's delegate
/// in the <paramref name="handler"/> parameter and checking it for
/// null before invoking it.
/// </summary>
/// <param name="sender">The object raising the event.</param>
/// <param name="handler"><see cref="EventHandler"/> to be invoked</param>
/// <param name="e">The <see cref="EventArgs"/> passed to <see cref="EventHandler"/></param>
internal static void Raise(object sender, EventHandler handler, EventArgs e)
{
if (handler != null)
handler(sender, e);
}
}
}
21 changes: 7 additions & 14 deletions MonoGame.Framework/Game.cs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
Raise(Disposed, EventArgs.Empty);
EventHelpers.Raise(this, Disposed, EventArgs.Empty);
}

protected virtual void Dispose(bool disposing)
Expand Down Expand Up @@ -570,19 +570,19 @@ protected virtual void Update(GameTime gameTime)

protected virtual void OnExiting(object sender, EventArgs args)
{
Raise(Exiting, args);
EventHelpers.Raise(this, Exiting, args);
}

protected virtual void OnActivated (object sender, EventArgs args)
{
AssertNotDisposed();
Raise(Activated, args);
AssertNotDisposed();
EventHelpers.Raise(this, Activated, args);
}

protected virtual void OnDeactivated (object sender, EventArgs args)
{
AssertNotDisposed();
Raise(Deactivated, args);
AssertNotDisposed();
EventHelpers.Raise(this, Deactivated, args);
}

#endregion Protected Methods
Expand Down Expand Up @@ -618,7 +618,7 @@ private void Platform_AsyncRunLoopEnded(object sender, EventArgs e)
private void Platform_ApplicationViewChanged(object sender, ViewStateChangedEventArgs e)
{
AssertNotDisposed();
Raise(ApplicationViewChanged, e);
EventHelpers.Raise(this, ApplicationViewChanged, e);
}
#endif

Expand Down Expand Up @@ -760,13 +760,6 @@ private void DecategorizeComponent(IGameComponent component)
_drawables.Remove((IDrawable)component);
}

private void Raise<TEventArgs>(EventHandler<TEventArgs> handler, TEventArgs e)
where TEventArgs : EventArgs
{
if (handler != null)
handler(this, e);
}

/// <summary>
/// The SortingFilteringCollection class provides efficient, reusable
/// sorting and filtering based on a configurable sort comparer, filter
Expand Down
18 changes: 10 additions & 8 deletions MonoGame.Framework/GameComponent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ public bool Enabled
if (_enabled != value)
{
_enabled = value;
if (this.EnabledChanged != null)
this.EnabledChanged(this, EventArgs.Empty);
OnEnabledChanged(this, null);
OnEnabledChanged(this, EventArgs.Empty);
}
}
}
Expand All @@ -36,9 +34,7 @@ public int UpdateOrder
if (_updateOrder != value)
{
_updateOrder = value;
if (this.UpdateOrderChanged != null)
this.UpdateOrderChanged(this, EventArgs.Empty);
OnUpdateOrderChanged(this, null);
OnUpdateOrderChanged(this, EventArgs.Empty);
}
}
}
Expand All @@ -60,9 +56,15 @@ public virtual void Initialize() { }

public virtual void Update(GameTime gameTime) { }

protected virtual void OnUpdateOrderChanged(object sender, EventArgs args) { }
protected virtual void OnUpdateOrderChanged(object sender, EventArgs args)
{
EventHelpers.Raise(this, UpdateOrderChanged, args);
}

protected virtual void OnEnabledChanged(object sender, EventArgs args) { }
protected virtual void OnEnabledChanged(object sender, EventArgs args)
{
EventHelpers.Raise(this, EnabledChanged, args);
}

/// <summary>
/// Shuts down the component.
Expand Down
10 changes: 2 additions & 8 deletions MonoGame.Framework/GameComponentCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,12 @@ protected override void InsertItem(int index, IGameComponent item)

private void OnComponentAdded(GameComponentCollectionEventArgs eventArgs)
{
if (this.ComponentAdded != null)
{
this.ComponentAdded(this, eventArgs);
}
EventHelpers.Raise(this, ComponentAdded, eventArgs);
}

private void OnComponentRemoved(GameComponentCollectionEventArgs eventArgs)
{
if (this.ComponentRemoved != null)
{
this.ComponentRemoved(this, eventArgs);
}
EventHelpers.Raise(this, ComponentRemoved, eventArgs);
}

protected override void RemoveItem(int index)
Expand Down
2 changes: 1 addition & 1 deletion MonoGame.Framework/GamePlatform.Desktop.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public ApplicationViewState ViewState
if (_viewState == value)
return;

Raise(ViewStateChanged, new ViewStateChangedEventArgs(value));
EventHelpers.Raise(this, ViewStateChanged, new ViewStateChangedEventArgs(value));

_viewState = value;
}
Expand Down
11 changes: 2 additions & 9 deletions MonoGame.Framework/GamePlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal set
if (_isActive != value)
{
_isActive = value;
Raise(_isActive ? Activated : Deactivated, EventArgs.Empty);
EventHelpers.Raise(this, _isActive ? Activated : Deactivated, EventArgs.Empty);
}
}
}
Expand Down Expand Up @@ -107,21 +107,14 @@ protected set
public event EventHandler<EventArgs> Activated;
public event EventHandler<EventArgs> Deactivated;

private void Raise<TEventArgs>(EventHandler<TEventArgs> handler, TEventArgs e)
where TEventArgs : EventArgs
{
if (handler != null)
handler(this, e);
}

/// <summary>
/// Raises the AsyncRunLoopEnded event. This method must be called by
/// derived classes when the asynchronous run loop they start has
/// stopped running.
/// </summary>
protected void RaiseAsyncRunLoopEnded()
{
Raise(AsyncRunLoopEnded, EventArgs.Empty);
EventHelpers.Raise(this, AsyncRunLoopEnded, EventArgs.Empty);
}

#endregion Events
Expand Down
Loading

0 comments on commit aa98ee6

Please sign in to comment.