From 4b9b87a77da477b8c658ce45f254fe668be30616 Mon Sep 17 00:00:00 2001 From: Hyper_ Date: Sun, 20 Oct 2024 21:42:41 -0300 Subject: [PATCH] More refactoring --- source/funkin/data/song/SongDataUtils.hx | 2 +- source/funkin/data/song/SongNoteDataUtils.hx | 95 ++++++++----------- .../ui/debug/charting/ChartEditorState.hx | 20 ++-- .../charting/commands/PasteItemsCommand.hx | 17 ++-- 4 files changed, 62 insertions(+), 72 deletions(-) diff --git a/source/funkin/data/song/SongDataUtils.hx b/source/funkin/data/song/SongDataUtils.hx index 2f015d2ec6..f9a3c0f710 100644 --- a/source/funkin/data/song/SongDataUtils.hx +++ b/source/funkin/data/song/SongDataUtils.hx @@ -90,7 +90,7 @@ class SongDataUtils */ public inline static function addNotes(notes:Array, addend:Array):Array { - return SongNoteDataUtils.concatNoOverlap(notes, addend, ChartEditorState.STACK_NOTE_THRESHOLD, true); + return SongNoteDataUtils.concatNoOverlap(notes, addend, ChartEditorState.stackNoteThreshold); } /** diff --git a/source/funkin/data/song/SongNoteDataUtils.hx b/source/funkin/data/song/SongNoteDataUtils.hx index 4d1fd21c2a..0fda581ec9 100644 --- a/source/funkin/data/song/SongNoteDataUtils.hx +++ b/source/funkin/data/song/SongNoteDataUtils.hx @@ -11,11 +11,12 @@ class SongNoteDataUtils /** * Retrieves all stacked notes + * * @param notes Sorted notes by time * @param threshold Threshold in ms * @return Stacked notes */ - public static function listStackedNotes(notes:Array, threshold:Float):Array + public static function listStackedNotes(notes:Array, threshold:Float = 20):Array { var stackedNotes:Array = []; @@ -68,64 +69,46 @@ class SongNoteDataUtils /** * Tries to concatenate two arrays of notes together but skips notes from `notesB` that overlap notes from `noteA`. - * This does not modify the original array. + * This operation modifies the second array by removing the overlapped notes + * * @param notesA An array of notes into which `notesB` will be concatenated. - * @param notesB Another array of notes that will be concated into `notesA`. - * @param threshold Threshold in ms - * @param modifyB If `true`, `notesB` will be modified in-place by removing the notes that overlap notes from `notesA`. - * @return Array + * @param notesB Another array of notes that will be concatenated into `notesA`. + * @param threshold Threshold in ms. + * @return The unsorted resulting array. */ - public static function concatNoOverlap(notesA:Array, notesB:Array, threshold:Float, modifyB:Bool = false):Array + public static function concatNoOverlap(notesA:Array, notesB:Array, threshold:Float = 20):Array { if (notesA == null || notesA.length == 0) return notesB; if (notesB == null || notesB.length == 0) return notesA; - var result:Array = notesA.copy(); - var overlappingNotes:Array = []; - - for (noteB in notesB) - { - var hasOverlap:Bool = false; - + var addend = notesB.copy(); + addend = addend.filter((noteB) -> { for (noteA in notesA) { if (doNotesStack(noteA, noteB, threshold)) { - hasOverlap = true; - break; + notesB.remove(noteB); + return false; } } + return true; + }); - if (!hasOverlap) - { - result.push(noteB); - } - else if (modifyB) - { - overlappingNotes.push(noteB); - } - } - - if (modifyB) - { - for (note in overlappingNotes) - notesB.remove(note); - } - - return result; + return notesA.concat(addend); } /** * Concatenates two arrays of notes but overwrites notes in `lhs` that are overlapped by notes from `rhs`. - * This does not modify the fist array but does modify the second. - * @param lhs - * @param rhs + * This operation only modifies the second array and `overwrittenNotes`. + * + * @param lhs An array of notes + * @param rhs An array of notes to concatenate into `lhs` + * @param overwrittenNotes An optional array that is modified in-place with the notes in `lhs` that were overwritten. * @param threshold Threshold in ms - * @param overwrittenNotes An array that is modified in-place with the notes in `lhs` that were overwritten. - * @return `lhs` + `rhs` + * @return The resulting array, note that the added notes are placed at the end of the array. */ - public static function concatOverwrite(lhs:Array, rhs:Array, threshold:Float, - ?overwrittenNotes:Array):Array + public static function concatOverwrite(lhs:Array, rhs:Array, ?overwrittenNotes:Array, + threshold:Float = 20):Array { if (lhs == null || rhs == null || rhs.length == 0) return lhs; @@ -145,7 +128,6 @@ class SongNoteDataUtils { overwrittenNotes?.push(result[j].clone()); result[j] = noteB; - // Do not resize array with remove() now to not screw with loop rhs[i] = null; } hasOverlap = true; @@ -155,39 +137,38 @@ class SongNoteDataUtils if (!hasOverlap) result.push(noteB); } - - // Now we can safely resize it rhs = rhs.filterNull(); return result; } /** - * @param threshold + * @param threshold Time difference in milliseconds. * @return Returns `true` if both notes are on the same strumline, have the same direction and their time difference is less than `threshold`. */ - public static function doNotesStack(noteA:SongNoteData, noteB:SongNoteData, threshold:Float):Bool + public static function doNotesStack(noteA:SongNoteData, noteB:SongNoteData, threshold:Float = 20):Bool { + // TODO: Make this function inline again when I'm done debugging. return noteA.data == noteB.data && Math.ffloor(Math.abs(noteA.time - noteB.time)) <= threshold; } // This is replacing SongNoteData's equals operator because for some reason its params check is unreliable. - static function noteEquals(noteA:SongNoteData, other:SongNoteData):Bool + static function noteEquals(note:SongNoteData, other:SongNoteData):Bool { - if (noteA == null) return other == null; + if (note == null) return other == null; if (other == null) return false; - // TESTME: These checks seem redundant when kind's getter already returns null if it's an empty string. - if (noteA.kind == null || noteA.kind == '') - { - if (other.kind != '' && noteA.kind != null) return false; - } - else - { - if (other.kind == '' || noteA.kind == null) return false; - } + // TESTME: These checks seem redundant when get_kind already returns null if it's an empty string. + /*if (noteA.kind == null) + { + if (other.kind != null) return false; + } + else + { + if (other.kind == null) return false; + }*/ // params check is unreliable and doNotesStack already checks data - return noteA.time == other.time && noteA.length == other.length; + return note.time == other.time && note.length == other.length && note.kind == other.kind; } } diff --git a/source/funkin/ui/debug/charting/ChartEditorState.hx b/source/funkin/ui/debug/charting/ChartEditorState.hx index 87bbe9585d..09f441d3a8 100644 --- a/source/funkin/ui/debug/charting/ChartEditorState.hx +++ b/source/funkin/ui/debug/charting/ChartEditorState.hx @@ -202,12 +202,6 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState */ public static final NOTE_SELECT_BUTTON_HEIGHT:Int = 24; - /** - * How "close" in milliseconds two notes have to be to be considered as stacked. - * TODO: This should probably be turned into a customizable value - */ - public static final STACK_NOTE_THRESHOLD:Int = 20; - /** * The amount of padding between the menu bar and the chart grid when fully scrolled up. */ @@ -814,6 +808,12 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState */ var currentLiveInputPlaceNoteData:Array = []; + /** + * How "close" in milliseconds two notes have to be to be considered as stacked. + * For instance, `0` means the notes should be exactly on top of each other + */ + public static var stackNoteThreshold:Int = 20; + // Note Movement /** @@ -1826,6 +1826,11 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState */ var menuBarItemNoteSnapIncrease:MenuItem; + /** + * The `Edit -> Stacked Note Threshold` menu item + */ + var menuBarItemStackedNoteThreshold:MenuItem; + /** * The `View -> Downscroll` menu item. */ @@ -3737,7 +3742,7 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState } // Gather stacked notes to render later - var stackedNotes = SongNoteDataUtils.listStackedNotes(currentSongChartNoteData, STACK_NOTE_THRESHOLD); + var stackedNotes = SongNoteDataUtils.listStackedNotes(currentSongChartNoteData, stackNoteThreshold); // Readd selection squares for selected notes. // Recycle selection squares if possible. @@ -3801,7 +3806,6 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState var stepLength = noteSprite.noteData.getStepLength(); selectionSquare.height = (stepLength <= 0) ? GRID_SIZE : ((stepLength + 1) * GRID_SIZE); } - // TODO: Move this to a function like isNoteSelected does else if (doesNoteStack(noteSprite.noteData, stackedNotes)) { // TODO: Maybe use another way to display these notes diff --git a/source/funkin/ui/debug/charting/commands/PasteItemsCommand.hx b/source/funkin/ui/debug/charting/commands/PasteItemsCommand.hx index 52bcf5df4c..92511da87c 100644 --- a/source/funkin/ui/debug/charting/commands/PasteItemsCommand.hx +++ b/source/funkin/ui/debug/charting/commands/PasteItemsCommand.hx @@ -4,7 +4,7 @@ import funkin.data.song.SongData.SongEventData; import funkin.data.song.SongData.SongNoteData; import funkin.data.song.SongDataUtils; import funkin.data.song.SongDataUtils.SongClipboardItems; -import funkin.data.song.SongNoteDataUtils; +import funkin.ui.debug.charting.ChartEditorState; /** * A command which inserts the contents of the clipboard into the chart editor. @@ -17,7 +17,6 @@ class PasteItemsCommand implements ChartEditorCommand // Notes we added and removed with this command, for undo. var addedNotes:Array = []; var addedEvents:Array = []; - var removedNotes:Array = []; public function new(targetTimestamp:Float) { @@ -42,11 +41,15 @@ class PasteItemsCommand implements ChartEditorCommand addedNotes = SongDataUtils.clampSongNoteData(addedNotes, 0.0, msCutoff); addedEvents = SongDataUtils.offsetSongEventData(currentClipboard.events, Std.int(targetTimestamp)); addedEvents = SongDataUtils.clampSongEventData(addedEvents, 0.0, msCutoff); + var removedNotes = addedNotes.copy(); state.currentSongChartNoteData = SongDataUtils.addNotes(state.currentSongChartNoteData, addedNotes); + // SongNoteDataUtils.concatOverwrite(state.currentSongChartNoteData, addedNotes, removedNotes, + // ChartEditorState.stackNoteThreshold); state.currentSongChartEventData = state.currentSongChartEventData.concat(addedEvents); - state.currentNoteSelection = addedNotes.copy(); + state.currentNoteSelection = removedNotes.copy(); state.currentEventSelection = addedEvents.copy(); + removedNotes = SongDataUtils.subtractNotes(removedNotes, addedNotes); state.saveDataDirty = true; state.noteDisplayDirty = true; @@ -54,7 +57,9 @@ class PasteItemsCommand implements ChartEditorCommand state.sortChartData(); - if (removedNotes.length > 0) state.warning('Paste Successful', 'However overlapped notes were overwritten.'); + // FIXME: execute() is reused as a redo function so these messages show up even when not actually pasting + if (addedNotes.length == 0) state.error('Paste Failed', 'All notes would overlap already placed notes.') + else if (removedNotes.length > 0) state.warning('Paste Successful', 'However overlapping notes were ignored.'); else state.success('Paste Successful', 'Successfully pasted clipboard contents.'); } @@ -63,7 +68,7 @@ class PasteItemsCommand implements ChartEditorCommand { state.playSound(Paths.sound('chartingSounds/undo')); - state.currentSongChartNoteData = SongDataUtils.subtractNotes(state.currentSongChartNoteData, addedNotes).concat(removedNotes); + state.currentSongChartNoteData = SongDataUtils.subtractNotes(state.currentSongChartNoteData, addedNotes); state.currentSongChartEventData = SongDataUtils.subtractEvents(state.currentSongChartEventData, addedEvents); state.currentNoteSelection = []; state.currentEventSelection = []; @@ -78,7 +83,7 @@ class PasteItemsCommand implements ChartEditorCommand public function shouldAddToHistory(state:ChartEditorState):Bool { // This command is undoable. Add to the history if we actually performed an action. - return (addedNotes.length > 0 || addedEvents.length > 0 || removedNotes.length > 0); + return (addedNotes.length > 0 || addedEvents.length > 0); } public function toString():String