Refactor Ornamentation (#212)

wbaldoumas · Dec 14, 2024 · 8ecfa2a · 8ecfa2a
1 parent 3d52763
commit 8ecfa2a
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 50 deletions.
diff --git a/src/BaroquenMelody.Library/Ornamentation/Cleaning/Engine/Processors/OrnamentationCleaner.cs b/src/BaroquenMelody.Library/Ornamentation/Cleaning/Engine/Processors/OrnamentationCleaner.cs
@@ -22,8 +22,10 @@ public void Process(OrnamentationCleaningItem item)
         var notePair = configuration.NotePairSelector.Select(item);
 
         var hasStrongPulseConflicts = configuration.NoteIndexPairs
-            .Where(noteIndexPair => noteIndexPair.OccursOnStrongPulse)
-            .Any(noteIndexPair => notePair.PrimaryNote.Ornamentations[noteIndexPair.PrimaryNoteIndex].IsDissonantWith(notePair.SecondaryNote.Ornamentations[noteIndexPair.SecondaryNoteIndex]));
+            .Any(noteIndexPair =>
+                noteIndexPair.OccursOnStrongPulse &&
+                notePair.PrimaryNote.Ornamentations[noteIndexPair.PrimaryNoteIndex].IsDissonantWith(notePair.SecondaryNote.Ornamentations[noteIndexPair.SecondaryNoteIndex])
+            );
 
         if (hasStrongPulseConflicts)
         {
@@ -33,8 +35,10 @@ public void Process(OrnamentationCleaningItem item)
         }
 
         var hasWeakPulseConflicts = configuration.NoteIndexPairs
-            .Where(noteIndexPair => !noteIndexPair.OccursOnStrongPulse)
-            .Any(noteIndexPair => notePair.PrimaryNote.Ornamentations[noteIndexPair.PrimaryNoteIndex].IsDissonantWith(notePair.SecondaryNote.Ornamentations[noteIndexPair.SecondaryNoteIndex]));
+            .Any(noteIndexPair =>
+                !noteIndexPair.OccursOnStrongPulse &&
+                notePair.PrimaryNote.Ornamentations[noteIndexPair.PrimaryNoteIndex].IsDissonantWith(notePair.SecondaryNote.Ornamentations[noteIndexPair.SecondaryNoteIndex])
+            );
 
         if (hasWeakPulseConflicts && weightedRandomBooleanGenerator.IsTrue(ChanceOfCleaningWeakConflicts))
         {
@@ -44,8 +48,6 @@ public void Process(OrnamentationCleaningItem item)
 
     private void CleanConflictingOrnamentation(OrnamentationCleaningItem item)
     {
-        var noteToClean = configuration.NoteTargetSelector.Select(item);
-
-        noteToClean.ResetOrnamentation(compositionConfiguration.DefaultNoteTimeSpan);
+        item.Note.ResetOrnamentation(compositionConfiguration.DefaultNoteTimeSpan);
     }
 }
diff --git a/src/BaroquenMelody.Library/Ornamentation/CompositionDecorator.cs b/src/BaroquenMelody.Library/Ornamentation/CompositionDecorator.cs
@@ -1,4 +1,4 @@
-using Atrea.PolicyEngine.Processors;
+using Atrea.PolicyEngine;
 using BaroquenMelody.Infrastructure.Collections;
 using BaroquenMelody.Library.Configurations;
 using BaroquenMelody.Library.Domain;
@@ -8,12 +8,12 @@ namespace BaroquenMelody.Library.Ornamentation;
 
 /// <inheritdoc cref="ICompositionDecorator"/>
 internal sealed class CompositionDecorator(
-    IProcessor<OrnamentationItem> ornamentationEngine,
-    IProcessor<OrnamentationItem> sustainEngine,
+    IPolicyEngine<OrnamentationItem> ornamentationEngine,
+    IPolicyEngine<OrnamentationItem> sustainEngine,
     CompositionConfiguration configuration
 ) : ICompositionDecorator
 {
-    public void Decorate(Composition composition) => Decorate(composition, ornamentationEngine);
+    public void Decorate(Composition composition) => Decorate(composition, ornamentationEngine, shuffleProcessors: true);
 
     public void ApplySustain(Composition composition) => Decorate(composition, sustainEngine);
 
@@ -22,22 +22,27 @@ public void Decorate(Composition composition, Instrument instrument)
         var compositionContext = new FixedSizeList<Beat>(configuration.CompositionContextSize);
         var beats = composition.Measures.SelectMany(static measure => measure.Beats).ToList();
 
-        Decorate(instrument, beats, compositionContext, ornamentationEngine);
+        Decorate(instrument, beats, compositionContext, ornamentationEngine, shuffleProcessors: true);
     }
 
-    private void Decorate(Composition composition, IProcessor<OrnamentationItem> processor)
+    private void Decorate(Composition composition, IPolicyEngine<OrnamentationItem> processor, bool shuffleProcessors = false)
     {
         var compositionContext = new FixedSizeList<Beat>(configuration.CompositionContextSize);
         var instruments = configuration.InstrumentConfigurations.Select(static instrumentConfiguration => instrumentConfiguration.Instrument);
         var beats = composition.Measures.SelectMany(static measure => measure.Beats).ToList();
 
         foreach (var instrument in instruments)
         {
-            Decorate(instrument, beats, compositionContext, processor);
+            Decorate(instrument, beats, compositionContext, processor, shuffleProcessors);
         }
     }
 
-    private static void Decorate(Instrument instrument, List<Beat> beats, FixedSizeList<Beat> compositionContext, IProcessor<OrnamentationItem> processor)
+    private static void Decorate(
+        Instrument instrument,
+        List<Beat> beats,
+        FixedSizeList<Beat> compositionContext,
+        IPolicyEngine<OrnamentationItem> processor,
+        bool shuffleProcessors = false)
     {
         for (var i = 0; i < beats.Count; ++i)
         {
@@ -53,6 +58,11 @@ private static void Decorate(Instrument instrument, List<Beat> beats, FixedSizeL
 
             processor.Process(ornamentationItem);
 
+            if (shuffleProcessors)
+            {
+                processor.Shuffle();
+            }
+
             compositionContext.Add(beats[i]);
         }
     }

diff --git a/src/BaroquenMelody.Library/Ornamentation/Engine/OrnamentationEngineBuilder.cs b/src/BaroquenMelody.Library/Ornamentation/Engine/OrnamentationEngineBuilder.cs
@@ -25,39 +25,59 @@
 namespace BaroquenMelody.Library.Ornamentation.Engine;
 
 [ExcludeFromCodeCoverage(Justification = "Trivial builder methods.")]
-internal sealed class OrnamentationEngineBuilder(CompositionConfiguration compositionConfiguration, IMusicalTimeSpanCalculator musicalTimeSpanCalculator, ILogger logger)
+internal sealed class OrnamentationEngineBuilder
 {
     private readonly IWeightedRandomBooleanGenerator _weightedRandomBooleanGenerator = new WeightedRandomBooleanGenerator();
 
     private readonly IInputPolicy<OrnamentationItem> _hasNoOrnamentation = new Not<OrnamentationItem>(new HasOrnamentation());
 
-    private readonly NoteIndexPairSelector _noteIndexPairSelector = new(new NoteOnsetCalculator(musicalTimeSpanCalculator, compositionConfiguration));
+    private readonly NoteIndexPairSelector _noteIndexPairSelector;
 
-    private readonly OrnamentationProcessorFactory _processorFactory = new(
-        musicalTimeSpanCalculator,
-        new OrnamentationProcessorConfigurationFactory(
-            new ChordNumberIdentifier(compositionConfiguration),
-            new WeightedRandomBooleanGenerator(),
-            compositionConfiguration,
-            logger
-        )
-    );
+    private readonly OrnamentationProcessorFactory _processorFactory;
+
+    private readonly CompositionConfiguration _compositionConfiguration;
+
+    private readonly IMusicalTimeSpanCalculator _musicalTimeSpanCalculator;
+
+    private readonly ILogger _logger;
+
+    public OrnamentationEngineBuilder(
+        CompositionConfiguration compositionConfiguration,
+        IMusicalTimeSpanCalculator musicalTimeSpanCalculator,
+        ILogger logger)
+    {
+        _compositionConfiguration = compositionConfiguration;
+        _musicalTimeSpanCalculator = musicalTimeSpanCalculator;
+        _logger = logger;
+        _noteIndexPairSelector = new NoteIndexPairSelector(new NoteOnsetCalculator(musicalTimeSpanCalculator, compositionConfiguration));
+
+        _processorFactory = new OrnamentationProcessorFactory(
+            musicalTimeSpanCalculator,
+            new OrnamentationProcessorConfigurationFactory(
+                new ChordNumberIdentifier(compositionConfiguration),
+                new WeightedRandomBooleanGenerator(),
+                compositionConfiguration,
+                logger
+            ),
+            new CleanConflictingOrnamentations(BuildOrnamentationCleaningEngine())
+        );
+    }
 
     public IPolicyEngine<OrnamentationItem> BuildOrnamentationEngine() => PolicyEngineBuilder<OrnamentationItem>.Configure()
         .WithoutInputPolicies()
-        .WithProcessors(_processorFactory.Create(compositionConfiguration).ToArray())
-        .WithOutputPolicies(new CleanConflictingOrnamentations(BuildOrnamentationCleaningEngine()))
+        .WithProcessors(_processorFactory.Create(_compositionConfiguration).ToArray())
+        .WithoutOutputPolicies()
         .Build();
 
-    public IProcessor<OrnamentationItem> BuildSustainedNoteEngine() => PolicyEngineBuilder<OrnamentationItem>.Configure()
+    public IPolicyEngine<OrnamentationItem> BuildSustainedNoteEngine() => PolicyEngineBuilder<OrnamentationItem>.Configure()
         .WithInputPolicies(
             new WantsToOrnament(_weightedRandomBooleanGenerator),
             new IsRepeatedNote(),
             _hasNoOrnamentation,
-            new IsApplicableInterval(compositionConfiguration, SustainedNoteProcessor.Interval)
+            new IsApplicableInterval(_compositionConfiguration, SustainedNoteProcessor.Interval)
         )
-        .WithProcessors(new SustainedNoteProcessor(musicalTimeSpanCalculator, compositionConfiguration))
-        .WithOutputPolicies(new LogOrnamentation(OrnamentationType.Sustain, logger))
+        .WithProcessors(new SustainedNoteProcessor(_musicalTimeSpanCalculator, _compositionConfiguration))
+        .WithOutputPolicies(new LogOrnamentation(OrnamentationType.Sustain, _logger))
         .Build();
 
     private IPolicyEngine<OrnamentationCleaningItem> BuildOrnamentationCleaningEngine()
@@ -99,7 +119,7 @@ and not OrnamentationType.Rest
             var processor = PolicyEngineBuilder<OrnamentationCleaningItem>
                 .Configure()
                 .WithInputPolicies(new HasTargetOrnamentations(primaryOrnamentation, secondaryOrnamentation))
-                .WithProcessors(new OrnamentationCleaner(ornamentationCleaningConfiguration, compositionConfiguration, _weightedRandomBooleanGenerator))
+                .WithProcessors(new OrnamentationCleaner(ornamentationCleaningConfiguration, _compositionConfiguration, _weightedRandomBooleanGenerator))
                 .Build();
 
             processors.Add(processor);

diff --git a/...quenMelody.Library/Ornamentation/Engine/Policies/Output/CleanConflictingOrnamentations.cs b/...quenMelody.Library/Ornamentation/Engine/Policies/Output/CleanConflictingOrnamentations.cs
@@ -2,7 +2,6 @@
 using Atrea.PolicyEngine.Policies.Output;
 using BaroquenMelody.Library.Enums;
 using BaroquenMelody.Library.Ornamentation.Cleaning;
-using LazyCart;
 
 namespace BaroquenMelody.Library.Ornamentation.Engine.Policies.Output;
 
@@ -13,26 +12,22 @@ internal sealed class CleanConflictingOrnamentations(IPolicyEngine<Ornamentation
 {
     public void Apply(OrnamentationItem item)
     {
-        var instruments = item.CurrentBeat.Chord.Notes.Select(note => note.Instrument).ToList();
-        var instrumentCombinations = new LazyCartesianProduct<Instrument, Instrument>(instruments, instruments);
+        var instruments = item.CurrentBeat.Chord.Notes.Select(note => note.Instrument).ToHashSet();
         var processedInstrumentCombinations = new HashSet<(Instrument, Instrument)>();
 
-        for (var i = 0; i < instrumentCombinations.Size; ++i)
+        foreach (var otherInstrument in instruments)
         {
-            var (instrumentA, instrumentB) = instrumentCombinations[i];
-
-            if (instrumentA == instrumentB || processedInstrumentCombinations.Contains((instrumentA, instrumentB)))
+            if (item.Instrument == otherInstrument || processedInstrumentCombinations.Contains((item.Instrument, otherInstrument)))
             {
                 continue;
             }
 
-            var noteA = item.CurrentBeat[instrumentA];
-            var noteB = item.CurrentBeat[instrumentB];
+            var note = item.CurrentBeat[item.Instrument];
+            var otherNote = item.CurrentBeat[otherInstrument];
 
-            ornamentationCleaningEngine.Process(new OrnamentationCleaningItem(noteA, noteB));
+            ornamentationCleaningEngine.Process(new OrnamentationCleaningItem(note, otherNote));
 
-            processedInstrumentCombinations.Add((instrumentA, instrumentB));
-            processedInstrumentCombinations.Add((instrumentB, instrumentA));
+            processedInstrumentCombinations.Add((item.Instrument, otherInstrument));
         }
     }
 }
diff --git a/...Melody.Library/Ornamentation/Engine/Processors/Factories/OrnamentationProcessorFactory.cs b/...Melody.Library/Ornamentation/Engine/Processors/Factories/OrnamentationProcessorFactory.cs
@@ -1,4 +1,5 @@
 using Atrea.PolicyEngine.Builders;
+using Atrea.PolicyEngine.Policies.Output;
 using Atrea.PolicyEngine.Processors;
 using BaroquenMelody.Library.Configurations;
 using BaroquenMelody.Library.Ornamentation.Utilities;
@@ -7,15 +8,17 @@ namespace BaroquenMelody.Library.Ornamentation.Engine.Processors.Factories;
 
 internal sealed class OrnamentationProcessorFactory(
     IMusicalTimeSpanCalculator musicalTimeSpanCalculator,
-    IOrnamentationProcessorConfigurationFactory configurationFactory
+    IOrnamentationProcessorConfigurationFactory configurationFactory,
+    IOutputPolicy<OrnamentationItem> ornamentationCleaningOutputPolicy
 ) : IOrnamentationProcessorFactory
 {
     public IEnumerable<IProcessor<OrnamentationItem>> Create(CompositionConfiguration compositionConfiguration) =>
-        from ornamentationConfiguration in compositionConfiguration.AggregateOrnamentationConfiguration.Configurations.Where(configuration => configuration.IsEnabled)
-        from processorConfiguration in configurationFactory.Create(ornamentationConfiguration)
+        from configuration in compositionConfiguration.AggregateOrnamentationConfiguration.Configurations
+        where configuration.IsEnabled
+        from processorConfiguration in configurationFactory.Create(configuration)
         select PolicyEngineBuilder<OrnamentationItem>.Configure()
             .WithInputPolicies(processorConfiguration.InputPolicies)
             .WithProcessors(new OrnamentationProcessor(musicalTimeSpanCalculator, compositionConfiguration, processorConfiguration))
-            .WithOutputPolicies(processorConfiguration.OutputPolicies)
+            .WithOutputPolicies([.. processorConfiguration.OutputPolicies, ornamentationCleaningOutputPolicy])
             .Build();
 }
diff --git a/...Library.Tests/Ornamentation/Engine/Policies/Output/CleanConflictingOrnamentationsTests.cs b/...Library.Tests/Ornamentation/Engine/Policies/Output/CleanConflictingOrnamentationsTests.cs
@@ -54,6 +54,6 @@ public void Apply_Invokes_Expected_Components_Expected_Number_Of_Times()
         _cleanConflictingOrnamentations.Apply(ornamentationItem);
 
         // assert
-        _mockOrnamentationCleaningEngine.Received(6).Process(Arg.Any<OrnamentationCleaningItem>());
+        _mockOrnamentationCleaningEngine.Received(3).Process(Arg.Any<OrnamentationCleaningItem>());
     }
 }