Remove placement events, make everything pass top-down

osu.Game.Tests/Visual/TestCaseHitObjectComposer.cs

@@ -38,8 +38,7 @@ namespace osu.Game.Tests.Visual
             typeof(HitCirclePlacementMask),
         };
 
-        public event Action<HitObject> PlacementStarted;
+        private HitObjectComposer composer;
-        public event Action<HitObject> PlacementFinished;
 
         [BackgroundDependencyLoader]
         private void load()
@@ -70,11 +69,13 @@ namespace osu.Game.Tests.Visual
             Dependencies.CacheAs<IAdjustableClock>(clock);
             Dependencies.CacheAs<IFrameBasedClock>(clock);
 
-            Child = new OsuHitObjectComposer(new OsuRuleset());
+            Child = composer = new OsuHitObjectComposer(new OsuRuleset());
         }
 
-        public void BeginPlacement(HitObject hitObject) => PlacementStarted?.Invoke(hitObject);
+        public void BeginPlacement(HitObject hitObject)
+        {
+        }
 
-        public void EndPlacement(HitObject hitObject) => PlacementFinished?.Invoke(hitObject);
+        public void EndPlacement(HitObject hitObject) => composer.Add(hitObject);
     }
 }

osu.Game/Rulesets/Edit/EditRulesetContainer.cs

@@ -21,7 +21,12 @@ namespace osu.Game.Rulesets.Edit
             RelativeSizeAxes = Axes.Both;
         }
 
-        public abstract DrawableHitObject AddHitObject(HitObject hitObject);
+        /// <summary>
+        /// Adds a <see cref="HitObject"/> to the <see cref="Beatmap"/> and displays a visual representation of it.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="HitObject"/> to add.</param>
+        /// <returns>The visual representation of <paramref name="hitObject"/>.</returns>
+        internal abstract DrawableHitObject Add(HitObject hitObject);
     }
 
     public abstract class EditRulesetContainer<TObject> : EditRulesetContainer
@@ -41,20 +46,22 @@ namespace osu.Game.Rulesets.Edit
             InternalChild = rulesetContainer = CreateRulesetContainer(ruleset, workingBeatmap);
         }
 
-        public override DrawableHitObject AddHitObject(HitObject hitObject)
+        internal override DrawableHitObject Add(HitObject hitObject)
         {
             var tObject = (TObject)hitObject;
 
-            // Insert into beatmap while maintaining sorting order
+            // Add to beatmap, preserving sorting order
             var insertionIndex = beatmap.HitObjects.FindLastIndex(h => h.StartTime <= hitObject.StartTime);
             beatmap.HitObjects.Insert(insertionIndex + 1, tObject);
 
+            // Process object
             var processor = ruleset.CreateBeatmapProcessor(beatmap);
 
             processor.PreProcess();
             tObject.ApplyDefaults(beatmap.ControlPointInfo, beatmap.BeatmapInfo.BaseDifficulty);
             processor.PostProcess();
 
+            // Add visual representation
             var drawableObject = rulesetContainer.GetVisualRepresentation(tObject);
 
             rulesetContainer.Playfield.Add(drawableObject);

osu.Game/Rulesets/Edit/HitObjectComposer.cs

@@ -13,9 +13,9 @@ using osu.Framework.Timing;
 using osu.Game.Beatmaps;
 using osu.Game.Rulesets.Configuration;
 using osu.Game.Rulesets.Edit.Tools;
+using osu.Game.Rulesets.Objects;
 using osu.Game.Rulesets.Objects.Drawables;
 using osu.Game.Rulesets.UI;
-using osu.Game.Screens.Edit.Screens.Compose;
 using osu.Game.Screens.Edit.Screens.Compose.Layers;
 using osu.Game.Screens.Edit.Screens.Compose.RadioButtons;
 
@@ -32,13 +32,10 @@ namespace osu.Game.Rulesets.Edit
         private readonly List<Container> layerContainers = new List<Container>();
         private readonly IBindable<WorkingBeatmap> beatmap = new Bindable<WorkingBeatmap>();
 
-        [Resolved]
-        private IPlacementHandler placementHandler { get; set; }
-
-        private HitObjectMaskLayer maskLayer;
         private EditRulesetContainer rulesetContainer;
 
-        private readonly Bindable<HitObjectCompositionTool> compositionTool = new Bindable<HitObjectCompositionTool>();
+        private HitObjectMaskLayer maskLayer;
+        private PlacementContainer placementContainer;
 
         protected HitObjectComposer(Ruleset ruleset)
         {
@@ -73,7 +70,7 @@ namespace osu.Game.Rulesets.Edit
             layerAboveRuleset.Children = new Drawable[]
             {
                 maskLayer = new HitObjectMaskLayer(),
-                new PlacementContainer(compositionTool),
+                placementContainer = new PlacementContainer(),
             };
 
             layerContainers.Add(layerBelowRuleset);
@@ -117,14 +114,11 @@ namespace osu.Game.Rulesets.Edit
             };
 
             toolboxCollection.Items =
-                CompositionTools.Select(t => new RadioButton(t.Name, () => compositionTool.Value = t))
+                CompositionTools.Select(t => new RadioButton(t.Name, () => placementContainer.CurrentTool = t))
-                .Prepend(new RadioButton("Select", () => compositionTool.Value = null))
+                .Prepend(new RadioButton("Select", () => placementContainer.CurrentTool = null))
                 .ToList();
 
             toolboxCollection.Items[0].Select();
-
-            // Todo: no
-            placementHandler.PlacementFinished += h => maskLayer.AddMask(rulesetContainer.AddHitObject(h));
         }
 
         protected override IReadOnlyDependencyContainer CreateChildDependencies(IReadOnlyDependencyContainer parent)
@@ -157,6 +151,12 @@ namespace osu.Game.Rulesets.Edit
             });
         }
 
+        /// <summary>
+        /// Adds a <see cref="HitObject"/> to the <see cref="Beatmap"/> and visualises it.
+        /// </summary>
+        /// <param name="hitObject">The <see cref="HitObject"/> to add.</param>
+        public void Add(HitObject hitObject) => maskLayer.AddMaskFor(rulesetContainer.Add(hitObject));
+
         protected abstract EditRulesetContainer CreateRulesetContainer(Ruleset ruleset, WorkingBeatmap beatmap);
 
         protected abstract IReadOnlyList<HitObjectCompositionTool> CompositionTools { get; }

osu.Game/Screens/Edit/Screens/Compose/Compose.cs

@@ -1,7 +1,6 @@
 // Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
-using System;
 using JetBrains.Annotations;
 using osu.Framework.Allocation;
 using OpenTK.Graphics;
@@ -10,6 +9,7 @@ using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Framework.Graphics.Shapes;
 using osu.Framework.Logging;
+using osu.Game.Rulesets.Edit;
 using osu.Game.Rulesets.Objects;
 using osu.Game.Screens.Edit.Screens.Compose.Timeline;
 
@@ -21,19 +21,9 @@ namespace osu.Game.Screens.Edit.Screens.Compose
         private const float vertical_margins = 10;
         private const float horizontal_margins = 20;
 
-        /// <summary>
-        /// Invoked when the placement of a <see cref="HitObject"/> has started.
-        /// </summary>
-        public event Action<HitObject> PlacementStarted;
-
-        /// <summary>
-        /// Invoked when the placement of a <see cref="HitObject"/> has finished.
-        /// </summary>
-        public event Action<HitObject> PlacementFinished;
-
         private readonly BindableBeatDivisor beatDivisor = new BindableBeatDivisor();
 
-        private Container composerContainer;
+        private HitObjectComposer composer;
 
         [BackgroundDependencyLoader(true)]
         private void load([CanBeNull] BindableBeatDivisor beatDivisor)
@@ -41,6 +31,8 @@ namespace osu.Game.Screens.Edit.Screens.Compose
             if (beatDivisor != null)
                 this.beatDivisor.BindTo(beatDivisor);
 
+            Container composerContainer;
+
             Children = new Drawable[]
             {
                 new GridContainer
@@ -114,7 +106,7 @@ namespace osu.Game.Screens.Edit.Screens.Compose
                 return;
             }
 
-            var composer = ruleset.CreateHitObjectComposer();
+            composer = ruleset.CreateHitObjectComposer();
             if (composer == null)
             {
                 Logger.Log($"Ruleset {ruleset.Description} doesn't support hitobject composition.");
@@ -125,8 +117,10 @@ namespace osu.Game.Screens.Edit.Screens.Compose
             composerContainer.Child = composer;
         }
 
-        public void BeginPlacement(HitObject hitObject) => PlacementStarted?.Invoke(hitObject);
+        public void BeginPlacement(HitObject hitObject)
+        {
+        }
 
-        public void EndPlacement(HitObject hitObject) => PlacementFinished?.Invoke(hitObject);
+        public void EndPlacement(HitObject hitObject) => composer.Add(hitObject);
     }
 }

osu.Game/Screens/Edit/Screens/Compose/IPlacementHandler.cs

@@ -1,16 +1,12 @@
 // Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
-using System;
 using osu.Game.Rulesets.Objects;
 
 namespace osu.Game.Screens.Edit.Screens.Compose
 {
     public interface IPlacementHandler
     {
-        event Action<HitObject> PlacementStarted;
-        event Action<HitObject> PlacementFinished;
-
         void BeginPlacement(HitObject hitObject);
         void EndPlacement(HitObject hitObject);
     }

osu.Game/Screens/Edit/Screens/Compose/Layers/HitObjectMaskLayer.cs

@@ -13,10 +13,9 @@ namespace osu.Game.Screens.Edit.Screens.Compose.Layers
     public class HitObjectMaskLayer : CompositeDrawable
     {
         private MaskContainer maskContainer;
-        private HitObjectComposer composer;
 
         [Resolved]
-        private IPlacementHandler placementHandler { get; set; }
+        private HitObjectComposer composer { get; set; }
 
         public HitObjectMaskLayer()
         {
@@ -24,10 +23,8 @@ namespace osu.Game.Screens.Edit.Screens.Compose.Layers
         }
 
         [BackgroundDependencyLoader]
-        private void load(HitObjectComposer composer)
+        private void load()
         {
-            this.composer = composer;
-
             maskContainer = new MaskContainer();
 
             var maskSelection = composer.CreateMaskSelection();
@@ -51,7 +48,7 @@ namespace osu.Game.Screens.Edit.Screens.Compose.Layers
             };
 
             foreach (var obj in composer.HitObjects)
-                AddMask(obj);
+                AddMaskFor(obj);
         }
 
         protected override bool OnMouseDown(MouseDownEvent e)
@@ -64,7 +61,7 @@ namespace osu.Game.Screens.Edit.Screens.Compose.Layers
         /// Adds a mask for a <see cref="DrawableHitObject"/> which adds movement support.
         /// </summary>
         /// <param name="hitObject">The <see cref="DrawableHitObject"/> to create a mask for.</param>
-        public void AddMask(DrawableHitObject hitObject)
+        public void AddMaskFor(DrawableHitObject hitObject)
         {
             var mask = composer.CreateMaskFor(hitObject);
             if (mask == null)

osu.Game/Screens/Edit/Screens/Compose/Layers/PlacementContainer.cs

@@ -1,8 +1,6 @@
 // Copyright (c) 2007-2018 ppy Pty Ltd <contact@ppy.sh>.
 // Licensed under the MIT Licence - https://raw.githubusercontent.com/ppy/osu/master/LICENCE
 
-using osu.Framework.Allocation;
-using osu.Framework.Configuration;
 using osu.Framework.Graphics;
 using osu.Framework.Graphics.Containers;
 using osu.Game.Rulesets.Edit.Tools;
@@ -14,32 +12,37 @@ namespace osu.Game.Screens.Edit.Screens.Compose.Layers
     {
         private readonly Container maskContainer;
 
-        private readonly IBindable<HitObjectCompositionTool> compositionTool = new Bindable<HitObjectCompositionTool>();
+        public PlacementContainer()
-
-        [Resolved]
-        private IPlacementHandler placementHandler { get; set; }
-
-        public PlacementContainer(IBindable<HitObjectCompositionTool> compositionTool)
         {
-            this.compositionTool.BindTo(compositionTool);
-
             RelativeSizeAxes = Axes.Both;
-
-            this.compositionTool.BindValueChanged(onToolChanged);
         }
 
-        [BackgroundDependencyLoader]
+        private HitObjectCompositionTool currentTool;
-        private void load()
+
+        /// <summary>
+        /// The current placement tool.
+        /// </summary>
+        public HitObjectCompositionTool CurrentTool
         {
-            // Refresh the mask after each placement
+            get => currentTool;
-            placementHandler.PlacementFinished += _ => onToolChanged(compositionTool.Value);
+            set
+            {
+                if (currentTool == value)
+                    return;
+                currentTool = value;
+
+                Refresh();
+            }
         }
 
-        private void onToolChanged(HitObjectCompositionTool tool)
+        /// <summary>
+        /// Refreshes the current placement tool.
+        /// </summary>
+        public void Refresh()
         {
             ClearInternal();
 
-            var mask = tool?.CreatePlacementMask();
+            var mask = CurrentTool?.CreatePlacementMask();
             if (mask != null)
                 InternalChild = mask;
         }