ParseSettings and Moonscraper cleanup

Since the scanning process now pre-calculates HopoThreshold and SustainCutoffThreshold, we need to adjust the reader code so that it doesn't re-calculate the thresholds to incorrect values when called from a song entry.

+ Remove `HopoFreq_FoF` & `EighthNoteHopo` ParseSettings variables
+ Remove `GetHopoThreshold`-like functions
+ Convert MoonSong resolution to uint

YARG.Core.UnitTests/Parsing/ParseBehaviorTests.Midi.cs

@@ -20,7 +20,7 @@ namespace YARG.Core.UnitTests.Parsing
     public class MidiParseBehaviorTests
     {
         private const uint SUSTAIN_CUTOFF_THRESHOLD = RESOLUTION / 3;
-        private static readonly uint HopoThreshold = (uint)GetHopoThreshold(ParseSettings.Default, RESOLUTION);
+        private const uint HOPO_THRESHOLD = (RESOLUTION / 3) + 1;
 
         private static readonly Dictionary<MoonInstrument, string> InstrumentToNameLookup = new()
         {
@@ -450,7 +450,7 @@ private static void GenerateNotesForDifficulty<TNoteEvent>(MidiEventList events,
             if ((canForceStrum || canForceHopo) && (flags & Flags.Forced) != 0)
             {
                 MoonNoteType type;
-                if (canForceHopo && lastStartDelta >= HopoThreshold)
+                if (canForceHopo && lastStartDelta >= HOPO_THRESHOLD)
                 {
                     type = MoonNoteType.Hopo;
                     // Apply additional flag to match the parsed data

YARG.Core/Chart/Loaders/MoonSong/MoonSongLoader.Lyrics.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using MoonscraperChartEditor.Song;
@@ -69,7 +69,7 @@ public void AddPhraseEvent(string text, uint tick)
         public LyricsTrack LoadLyrics()
         {
             var converter = new LyricConverter(_moonSong);
-            var maxTick = _moonSong.Charts.Max(x => x.events.LastOrDefault()?.tick + (uint)_moonSong.resolution ?? 0);
+            var maxTick = _moonSong.Charts.Max(x => x.events.LastOrDefault()?.tick + _moonSong.resolution ?? 0);
             TextEvents.ConvertToPhrases(_moonSong.events, converter, maxTick);
             return new LyricsTrack(converter.Phrases);
         }

YARG.Core/Chart/Loaders/MoonSong/MoonSongLoader.cs

@@ -30,11 +30,8 @@ private delegate TNote CreateNoteDelegate<TNote>(MoonNote moonNote, CurrentPhras
         private MoonSong.MoonInstrument _currentMoonInstrument;
         private MoonSong.Difficulty _currentMoonDifficulty;
 
-        public MoonSongLoader(MoonSong song, ParseSettings settings)
+        public MoonSongLoader(MoonSong song, in ParseSettings settings)
         {
-            if (settings.NoteSnapThreshold < 0)
-                settings.NoteSnapThreshold = 0;
-
             _moonSong = song;
             _settings = settings;
         }
@@ -333,7 +330,7 @@ private static bool IsEventInPhrase(MoonObject songObj, MoonPhrase phrase)
             if (phrase.length == 0)
                 return songObj.tick == phrase.tick;
 
-            return songObj.tick >= phrase.tick && songObj.tick < (phrase.tick + phrase.length);
+            return phrase.tick <= songObj.tick && songObj.tick < (phrase.tick + phrase.length);
         }
 
         private static bool IsNoteClosestToEndOfPhrase(MoonSong song, MoonNote note, MoonPhrase phrase)
@@ -366,7 +363,7 @@ private static bool IsNoteClosestToEndOfPhrase(MoonSong song, MoonNote note, Moo
                     if (previousNote is null)
                     {
                         // This is the first note in the chart, check by distance
-                        float tickThreshold = song.resolution / 3; // 1/12th note
+                        uint tickThreshold = song.resolution / 3; // 1/12th note
                         return Math.Abs((int) note.tick - endTick) < tickThreshold;
                     }
                     else if (note.tick >= endTick && previousNote.tick < endTick)

YARG.Core/Chart/ParsingProperties.cs

@@ -72,23 +72,6 @@ public struct ParseSettings
         /// </remarks>
         public long HopoThreshold;
 
-        /// <summary>
-        /// The FoF HOPO threshold setting number to use.
-        /// </summary>
-        /// <remarks>
-        /// Uses the <c>hopofreq</c> tag from song.ini files.<br/>
-        /// 0 -> 1/24th note, 1 -> 1/16th note, 2 -> 1/12th note, 3 -> 1/8th note, 4 -> 1/6th note, 5 -> 1/4th note.
-        /// </remarks>
-        public int HopoFreq_FoF;
-
-        /// <summary>
-        /// Set the HOPO threshold to a 1/8th note instead of a 1/12th note.
-        /// </summary>
-        /// <remarks>
-        /// Uses the <c>eighthnote_hopo</c> tag from song.ini files.
-        /// </remarks>
-        public bool EighthNoteHopo;
-
         /// <summary>
         /// Skip marking single notes after chords as HOPOs
         /// if the single note shares a fret with the chord.
@@ -120,43 +103,5 @@ public struct ParseSettings
         /// Defaults to 116.
         /// </remarks>
         public int StarPowerNote;
-
-        /// <summary>
-        /// Calculates the HOPO threshold to use from the various HOPO settings.
-        /// </summary>
-        public readonly float GetHopoThreshold(float resolution)
-        {
-            // Prefer in this order:
-            // 1. hopo_threshold
-            // 2. eighthnote_hopo
-            // 3. hopofreq
-
-            if (HopoThreshold > 0)
-            {
-                return HopoThreshold;
-            }
-            else if (EighthNoteHopo)
-            {
-                return resolution / 2;
-            }
-            else if (HopoFreq_FoF >= 0)
-            {
-                int denominator = HopoFreq_FoF switch
-                {
-                    0 => 24,
-                    1 => 16,
-                    2 => 12,
-                    3 => 8,
-                    4 => 6,
-                    5 => 4,
-                    _ => throw new NotImplementedException($"Unhandled hopofreq value {HopoFreq_FoF}!")
-                };
-                return (resolution * 4) / denominator;
-            }
-            else
-            {
-                return resolution / 3;
-            }
-        }
     }
 }

YARG.Core/MoonscraperChartParser/IO/Chart/ChartIOHelper.cs

@@ -96,17 +96,5 @@ internal static class ChartIOHelper
             { "GHLRhythm",      MoonSong.MoonInstrument.GHLiveRhythm },
             { "GHLCoop",        MoonSong.MoonInstrument.GHLiveCoop },
         };
-
-        public static float GetHopoThreshold(in ParseSettings settings, float resolution)
-        {
-            // With a 192 resolution, .chart has a HOPO threshold of 65 ticks, not 64,
-            // so we need to scale this factor to different resolutions (480 res = 162.5 threshold)
-            // This extra tick is meant for some slight leniency; .mid has it too, but it's applied
-            // after factoring in the resolution there, not before.
-            const float DEFAULT_RESOLUTION = 192;
-            const float THRESHOLD_LENIENCY_FACTOR = 1 / DEFAULT_RESOLUTION;
-
-            return settings.GetHopoThreshold(resolution) + THRESHOLD_LENIENCY_FACTOR * resolution;
-        }
     }
 }

YARG.Core/MoonscraperChartParser/IO/Chart/ChartReader.cs

@@ -107,6 +107,7 @@ public static MoonSong ReadFromFile(ref ParseSettings settings, string filepath)
             }
         }
 
+        private const uint DEFAULT_RESOLUTION = 192;
         public static MoonSong ReadFromText(ref ParseSettings settings, ReadOnlySpan<char> chartText)
         {
             int textIndex = 0;
@@ -124,7 +125,15 @@ static void ExpectSection(ReadOnlySpan<char> chartText, ref int textIndex,
             // Check for the [Song] section first explicitly, need the Resolution property up-front
             ExpectSection(chartText, ref textIndex, ChartIOHelper.SECTION_SONG, out var sectionBody);
             var song = SubmitDataSong(sectionBody);
-            ValidateAndApplySettings(song, ref settings);
+
+            // With a 192 resolution, .chart has a HOPO threshold of 65 ticks, not 64,
+            // so we need to scale this factor to different resolutions (480 res = 162.5 threshold)
+            // This extra tick is meant for some slight leniency; .mid has it too, but it's applied
+            // after factoring in the resolution there, not before.
+            const uint THRESHOLD_AT_DEFAULT = 65;
+            song.hopoThreshold = settings.HopoThreshold > ParseSettings.SETTING_DEFAULT
+                ? (uint) settings.HopoThreshold
+                : (song.resolution * THRESHOLD_AT_DEFAULT) / DEFAULT_RESOLUTION;
 
             // Check for [SyncTrack] next, we need it for time conversions
             ExpectSection(chartText, ref textIndex, ChartIOHelper.SECTION_SYNC_TRACK, out sectionBody);
@@ -255,32 +264,19 @@ private static void SubmitChartData(ref ParseSettings settings, MoonSong song, R
 
         private static MoonSong SubmitDataSong(AsciiTrimSplitter sectionLines)
         {
+            uint resolution = DEFAULT_RESOLUTION;
             foreach (var line in sectionLines)
             {
                 var key = line.SplitOnceTrimmed('=', out var value);
                 value = value.Trim('"'); // Strip off any quotation marks
 
                 if (key.Equals("Resolution", StringComparison.Ordinal))
                 {
-                    uint resolution = (uint)FastInt32Parse(value);
-                    return new MoonSong(resolution);
+                    resolution = (uint)FastInt32Parse(value);
+                    break;
                 }
             }
-
-            throw new InvalidDataException("No resolution was found in the chart data!");
-        }
-
-        private static void ValidateAndApplySettings(MoonSong song, ref ParseSettings settings)
-        {
-            // Apply HOPO threshold settings
-            song.hopoThreshold = ChartIOHelper.GetHopoThreshold(in settings, song.resolution);
-
-            // Sustain cutoff threshold is not verified, sustains are not cut off by default in .chart
-            // SP note is not verified, as it is only relevant for .mid
-            // Note snap threshold is not verified, as the parser doesn't use it
-
-            // Chord HOPO cancellation does not apply in .chart
-            settings.ChordHopoCancellation = false;
+            return new MoonSong(resolution);
         }
 
         private static void SubmitDataSync(MoonSong song, AsciiTrimSplitter sectionLines)

YARG.Core/MoonscraperChartParser/IO/Midi/MidIOHelper.cs

@@ -1,4 +1,4 @@
-// Copyright (c) 2016-2020 Alexander Ong
+// Copyright (c) 2016-2020 Alexander Ong
 // See LICENSE in project root for license information.
 
 using System.Collections.Generic;
@@ -283,10 +283,5 @@ public static bool IsTextEvent(MidiEvent trackEvent, [NotNullWhen(true)] out Bas
 
             return false;
         }
-
-        public static float GetHopoThreshold(ParseSettings settings, float resolution)
-        {
-            return settings.GetHopoThreshold(resolution) + 1; // +1 for a small bit of leniency
-        }
     }
 }

YARG.Core/MoonscraperChartParser/IO/Midi/MidReader.cs

@@ -108,7 +108,18 @@ public static MoonSong ReadMidi(ref ParseSettings settings, MidiFile midi)
             var song = new MoonSong((uint)ticks.TicksPerQuarterNote);
 
             // Apply settings
-            ValidateAndApplySettings(song, ref settings);
+            song.hopoThreshold = settings.HopoThreshold > ParseSettings.SETTING_DEFAULT
+                ? (uint)settings.HopoThreshold 
+                : (song.resolution / 3);
+
+            if (settings.SustainCutoffThreshold <= ParseSettings.SETTING_DEFAULT)
+            {
+                settings.SustainCutoffThreshold = song.resolution / 3;
+            }
+
+            // +1 for a small bit of leniency
+            song.hopoThreshold++;
+            settings.SustainCutoffThreshold++;
 
             // Read all bpm data in first. This will also allow song.TimeToTick to function properly.
             ReadSync(midi.GetTempoMap(), song);
@@ -195,30 +206,6 @@ static void ReadProKeys(ref ParseSettings settings, TrackChunk track, MoonSong s
             }
         }
 
-        private static void ValidateAndApplySettings(MoonSong song, ref ParseSettings settings)
-        {
-            // Apply HOPO threshold settings
-            song.hopoThreshold = MidIOHelper.GetHopoThreshold(settings, song.resolution);
-
-            // Verify sustain cutoff threshold
-            if (settings.SustainCutoffThreshold < 0)
-            {
-                // Default to 1/12th step + 1
-                settings.SustainCutoffThreshold = (long) (song.resolution / 3) + 1;
-            }
-            else
-            {
-                // Limit minimum cutoff to 1 tick, non-sustain notes created by charting programs are 1 tick
-                settings.SustainCutoffThreshold = Math.Max(settings.SustainCutoffThreshold, 1);
-            }
-
-            // SP note is not verified, as it being set is checked for by SP fixups
-            // Note snap threshold is also not verified, as the parser doesn't use it
-
-            // Enable chord HOPO cancellation
-            settings.ChordHopoCancellation = true;
-        }
-
         private static void ReadSync(TempoMap tempoMap, MoonSong song)
         {
             YargLogger.LogTrace("Reading sync track");

YARG.Core/MoonscraperChartParser/MoonSong.cs

@@ -11,8 +11,8 @@ namespace MoonscraperChartEditor.Song
 {
     internal class MoonSong
     {
-        public float resolution => syncTrack.Resolution;
-        public float hopoThreshold;
+        public uint resolution => syncTrack.Resolution;
+        public uint hopoThreshold;
 
         // Charts
         private readonly MoonChart[] charts;
@@ -171,9 +171,9 @@ public bool Remove(MoonVenue venueEvent)
             return MoonObjectHelper.Remove(venueEvent, venue);
         }
 
-        public float ResolutionScaleRatio(float targetResoltion)
+        public float ResolutionScaleRatio(uint targetResoltion)
         {
-            return targetResoltion / resolution;
+            return (float)targetResoltion / resolution;
         }
 
         public static MoonChart.GameMode InstrumentToChartGameMode(MoonInstrument instrument)