Fix a bug where modifying a copied template song's BPM in the chart editor would modify BPM in Freeplay.

source/funkin/data/song/SongData.hx

@@ -2,6 +2,7 @@ package funkin.data.song;
 
 import funkin.data.song.SongRegistry;
 import thx.semver.Version;
+import funkin.util.tools.ICloneable;
 
 /**
  * Data containing information about a song.
@@ -9,7 +10,7 @@ import thx.semver.Version;
  * Data which is only necessary in-game should be stored in the SongChartData.
  */
 @:nullSafety
-class SongMetadata
+class SongMetadata implements ICloneable<SongMetadata>
 {
   /**
    * A semantic versioning string for the song data format.
@@ -84,16 +85,16 @@ class SongMetadata
    * @param newVariation Set to a new variation ID to change the new metadata.
    * @return The cloned SongMetadata
    */
-  public function clone(?newVariation:String = null):SongMetadata
+  public function clone():SongMetadata
   {
-    var result:SongMetadata = new SongMetadata(this.songName, this.artist, newVariation == null ? this.variation : newVariation);
+    var result:SongMetadata = new SongMetadata(this.songName, this.artist, this.variation);
     result.version = this.version;
     result.timeFormat = this.timeFormat;
     result.divisions = this.divisions;
-    result.offsets = this.offsets;
-    result.timeChanges = this.timeChanges;
+    result.offsets = this.offsets.clone();
+    result.timeChanges = this.timeChanges.deepClone();
     result.looped = this.looped;
-    result.playData = this.playData;
+    result.playData = this.playData.clone();
     result.generatedBy = this.generatedBy;
 
     return result;
@@ -128,7 +129,7 @@ enum abstract SongTimeFormat(String) from String to String
   var MILLISECONDS = 'ms';
 }
 
-class SongTimeChange
+class SongTimeChange implements ICloneable<SongTimeChange>
 {
   public static final DEFAULT_SONGTIMECHANGE:SongTimeChange = new SongTimeChange(0, 100);
 
@@ -195,6 +196,11 @@ class SongTimeChange
     this.beatTuplets = beatTuplets == null ? DEFAULT_BEAT_TUPLETS : beatTuplets;
   }
 
+  public function clone():SongTimeChange
+  {
+    return new SongTimeChange(this.timeStamp, this.bpm, this.timeSignatureNum, this.timeSignatureDen, this.beatTime, this.beatTuplets);
+  }
+
   /**
    * Produces a string representation suitable for debugging.
    */
@@ -209,7 +215,7 @@ class SongTimeChange
  * These are intended to correct for issues with the chart, or with the song's audio (for example a 10ms delay before the song starts).
  * This is independent of the offsets applied in the user's settings, which are applied after these offsets and intended to correct for the user's hardware.
  */
-class SongOffsets
+class SongOffsets implements ICloneable<SongOffsets>
 {
   /**
    * The offset, in milliseconds, to apply to the song's instrumental relative to the chart.
@@ -279,6 +285,15 @@ class SongOffsets
     return value;
   }
 
+  public function clone():SongOffsets
+  {
+    var result:SongOffsets = new SongOffsets(this.instrumental);
+    result.altInstrumentals = this.altInstrumentals.clone();
+    result.vocals = this.vocals.clone();
+
+    return result;
+  }
+
   /**
    * Produces a string representation suitable for debugging.
    */
@@ -292,7 +307,7 @@ class SongOffsets
  * Metadata for a song only used for the music.
  * For example, the menu music.
  */
-class SongMusicData
+class SongMusicData implements ICloneable<SongMusicData>
 {
   /**
    * A semantic versioning string for the song data format.
@@ -346,13 +361,13 @@ class SongMusicData
     this.variation = variation == null ? Constants.DEFAULT_VARIATION : variation;
   }
 
-  public function clone(?newVariation:String = null):SongMusicData
+  public function clone():SongMusicData
   {
-    var result:SongMusicData = new SongMusicData(this.songName, this.artist, newVariation == null ? this.variation : newVariation);
+    var result:SongMusicData = new SongMusicData(this.songName, this.artist, this.variation);
     result.version = this.version;
     result.timeFormat = this.timeFormat;
     result.divisions = this.divisions;
-    result.timeChanges = this.timeChanges;
+    result.timeChanges = this.timeChanges.clone();
     result.looped = this.looped;
     result.generatedBy = this.generatedBy;
 
@@ -368,7 +383,7 @@ class SongMusicData
   }
 }
 
-class SongPlayData
+class SongPlayData implements ICloneable<SongPlayData>
 {
   /**
    * The variations this song has. The associated metadata files should exist.
@@ -417,6 +432,20 @@ class SongPlayData
     ratings = new Map<String, Int>();
   }
 
+  public function clone():SongPlayData
+  {
+    var result:SongPlayData = new SongPlayData();
+    result.songVariations = this.songVariations.clone();
+    result.difficulties = this.difficulties.clone();
+    result.characters = this.characters.clone();
+    result.stage = this.stage;
+    result.noteStyle = this.noteStyle;
+    result.ratings = this.ratings.clone();
+    result.album = this.album;
+
+    return result;
+  }
+
   /**
    * Produces a string representation suitable for debugging.
    */
@@ -430,7 +459,7 @@ class SongPlayData
  * Information about the characters used in this variation of the song.
  * Create a new variation if you want to change the characters.
  */
-class SongCharacterData
+class SongCharacterData implements ICloneable<SongCharacterData>
 {
   @:optional
   @:default('')
@@ -460,6 +489,14 @@ class SongCharacterData
     this.instrumental = instrumental;
   }
 
+  public function clone():SongCharacterData
+  {
+    var result:SongCharacterData = new SongCharacterData(this.player, this.girlfriend, this.opponent, this.instrumental);
+    result.altInstrumentals = this.altInstrumentals.clone();
+
+    return result;
+  }
+
   /**
    * Produces a string representation suitable for debugging.
    */
@@ -469,7 +506,7 @@ class SongCharacterData
   }
 }
 
-class SongChartData
+class SongChartData implements ICloneable<SongChartData>
 {
   @:default(funkin.data.song.SongRegistry.SONG_CHART_DATA_VERSION)
   @:jcustomparse(funkin.data.DataParse.semverVersion)
@@ -539,6 +576,24 @@ class SongChartData
     return writer.write(this, pretty ? '  ' : null);
   }
 
+  public function clone():SongChartData
+  {
+    // We have to manually perform the deep clone here because Map.deepClone() doesn't work.
+    var noteDataClone:Map<String, Array<SongNoteData>> = new Map<String, Array<SongNoteData>>();
+    for (key in this.notes.keys())
+    {
+      noteDataClone.set(key, this.notes.get(key).deepClone());
+    }
+    var eventDataClone:Array<SongEventData> = this.events.deepClone();
+
+    var result:SongChartData = new SongChartData(this.scrollSpeed.clone(), eventDataClone, noteDataClone);
+    result.version = this.version;
+    result.generatedBy = this.generatedBy;
+    result.variation = this.variation;
+
+    return result;
+  }
+
   /**
    * Produces a string representation suitable for debugging.
    */
@@ -548,7 +603,7 @@ class SongChartData
   }
 }
 
-class SongEventDataRaw
+class SongEventDataRaw implements ICloneable<SongEventDataRaw>
 {
   /**
    * The timestamp of the event. The timestamp is in the format of the song's time format.
@@ -604,12 +659,17 @@ class SongEventDataRaw
 
     return _stepTime = Conductor.getTimeInSteps(this.time);
   }
+
+  public function clone():SongEventDataRaw
+  {
+    return new SongEventDataRaw(this.time, this.event, this.value);
+  }
 }
 
 /**
  * Wrap SongEventData in an abstract so we can overload operators.
  */
-@:forward(time, event, value, activated, getStepTime)
+@:forward(time, event, value, activated, getStepTime, clone)
 abstract SongEventData(SongEventDataRaw) from SongEventDataRaw to SongEventDataRaw
 {
   public function new(time:Float, event:String, value:Dynamic = null)
@@ -662,11 +722,6 @@ abstract SongEventData(SongEventDataRaw) from SongEventDataRaw to SongEventDataR
     return this.value == null ? null : cast Reflect.field(this.value, key);
   }
 
-  public function clone():SongEventData
-  {
-    return new SongEventData(this.time, this.event, this.value);
-  }
-
   @:op(A == B)
   public function op_equals(other:SongEventData):Bool
   {
@@ -712,7 +767,7 @@ abstract SongEventData(SongEventDataRaw) from SongEventDataRaw to SongEventDataR
   }
 }
 
-class SongNoteDataRaw
+class SongNoteDataRaw implements ICloneable<SongNoteDataRaw>
 {
   /**
    * The timestamp of the note. The timestamp is in the format of the song's time format.
@@ -828,6 +883,11 @@ class SongNoteDataRaw
     }
     _stepLength = null;
   }
+
+  public function clone():SongNoteDataRaw
+  {
+    return new SongNoteDataRaw(this.time, this.data, this.length, this.kind);
+  }
 }
 
 /**

source/funkin/ui/debug/charting/ChartEditorState.hx

@@ -860,6 +860,70 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState
     return Save.get().chartEditorHasBackup = value;
   }
 
+  /**
+   * A list of previous working file paths.
+   * Also known as the "recent files" list.
+   * The first element is [null] if the current working file has not been saved anywhere yet.
+   */
+  public var previousWorkingFilePaths(default, set):Array<Null<String>> = [null];
+
+  function set_previousWorkingFilePaths(value:Array<Null<String>>):Array<Null<String>>
+  {
+    // Called only when the WHOLE LIST is overridden.
+    previousWorkingFilePaths = value;
+    applyWindowTitle();
+    populateOpenRecentMenu();
+    applyCanQuickSave();
+    return value;
+  }
+
+  /**
+   * The current file path which the chart editor is working with.
+   * If `null`, the current chart has not been saved yet.
+   */
+  public var currentWorkingFilePath(get, set):Null<String>;
+
+  function get_currentWorkingFilePath():Null<String>
+  {
+    return previousWorkingFilePaths[0];
+  }
+
+  function set_currentWorkingFilePath(value:Null<String>):Null<String>
+  {
+    if (value == previousWorkingFilePaths[0]) return value;
+
+    if (previousWorkingFilePaths.contains(null))
+    {
+      // Filter all instances of `null` from the array.
+      previousWorkingFilePaths = previousWorkingFilePaths.filter(function(x:Null<String>):Bool {
+        return x != null;
+      });
+    }
+
+    if (previousWorkingFilePaths.contains(value))
+    {
+      // Move the path to the front of the list.
+      previousWorkingFilePaths.remove(value);
+      previousWorkingFilePaths.unshift(value);
+    }
+    else
+    {
+      // Add the path to the front of the list.
+      previousWorkingFilePaths.unshift(value);
+    }
+
+    while (previousWorkingFilePaths.length > Constants.MAX_PREVIOUS_WORKING_FILES)
+    {
+      // Remove the last path in the list.
+      previousWorkingFilePaths.pop();
+    }
+
+    populateOpenRecentMenu();
+    applyWindowTitle();
+
+    return value;
+  }
+
   /**
    * Whether the difficulty tree view in the toolbox has been modified and needs to be updated.
    * This happens when we add/remove difficulties.
@@ -889,6 +953,12 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState
    */
   var commandHistoryDirty:Bool = true;
 
+  /**
+   * If true, we are currently in the process of quitting the chart editor.
+   * Skip any update functions as most of them will call a crash.
+   */
+  var criticalFailure:Bool = false;
+
   // Input
 
   /**
@@ -1717,70 +1787,6 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState
    */
   var params:Null<ChartEditorParams>;
 
-  /**
-   * A list of previous working file paths.
-   * Also known as the "recent files" list.
-   * The first element is [null] if the current working file has not been saved anywhere yet.
-   */
-  public var previousWorkingFilePaths(default, set):Array<Null<String>> = [null];
-
-  function set_previousWorkingFilePaths(value:Array<Null<String>>):Array<Null<String>>
-  {
-    // Called only when the WHOLE LIST is overridden.
-    previousWorkingFilePaths = value;
-    applyWindowTitle();
-    populateOpenRecentMenu();
-    applyCanQuickSave();
-    return value;
-  }
-
-  /**
-   * The current file path which the chart editor is working with.
-   * If `null`, the current chart has not been saved yet.
-   */
-  public var currentWorkingFilePath(get, set):Null<String>;
-
-  function get_currentWorkingFilePath():Null<String>
-  {
-    return previousWorkingFilePaths[0];
-  }
-
-  function set_currentWorkingFilePath(value:Null<String>):Null<String>
-  {
-    if (value == previousWorkingFilePaths[0]) return value;
-
-    if (previousWorkingFilePaths.contains(null))
-    {
-      // Filter all instances of `null` from the array.
-      previousWorkingFilePaths = previousWorkingFilePaths.filter(function(x:Null<String>):Bool {
-        return x != null;
-      });
-    }
-
-    if (previousWorkingFilePaths.contains(value))
-    {
-      // Move the path to the front of the list.
-      previousWorkingFilePaths.remove(value);
-      previousWorkingFilePaths.unshift(value);
-    }
-    else
-    {
-      // Add the path to the front of the list.
-      previousWorkingFilePaths.unshift(value);
-    }
-
-    while (previousWorkingFilePaths.length > Constants.MAX_PREVIOUS_WORKING_FILES)
-    {
-      // Remove the last path in the list.
-      previousWorkingFilePaths.pop();
-    }
-
-    populateOpenRecentMenu();
-    applyWindowTitle();
-
-    return value;
-  }
-
   public function new(?params:ChartEditorParams)
   {
     super();
@@ -2732,7 +2738,7 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState
   public override function update(elapsed:Float):Void
   {
     // Override F4 behavior to include the autosave.
-    if (FlxG.keys.justPressed.F4)
+    if (FlxG.keys.justPressed.F4 && !criticalFailure)
     {
       quitChartEditor();
       return;
@@ -2741,6 +2747,8 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState
     // dispatchEvent gets called here.
     super.update(elapsed);
 
+    if (criticalFailure) return;
+
     // These ones happen even if the modal dialog is open.
     handleMusicPlayback(elapsed);
     handleNoteDisplay();
@@ -4516,6 +4524,8 @@ class ChartEditorState extends UIState // UIState derives from MusicBeatState
     FlxG.switchState(new MainMenuState());
 
     resetWindowTitle();
+
+    criticalFailure = true;
   }
 
   /**

source/funkin/ui/debug/charting/commands/ChangeStartingBPMCommand.hx

@@ -34,6 +34,11 @@ class ChangeStartingBPMCommand implements ChartEditorCommand
 
     state.currentSongMetadata.timeChanges = timeChanges;
 
+    state.noteDisplayDirty = true;
+    state.notePreviewDirty = true;
+    state.notePreviewViewportBoundsDirty = true;
+    state.scrollPositionInPixels = 0;
+
     Conductor.mapTimeChanges(state.currentSongMetadata.timeChanges);
   }
 
@@ -51,6 +56,11 @@ class ChangeStartingBPMCommand implements ChartEditorCommand
 
     state.currentSongMetadata.timeChanges = timeChanges;
 
+    state.noteDisplayDirty = true;
+    state.notePreviewDirty = true;
+    state.notePreviewViewportBoundsDirty = true;
+    state.scrollPositionInPixels = 0;
+
     Conductor.mapTimeChanges(state.currentSongMetadata.timeChanges);
   }
 

source/funkin/ui/debug/charting/commands/MoveItemsCommand.hx

@@ -21,8 +21,8 @@ class MoveItemsCommand implements ChartEditorCommand
   public function new(notes:Array<SongNoteData>, events:Array<SongEventData>, offset:Float, columns:Int)
   {
     // Clone the notes to prevent editing from affecting the history.
-    this.notes = [for (note in notes) note.clone()];
-    this.events = [for (event in events) event.clone()];
+    this.notes = notes.clone();
+    this.events = events.clone();
     this.offset = offset;
     this.columns = columns;
     this.movedNotes = [];

source/funkin/ui/debug/charting/handlers/ChartEditorImportExportHandler.hx

@@ -43,7 +43,8 @@ class ChartEditorImportExportHandler
       var variation = (metadata.variation == null || metadata.variation == '') ? Constants.DEFAULT_VARIATION : metadata.variation;
 
       // Clone to prevent modifying the original.
-      var metadataClone:SongMetadata = metadata.clone(variation);
+      var metadataClone:SongMetadata = metadata.clone();
+      metadataClone.variation = variation;
       if (metadataClone != null) songMetadata.set(variation, metadataClone);
 
       var chartData:Null<SongChartData> = SongRegistry.instance.parseEntryChartData(songId, metadata.variation);

source/funkin/util/tools/ArrayTools.hx

@@ -76,4 +76,72 @@ class ArrayTools
     while (array.length > 0)
       array.pop();
   }
+
+  /**
+   * Create a new array with all elements of the given array, to prevent modifying the original.
+   */
+  public static function clone<T>(array:Array<T>):Array<T>
+  {
+    return [for (element in array) element];
+  }
+
+  /**
+   * Create a new array with clones of all elements of the given array, to prevent modifying the original.
+   */
+  public static function deepClone<T, U:ICloneable<T>>(array:Array<U>):Array<T>
+  {
+    return [for (element in array) element.clone()];
+  }
+
+  /**
+   * Return true only if both arrays contain the same elements (possibly in a different order).
+   * @param a The first array to compare.
+   * @param b The second array to compare.
+   * @return Weather both arrays contain the same elements.
+   */
+  public static function isEqualUnordered<T>(a:Array<T>, b:Array<T>):Bool
+  {
+    if (a.length != b.length) return false;
+    for (element in a)
+    {
+      if (!b.contains(element)) return false;
+    }
+    for (element in b)
+    {
+      if (!a.contains(element)) return false;
+    }
+    return true;
+  }
+
+  /**
+   * Returns true if `superset` contains all elements of `subset`.
+   * @param superset The array to query for each element.
+   * @param subset The array containing the elements to query for.
+   * @return Weather `superset` contains all elements of `subset`.
+   */
+  public static function isSuperset<T>(superset:Array<T>, subset:Array<T>):Bool
+  {
+    // Shortcuts.
+    if (subset.length == 0) return true;
+    if (subset.length > superset.length) return false;
+
+    // Check each element.
+    for (element in subset)
+    {
+      if (!superset.contains(element)) return false;
+    }
+    return true;
+  }
+
+  /**
+   * Returns true if `superset` contains all elements of `subset`.
+   * @param subset The array containing the elements to query for.
+   * @param superset The array to query for each element.
+   * @return Weather `superset` contains all elements of `subset`.
+   */
+  public static function isSubset<T>(subset:Array<T>, superset:Array<T>):Bool
+  {
+    // Switch it around.
+    return isSuperset(superset, subset);
+  }
 }

source/funkin/util/tools/ICloneable.hx

@@ -0,0 +1,10 @@
+package funkin.util.tools;
+
+/**
+ * Implement this on a class to enable `Array<T>.deepClone()` to work on it.
+ * NOTE: T should be the type of the class that implements this interface.
+ */
+interface ICloneable<T>
+{
+  public function clone():T;
+}

source/funkin/util/tools/MapTools.hx

@@ -25,6 +25,33 @@ class MapTools
     return [for (i in map.iterator()) i];
   }
 
+  /**
+   * Create a new array with all elements of the given array, to prevent modifying the original.
+   */
+  public static function clone<K, T>(map:Map<K, T>):Map<K, T>
+  {
+    return map.copy();
+  }
+
+  /**
+   * Create a new array with clones of all elements of the given array, to prevent modifying the original.
+   */
+  public static function deepClone<K, T, U:ICloneable<T>>(map:Map<K, U>):Map<K, T>
+  {
+    // TODO: This function does NOT work.
+    throw "Not implemented";
+
+    /*
+      var newMap:Map<K, T> = [];
+      // Replace each value with a clone of itself.
+      for (key in newMap.keys())
+      {
+        newMap.set(key, newMap.get(key).clone());
+      }
+      return newMap;
+     */
+  }
+
   /**
    * Return a list of keys from the map (as an array, rather than an iterator).
    * TODO: Rename this?