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

Prevent dependency pollution up the scene graph #922

Merged
merged 2 commits into from
Jul 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions osu.Framework/Allocation/DependencyContainer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@ namespace osu.Framework.Allocation
/// <summary>
/// Hierarchically caches dependencies and can inject those automatically into types registered for dependency injection.
/// </summary>
public class DependencyContainer
public class DependencyContainer : IReadOnlyDependencyContainer
{
private delegate object ObjectActivator(DependencyContainer dc, object instance);

private readonly ConcurrentDictionary<Type, ObjectActivator> activators = new ConcurrentDictionary<Type, ObjectActivator>();
private readonly ConcurrentDictionary<Type, object> cache = new ConcurrentDictionary<Type, object>();
private readonly HashSet<Type> cacheable = new HashSet<Type>();

private readonly DependencyContainer parentContainer;
private readonly IReadOnlyDependencyContainer parentContainer;

/// <summary>
/// Create a new DependencyContainer instance.
/// </summary>
/// <param name="parent">An optional parent container which we should use as a fallback for cache lookups.</param>
public DependencyContainer(DependencyContainer parent = null)
public DependencyContainer(IReadOnlyDependencyContainer parent = null)
{
parentContainer = parent;
Cache(this);
Expand Down Expand Up @@ -70,7 +70,7 @@ private void register(Type type, bool lazy)
var parameters = initializer.GetParameters().Select(p => p.ParameterType)
.Select(t => new Func<object>(() =>
{
var val = get(t);
var val = Get(t);
if (val == null && !permitNull)
{
throw new InvalidOperationException(
Expand Down Expand Up @@ -132,13 +132,20 @@ public T Cache<T>(T instance = null, bool overwrite = false, bool lazy = false)
return instance;
}

private object get(Type type)
/// <summary>
/// Retrieves a cached dependency of <paramref name="type"/> if it exists. If not, then the parent
/// <see cref="IReadOnlyDependencyContainer"/> is recursively queried. If no parent contains
/// <paramref name="type"/>, then null is returned.
/// </summary>
/// <param name="type">The dependency type to query for.</param>
/// <returns>The requested dependency, or null if not found.</returns>
public object Get(Type type)
{
object ret;
if (cache.TryGetValue(type, out ret))
return ret;

return parentContainer?.get(type);
return parentContainer?.Get(type);

//we don't ever want to instantiate for now, as this breaks expectations when using permitNull.
//need to revisit this when/if it is required.
Expand All @@ -155,20 +162,20 @@ private object get(Type type)
/// </summary>
public T Get<T>(bool autoRegister = true, bool lazy = false) where T : class
{
T instance = (T)get(typeof(T));
T instance = (T)Get(typeof(T));
if (autoRegister && instance == null)
{
Register<T>(lazy);
instance = (T)get(typeof(T));
instance = (T)Get(typeof(T));
}
return instance;
}

/// <summary>
/// Injects dependencies for the given instance.
/// Injects dependencies into the given instance.
/// </summary>
/// <typeparam name="T">The type of the instance to inject dependencies for.</typeparam>
/// <param name="instance">The instance to inject dependencies for.</param>
/// <typeparam name="T">The type of the instance to inject dependencies into.</typeparam>
/// <param name="instance">The instance to inject dependencies into.</param>
/// <param name="autoRegister">True if the instance should be automatically registered as injectable if it isn't already.</param>
/// <param name="lazy">True if the dependencies should be initialized lazily.</param>
public void Inject<T>(T instance, bool autoRegister = true, bool lazy = false) where T : class
Expand Down
30 changes: 30 additions & 0 deletions osu.Framework/Allocation/IReadOnlyDependencyContainer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) 2007-2017 ppy Pty Ltd <contact@ppy.sh>.
// Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu-framework/master/LICENCE

using System;

namespace osu.Framework.Allocation
{
/// <summary>
/// Read-only interface into a dependency container capable of injective and retrieving dependencies based
/// on types.
/// </summary>
public interface IReadOnlyDependencyContainer
{
/// <summary>
/// Retrieves a cached dependency of <paramref name="type"/> if it exists and null otherwise.
/// </summary>
/// <param name="type">The dependency type to query for.</param>
/// <returns>The requested dependency, or null if not found.</returns>
object Get(Type type);

/// <summary>
/// Injects dependencies into the given instance.
/// </summary>
/// <typeparam name="T">The type of the instance to inject dependencies into.</typeparam>
/// <param name="instance">The instance to inject dependencies into.</param>
/// <param name="autoRegister">True if the instance should be automatically registered as injectable if it isn't already.</param>
/// <param name="lazy">True if the dependencies should be initialized lazily.</param>
void Inject<T>(T instance, bool autoRegister = true, bool lazy = false) where T : class;
}
}
13 changes: 9 additions & 4 deletions osu.Framework/Game.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ public virtual void SetHost(GameHost host)
host.Deactivated += () => IsActive = false;
}

private DependencyContainer dependencies;

protected override IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) =>
dependencies = new DependencyContainer(base.CreateLocalDependencies(parent));

[BackgroundDependencyLoader]
private void load(FrameworkConfigManager config)
{
Expand All @@ -104,7 +109,7 @@ private void load(FrameworkConfigManager config)

Textures = new TextureStore(new RawTextureLoaderStore(new NamespacedResourceStore<byte[]>(Resources, @"Textures")));
Textures.AddStore(new RawTextureLoaderStore(new OnlineStore()));
Dependencies.Cache(Textures);
dependencies.Cache(Textures);

var tracks = new ResourceStore<byte[]>(Resources);
tracks.AddStore(new NamespacedResourceStore<byte[]>(Resources, @"Tracks"));
Expand All @@ -114,7 +119,7 @@ private void load(FrameworkConfigManager config)
samples.AddStore(new NamespacedResourceStore<byte[]>(Resources, @"Samples"));
samples.AddStore(new OnlineStore());

Audio = Dependencies.Cache(new AudioManager(
Audio = dependencies.Cache(new AudioManager(
tracks,
samples)
{
Expand All @@ -130,13 +135,13 @@ private void load(FrameworkConfigManager config)
config.BindWith(FrameworkSetting.VolumeMusic, Audio.VolumeTrack);

Shaders = new ShaderManager(new NamespacedResourceStore<byte[]>(Resources, @"Shaders"));
Dependencies.Cache(Shaders);
dependencies.Cache(Shaders);

Fonts = new FontStore(new GlyphStore(Resources, @"Fonts/OpenSans"))
{
ScaleAdjust = 100
};
Dependencies.Cache(Fonts);
dependencies.Cache(Fonts);
}

protected override void LoadComplete()
Expand Down
10 changes: 5 additions & 5 deletions osu.Framework/Graphics/Drawable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -148,22 +148,22 @@ internal Task LoadAsync(Game game, Drawable target, Action onLoaded = null)
/// Create a local dependency container which will be used by ourselves and all our nested children.
/// If not overridden, the load-time parent's dependency tree will be used.
/// </summary>
/// <param name="parent">The parent <see cref="DependencyContainer"/> which should be passed through if we want fallback lookups to work.</param>
/// <param name="parent">The parent <see cref="IReadOnlyDependencyContainer"/> which should be passed through if we want fallback lookups to work.</param>
/// <returns>A new dependency container to be stored for this Drawable.</returns>
protected virtual DependencyContainer CreateLocalDependencies(DependencyContainer parent) => parent;
protected virtual IReadOnlyDependencyContainer CreateLocalDependencies(IReadOnlyDependencyContainer parent) => parent;

/// <summary>
/// Contains all dependencies that can be injected into this Drawable using <see cref="BackgroundDependencyLoader"/>.
/// Add or override dependencies by calling <see cref="DependencyContainer.Cache{T}(T, bool, bool)"/>.
/// </summary>
protected DependencyContainer Dependencies { get; private set; }
protected IReadOnlyDependencyContainer Dependencies { get; private set; }

/// <summary>
/// Loads this drawable, including the gathering of dependencies and initialisation of required resources.
/// </summary>
/// <param name="clock">The clock we should use by default.</param>
/// <param name="dependencies">The dependency tree we will inherit by default. May be extended via <see cref="CreateLocalDependencies(DependencyContainer)"/></param>
internal void Load(IFrameBasedClock clock, DependencyContainer dependencies)
/// <param name="dependencies">The dependency tree we will inherit by default. May be extended via <see cref="CreateLocalDependencies(IReadOnlyDependencyContainer)"/></param>
internal void Load(IFrameBasedClock clock, IReadOnlyDependencyContainer dependencies)
{
// Blocks when loading from another thread already.
lock (loadLock)
Expand Down
1 change: 1 addition & 0 deletions osu.Framework/osu.Framework.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
</Reference>
</ItemGroup>
<ItemGroup>
<Compile Include="Allocation\IReadOnlyDependencyContainer.cs" />
<Compile Include="Audio\AudioComponent.cs" />
<Compile Include="Audio\IBassAudio.cs" />
<Compile Include="Audio\IHasPitchAdjust.cs" />
Expand Down