Skip to content

Commit

Permalink
More refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
@NotHyper-474
NotHyper-474 committed Oct 26, 2024
1 parent 2d925a3 commit b68f06d
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 72 deletions.
2 changes: 1 addition & 1 deletion source/funkin/data/song/SongDataUtils.hx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class SongDataUtils
*/
public inline static function addNotes(notes:Array<SongNoteData>, addend:Array<SongNoteData>):Array<SongNoteData>
{
return SongNoteDataUtils.concatNoOverlap(notes, addend, ChartEditorState.STACK_NOTE_THRESHOLD, true);
return SongNoteDataUtils.concatNoOverlap(notes, addend, ChartEditorState.stackNoteThreshold);
}

/**
Expand Down
95 changes: 38 additions & 57 deletions source/funkin/data/song/SongNoteDataUtils.hx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SongNoteData>, threshold:Float):Array<SongNoteData>
public static function listStackedNotes(notes:Array<SongNoteData>, threshold:Float = 20):Array<SongNoteData>
{
var stackedNotes:Array<SongNoteData> = [];

Expand Down Expand Up @@ -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<SongNoteData>
* @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<SongNoteData>, notesB:Array<SongNoteData>, threshold:Float, modifyB:Bool = false):Array<SongNoteData>
public static function concatNoOverlap(notesA:Array<SongNoteData>, notesB:Array<SongNoteData>, threshold:Float = 20):Array<SongNoteData>
{
if (notesA == null || notesA.length == 0) return notesB;
if (notesB == null || notesB.length == 0) return notesA;

var result:Array<SongNoteData> = notesA.copy();
var overlappingNotes:Array<SongNoteData> = [];

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<SongNoteData>, rhs:Array<SongNoteData>, threshold:Float,
?overwrittenNotes:Array<SongNoteData>):Array<SongNoteData>
public static function concatOverwrite(lhs:Array<SongNoteData>, rhs:Array<SongNoteData>, ?overwrittenNotes:Array<SongNoteData>,
threshold:Float = 20):Array<SongNoteData>
{
if (lhs == null || rhs == null || rhs.length == 0) return lhs;

Expand All @@ -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;
Expand All @@ -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;
}
}
20 changes: 12 additions & 8 deletions source/funkin/ui/debug/charting/ChartEditorState.hx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,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.
*/
Expand Down Expand Up @@ -814,6 +808,12 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState
*/
var currentLiveInputPlaceNoteData:Array<SongNoteData> = [];

/**
* 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

/**
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -3741,7 +3746,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.
Expand Down Expand Up @@ -3805,7 +3810,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
Expand Down
17 changes: 11 additions & 6 deletions source/funkin/ui/debug/charting/commands/PasteItemsCommand.hx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -17,7 +17,6 @@ class PasteItemsCommand implements ChartEditorCommand
// Notes we added and removed with this command, for undo.
var addedNotes:Array<SongNoteData> = [];
var addedEvents:Array<SongEventData> = [];
var removedNotes:Array<SongNoteData> = [];

public function new(targetTimestamp:Float)
{
Expand All @@ -42,19 +41,25 @@ 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;
state.notePreviewDirty = true;

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.');
}
Expand All @@ -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 = [];
Expand All @@ -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
Expand Down

0 comments on commit b68f06d

Please sign in to comment.