Merge branch 'master' into better-mp-songselect-deletion-handling

osu.Game.Rulesets.Osu/Objects/Drawables/Connections/ConnectionRenderer.cs

@@ -10,7 +10,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
     /// <summary>
     /// Connects hit objects visually, for example with follow points.
     /// </summary>
-    public abstract class ConnectionRenderer<T> : Container
+    public abstract class ConnectionRenderer<T> : LifetimeManagementContainer
         where T : HitObject
     {
         /// <summary>

osu.Game.Rulesets.Osu/Objects/Drawables/Connections/FollowPointRenderer.cs

@@ -56,7 +56,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
 
         private void update()
         {
-            Clear();
+            ClearInternal();
 
             if (hitObjects == null)
                 return;
@@ -86,7 +86,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Connections
 
                         FollowPoint fp;
 
-                        Add(fp = new FollowPoint
+                        AddInternal(fp = new FollowPoint
                         {
                             Position = pointStartPosition,
                             Rotation = rotation,

osu.Game.Rulesets.Osu/Objects/Drawables/DrawableHitCircle.cs

@@ -139,6 +139,7 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables
 
             ApproachCircle.FadeIn(Math.Min(HitObject.TimeFadeIn * 2, HitObject.TimePreempt));
             ApproachCircle.ScaleTo(1.1f, HitObject.TimePreempt);
+            ApproachCircle.Expire(true);
         }
 
         protected override void UpdateCurrentState(ArmedState state)

osu.Game.Rulesets.Osu/Objects/Drawables/Pieces/ApproachCircle.cs

@@ -12,6 +12,8 @@ namespace osu.Game.Rulesets.Osu.Objects.Drawables.Pieces
 {
     public class ApproachCircle : Container
     {
+        public override bool RemoveWhenNotAlive => false;
+
         public ApproachCircle()
         {
             Anchor = Anchor.Centre;

osu.Game.Rulesets.Osu/UI/OsuPlayfield.cs

@@ -16,7 +16,7 @@ namespace osu.Game.Rulesets.Osu.UI
 {
     public class OsuPlayfield : Playfield
     {
-        private readonly Container approachCircles;
+        private readonly ApproachCircleProxyContainer approachCircles;
         private readonly JudgementContainer<DrawableOsuJudgement> judgementLayer;
         private readonly ConnectionRenderer<OsuHitObject> connectionLayer;
 
@@ -45,7 +45,7 @@ namespace osu.Game.Rulesets.Osu.UI
                         Depth = 1,
                     },
                     HitObjectContainer,
-                    approachCircles = new Container
+                    approachCircles = new ApproachCircleProxyContainer
                     {
                         RelativeSizeAxes = Axes.Both,
                         Depth = -1,
@@ -60,11 +60,25 @@ namespace osu.Game.Rulesets.Osu.UI
 
             var c = h as IDrawableHitObjectWithProxiedApproach;
             if (c != null)
-                approachCircles.Add(c.ProxiedLayer.CreateProxy());
+            {
+                var original = c.ProxiedLayer;
+
+                // Hitobjects only have lifetimes set on LoadComplete. For nested hitobjects (e.g. SliderHeads), this only happens when the parenting slider becomes visible.
+                // This delegation is required to make sure that the approach circles for those not-yet-loaded objects aren't added prematurely.
+                original.OnLoadComplete += addApproachCircleProxy;
+            }
 
             base.Add(h);
         }
 
+        private void addApproachCircleProxy(Drawable d)
+        {
+            var proxy = d.CreateProxy();
+            proxy.LifetimeStart = d.LifetimeStart;
+            proxy.LifetimeEnd = d.LifetimeEnd;
+            approachCircles.Add(proxy);
+        }
+
         public override void PostProcess()
         {
             connectionLayer.HitObjects = HitObjectContainer.Objects.Select(d => d.HitObject).OfType<OsuHitObject>();
@@ -86,5 +100,10 @@ namespace osu.Game.Rulesets.Osu.UI
         }
 
         public override bool ReceivePositionalInputAt(Vector2 screenSpacePos) => HitObjectContainer.ReceivePositionalInputAt(screenSpacePos);
+
+        private class ApproachCircleProxyContainer : LifetimeManagementContainer
+        {
+            public void Add(Drawable approachCircleProxy) => AddInternal(approachCircleProxy);
+        }
     }
 }

osu.Game.Tests/Visual/TestCaseSkinReloadable.cs

@@ -0,0 +1,153 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using NUnit.Framework;
+using osu.Framework.Audio.Sample;
+using osu.Framework.Graphics;
+using osu.Framework.Graphics.Containers;
+using osu.Framework.Graphics.Shapes;
+using osu.Framework.Graphics.Sprites;
+using osu.Framework.Graphics.Textures;
+using osu.Game.Graphics;
+using osu.Game.Skinning;
+using osuTK.Graphics;
+
+namespace osu.Game.Tests.Visual
+{
+    public class TestCaseSkinReloadable : OsuTestCase
+    {
+        [Test]
+        public void TestInitialLoad()
+        {
+            var secondarySource = new SecondarySource();
+            SkinConsumer consumer = null;
+
+            AddStep("setup layout", () =>
+            {
+                Child = new SkinSourceContainer
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Child = new LocalSkinOverrideContainer(secondarySource)
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                        Child = consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true)
+                    }
+                };
+            });
+
+            AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox);
+            AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1);
+        }
+
+        [Test]
+        public void TestOverride()
+        {
+            var secondarySource = new SecondarySource();
+
+            SkinConsumer consumer = null;
+            Container target = null;
+
+            AddStep("setup layout", () =>
+            {
+                Child = new SkinSourceContainer
+                {
+                    RelativeSizeAxes = Axes.Both,
+                    Child = target = new LocalSkinOverrideContainer(secondarySource)
+                    {
+                        RelativeSizeAxes = Axes.Both,
+                    }
+                };
+            });
+
+            AddStep("add permissive", () => target.Add(consumer = new SkinConsumer("test", name => new NamedBox("Default Implementation"), source => true)));
+            AddAssert("consumer using override source", () => consumer.Drawable is SecondarySourceBox);
+            AddAssert("skinchanged only called once", () => consumer.SkinChangedCount == 1);
+        }
+
+        private class NamedBox : Container
+        {
+            public NamedBox(string name)
+            {
+                Children = new Drawable[]
+                {
+                    new Box
+                    {
+                        Colour = Color4.Black,
+                        RelativeSizeAxes = Axes.Both,
+                    },
+                    new SpriteText
+                    {
+                        Font = OsuFont.Default.With(size: 40),
+                        Anchor = Anchor.Centre,
+                        Origin = Anchor.Centre,
+                        Text = name
+                    }
+                };
+            }
+        }
+
+        private class SkinConsumer : SkinnableDrawable
+        {
+            public new Drawable Drawable => base.Drawable;
+            public int SkinChangedCount { get; private set; }
+
+            public SkinConsumer(string name, Func<string, Drawable> defaultImplementation, Func<ISkinSource, bool> allowFallback = null, bool restrictSize = true)
+                : base(name, defaultImplementation, allowFallback, restrictSize)
+            {
+            }
+
+            protected override void SkinChanged(ISkinSource skin, bool allowFallback)
+            {
+                base.SkinChanged(skin, allowFallback);
+                SkinChangedCount++;
+            }
+        }
+
+        private class BaseSourceBox : NamedBox
+        {
+            public BaseSourceBox()
+                : base("Base Source")
+            {
+            }
+        }
+
+        private class SecondarySourceBox : NamedBox
+        {
+            public SecondarySourceBox()
+                : base("Secondary Source")
+            {
+            }
+        }
+
+        private class SecondarySource : ISkinSource
+        {
+            public event Action SourceChanged;
+
+            public void TriggerSourceChanged() => SourceChanged?.Invoke();
+
+            public Drawable GetDrawableComponent(string componentName) => new SecondarySourceBox();
+
+            public Texture GetTexture(string componentName) => throw new NotImplementedException();
+
+            public SampleChannel GetSample(string sampleName) => throw new NotImplementedException();
+
+            public TValue GetValue<TConfiguration, TValue>(Func<TConfiguration, TValue> query) where TConfiguration : SkinConfiguration => throw new NotImplementedException();
+        }
+
+        private class SkinSourceContainer : Container, ISkinSource
+        {
+            public event Action SourceChanged;
+
+            public void TriggerSourceChanged() => SourceChanged?.Invoke();
+
+            public Drawable GetDrawableComponent(string componentName) => new BaseSourceBox();
+
+            public Texture GetTexture(string componentName) => throw new NotImplementedException();
+
+            public SampleChannel GetSample(string sampleName) => throw new NotImplementedException();
+
+            public TValue GetValue<TConfiguration, TValue>(Func<TConfiguration, TValue> query) where TConfiguration : SkinConfiguration => throw new NotImplementedException();
+        }
+    }
+}

osu.Game/Audio/PreviewTrack.cs

@@ -28,7 +28,7 @@ namespace osu.Game.Audio
         private void load()
         {
             track = GetTrack();
-            track.Completed += Stop;
+            track.Completed += () => Schedule(Stop);
         }
 
         /// <summary>

osu.Game/Overlays/MusicController.cs

@@ -351,11 +351,11 @@ namespace osu.Game.Overlays
             queuedDirection = null;
         }
 
-        private void currentTrackCompleted()
+        private void currentTrackCompleted() => Schedule(() =>
         {
-            if (!beatmap.Disabled && beatmapSets.Any())
+            if (!current.Track.Looping && !beatmap.Disabled && beatmapSets.Any())
                 next();
-        }
+        });
 
         private ScheduledDelegate pendingBeatmapSwitch;
 

osu.Game/Rulesets/Objects/Drawables/DrawableHitObject.cs

@@ -220,6 +220,16 @@ namespace osu.Game.Rulesets.Objects.Drawables
             OnNewResult?.Invoke(this, Result);
         }
 
+        /// <summary>
+        /// Will called at least once after the <see cref="LifetimeEnd"/> of this <see cref="DrawableHitObject"/> has been passed.
+        /// </summary>
+        internal void OnLifetimeEnd()
+        {
+            foreach (var nested in NestedHitObjects)
+                nested.OnLifetimeEnd();
+            UpdateResult(false);
+        }
+
         /// <summary>
         /// Processes this <see cref="DrawableHitObject"/>, checking if a scoring result has occurred.
         /// </summary>

osu.Game/Rulesets/UI/HitObjectContainer.cs

@@ -9,7 +9,7 @@ using osu.Game.Rulesets.Objects.Drawables;
 
 namespace osu.Game.Rulesets.UI
 {
-    public class HitObjectContainer : CompositeDrawable
+    public class HitObjectContainer : LifetimeManagementContainer
     {
         public IEnumerable<DrawableHitObject> Objects => InternalChildren.Cast<DrawableHitObject>().OrderBy(h => h.HitObject.StartTime);
         public IEnumerable<DrawableHitObject> AliveObjects => AliveInternalChildren.Cast<DrawableHitObject>().OrderBy(h => h.HitObject.StartTime);
@@ -31,5 +31,11 @@ namespace osu.Game.Rulesets.UI
             int i = yObj.HitObject.StartTime.CompareTo(xObj.HitObject.StartTime);
             return i == 0 ? CompareReverseChildID(x, y) : i;
         }
+
+        protected override void OnChildLifetimeBoundaryCrossed(LifetimeBoundaryCrossedEvent e)
+        {
+            if (e.Kind == LifetimeBoundaryKind.End && e.Direction == LifetimeBoundaryCrossingDirection.Forward && e.Child is DrawableHitObject hitObject)
+                hitObject.OnLifetimeEnd();
+        }
     }
 }

osu.Game/Skinning/LocalSkinOverrideContainer.cs

@@ -71,31 +71,27 @@ namespace osu.Game.Skinning
             var dependencies = new DependencyContainer(base.CreateChildDependencies(parent));
 
             fallbackSource = dependencies.Get<ISkinSource>();
+            if (fallbackSource != null)
+                fallbackSource.SourceChanged += onSourceChanged;
+
             dependencies.CacheAs<ISkinSource>(this);
 
+            var config = dependencies.Get<OsuConfigManager>();
+
+            config.BindWith(OsuSetting.BeatmapSkins, beatmapSkins);
+            config.BindWith(OsuSetting.BeatmapHitsounds, beatmapHitsounds);
+
+            beatmapSkins.BindValueChanged(_ => onSourceChanged());
+            beatmapHitsounds.BindValueChanged(_ => onSourceChanged());
+
             return dependencies;
         }
 
-        [BackgroundDependencyLoader]
-        private void load(OsuConfigManager config)
-        {
-            config.BindWith(OsuSetting.BeatmapSkins, beatmapSkins);
-            config.BindWith(OsuSetting.BeatmapHitsounds, beatmapHitsounds);
-        }
-
-        protected override void LoadComplete()
-        {
-            base.LoadComplete();
-
-            if (fallbackSource != null)
-                fallbackSource.SourceChanged += onSourceChanged;
-
-            beatmapSkins.BindValueChanged(_ => onSourceChanged());
-            beatmapHitsounds.BindValueChanged(_ => onSourceChanged(), true);
-        }
-
         protected override void Dispose(bool isDisposing)
         {
+            // Must be done before base.Dispose()
+            SourceChanged = null;
+
             base.Dispose(isDisposing);
 
             if (fallbackSource != null)

osu.Game/Storyboards/Drawables/DrawableStoryboardLayer.cs

@@ -7,7 +7,7 @@ using osu.Framework.Graphics.Containers;
 
 namespace osu.Game.Storyboards.Drawables
 {
-    public class DrawableStoryboardLayer : Container
+    public class DrawableStoryboardLayer : LifetimeManagementContainer
     {
         public StoryboardLayer Layer { get; private set; }
         public bool Enabled;
@@ -29,7 +29,7 @@ namespace osu.Game.Storyboards.Drawables
             foreach (var element in Layer.Elements)
             {
                 if (element.IsDrawable)
-                    Add(element.CreateDrawable());
+                    AddInternal(element.CreateDrawable());
             }
         }
     }