Merge pull request #195 from FunkinCrew/rewrite/bugfix/chart-editor-various-fixes

Fixes for a few bugs in the chart editor.

.gitmodules

@@ -1,6 +1,7 @@
 [submodule "assets"]
 	path = assets
 	url = https://github.com/FunkinCrew/Funkin-history-rewrite-assets
+	branch = master
 [submodule "art"]
 	path = art
 	url = https://github.com/FunkinCrew/Funkin-history-rewrite-art

assets

@@ -1 +1 @@
-Subproject commit f66c8559db05df42a8503abc7b59166c35ba0b2e
+Subproject commit 777410e4ad628f274065b0245f73a4985ffbf28a

source/funkin/play/HealthIcon.hx

@@ -24,13 +24,14 @@ import openfl.utils.Assets;
  *     - i.e. `PlayState.instance.iconP1.animation.addByPrefix("jumpscare", "jumpscare", 24, false);`
  * @author MasterEric
  */
+@:nullSafety
 class HealthIcon extends FlxSprite
 {
   /**
    * The character this icon is representing.
    * Setting this variable will automatically update the graphic.
    */
-  public var characterId(default, set):String;
+  public var characterId(default, set):Null<String>;
 
   /**
    * Whether this health icon should automatically update its state based on the character's health.
@@ -123,13 +124,13 @@ class HealthIcon extends FlxSprite
     initTargetSize();
   }
 
-  function set_characterId(value:String):String
+  function set_characterId(value:Null<String>):Null<String>
   {
     if (value == characterId) return value;
 
-    characterId = value;
+    characterId = value ?? Constants.DEFAULT_HEALTH_ICON;
     loadCharacter(characterId);
-    return value;
+    return characterId;
   }
 
   function set_isPixel(value:Bool):Bool
@@ -138,7 +139,7 @@ class HealthIcon extends FlxSprite
 
     isPixel = value;
     loadCharacter(characterId);
-    return value;
+    return isPixel;
   }
 
   /**
@@ -156,6 +157,32 @@ class HealthIcon extends FlxSprite
     }
   }
 
+  /**
+   * Use the provided CharacterHealthIconData to configure this health icon's appearance.
+   * @param data The data to use to configure this health icon.
+   */
+  public function configure(data:Null<HealthIconData>):Void
+  {
+    if (data == null)
+    {
+      this.isPixel = false;
+      this.characterId = Constants.DEFAULT_HEALTH_ICON;
+      this.size.set(1.0, 1.0);
+      this.offset.x = 0.0;
+      this.offset.y = 0.0;
+      this.flipX = false;
+    }
+    else
+    {
+      this.isPixel = data.isPixel ?? false;
+      this.characterId = data.id;
+      this.size.set(data.scale ?? 1.0, data.scale ?? 1.0);
+      this.offset.x = (data.offsets != null) ? data.offsets[0] : 0.0;
+      this.offset.y = (data.offsets != null) ? data.offsets[1] : 0.0;
+      this.flipX = data.flipX ?? false; // Face the OTHER way by default, since that is more common.
+    }
+  }
+
   /**
    * Called by Flixel every frame. Includes logic to manage the currently playing animation.
    */
@@ -341,12 +368,17 @@ class HealthIcon extends FlxSprite
     this.animation.add(Losing, [1], 0, false, false);
   }
 
-  function correctCharacterId(charId:String):String
+  function correctCharacterId(charId:Null<String>):String
   {
+    if (charId == null)
+    {
+      return Constants.DEFAULT_HEALTH_ICON;
+    }
+
     if (!Assets.exists(Paths.image('icons/icon-$charId')))
     {
       FlxG.log.warn('No icon for character: $charId : using default placeholder face instead!');
-      return 'face';
+      return Constants.DEFAULT_HEALTH_ICON;
     }
 
     return charId;
@@ -357,10 +389,11 @@ class HealthIcon extends FlxSprite
     return Assets.exists(Paths.file('images/icons/icon-$characterId.xml'));
   }
 
-  function loadCharacter(charId:String):Void
+  function loadCharacter(charId:Null<String>):Void
   {
-    if (correctCharacterId(charId) != charId)
+    if (charId == null || correctCharacterId(charId) != charId)
     {
+      // This will recursively trigger loadCharacter to be called again.
       characterId = correctCharacterId(charId);
       return;
     }

source/funkin/play/character/BaseCharacter.hx

@@ -317,12 +317,8 @@ class BaseCharacter extends Bopper
         trace('[WARN] Player 1 health icon not found!');
         return;
       }
-      PlayState.instance.iconP1.isPixel = _data.healthIcon?.isPixel ?? false;
+      PlayState.instance.iconP1.configure(_data.healthIcon);
-      PlayState.instance.iconP1.characterId = _data.healthIcon.id;
+      PlayState.instance.iconP1.flipX = !PlayState.instance.iconP1.flipX; // BF is looking the other way.
-      PlayState.instance.iconP1.size.set(_data.healthIcon.scale, _data.healthIcon.scale);
-      PlayState.instance.iconP1.offset.x = _data.healthIcon.offsets[0];
-      PlayState.instance.iconP1.offset.y = _data.healthIcon.offsets[1];
-      PlayState.instance.iconP1.flipX = !_data.healthIcon.flipX;
     }
     else
     {
@@ -331,12 +327,7 @@ class BaseCharacter extends Bopper
         trace('[WARN] Player 2 health icon not found!');
         return;
       }
-      PlayState.instance.iconP2.isPixel = _data.healthIcon?.isPixel ?? false;
+      PlayState.instance.iconP2.configure(_data.healthIcon);
-      PlayState.instance.iconP2.characterId = _data.healthIcon.id;
-      PlayState.instance.iconP2.size.set(_data.healthIcon.scale, _data.healthIcon.scale);
-      PlayState.instance.iconP2.offset.x = _data.healthIcon.offsets[0];
-      PlayState.instance.iconP2.offset.y = _data.healthIcon.offsets[1];
-      PlayState.instance.iconP2.flipX = _data.healthIcon.flipX;
     }
   }
 

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

@@ -87,9 +87,8 @@ using Lambda;
  *
  * @author MasterEric
  */
+@:nullSafety
 // Give other classes access to private instance fields
-// @:nullSafety(Loose) // Enable this while developing, then disable to keep unit tests functional!
-
 @:allow(funkin.ui.debug.charting.ChartEditorCommand)
 @:allow(funkin.ui.debug.charting.ChartEditorDropdowns)
 @:allow(funkin.ui.debug.charting.ChartEditorDialogHandler)
@@ -965,7 +964,7 @@ class ChartEditorState extends HaxeUIState
 
   function get_currentSongChartNoteData():Array<SongNoteData>
   {
-    var result:Array<SongNoteData> = currentSongChartData.notes.get(selectedDifficulty);
+    var result:Null<Array<SongNoteData>> = currentSongChartData.notes.get(selectedDifficulty);
     if (result == null)
     {
       // Initialize to the default value if not set.
@@ -1391,16 +1390,12 @@ class ChartEditorState extends HaxeUIState
     healthIconDad = new HealthIcon(currentSongMetadata.playData.characters.opponent);
     healthIconDad.autoUpdate = false;
     healthIconDad.size.set(0.5, 0.5);
-    healthIconDad.x = gridTiledSprite.x - 15 - (HealthIcon.HEALTH_ICON_SIZE * 0.5);
-    healthIconDad.y = gridTiledSprite.y + 5;
     add(healthIconDad);
     healthIconDad.zIndex = 30;
 
     healthIconBF = new HealthIcon(currentSongMetadata.playData.characters.player);
     healthIconBF.autoUpdate = false;
     healthIconBF.size.set(0.5, 0.5);
-    healthIconBF.x = gridTiledSprite.x + gridTiledSprite.width + 15;
-    healthIconBF.y = gridTiledSprite.y + 5;
     healthIconBF.flipX = true;
     add(healthIconBF);
     healthIconBF.zIndex = 30;
@@ -1627,6 +1622,12 @@ class ChartEditorState extends HaxeUIState
     addUIClickListener('playbarForward', _ -> playbarButtonPressed = 'playbarForward');
     addUIClickListener('playbarEnd', _ -> playbarButtonPressed = 'playbarEnd');
 
+    // Cycle note snap quant.
+    addUIClickListener('playbarNoteSnap', function(_) {
+      noteSnapQuantIndex++;
+      if (noteSnapQuantIndex >= SNAP_QUANTS.length) noteSnapQuantIndex = 0;
+    });
+
     // Add functionality to the menu items.
 
     addUIClickListener('menubarItemNewChart', _ -> ChartEditorDialogHandler.openWelcomeDialog(this, true));
@@ -2494,27 +2495,30 @@ class ChartEditorState extends HaxeUIState
         var dragLengthMs:Float = dragLengthSteps * Conductor.stepLengthMs;
         var dragLengthPixels:Float = dragLengthSteps * GRID_SIZE;
 
-        if (dragLengthSteps > 0)
+        if (gridGhostNote != null && gridGhostNote.noteData != null && gridGhostHoldNote != null)
         {
-          if (dragLengthCurrent != dragLengthSteps)
+          if (dragLengthSteps > 0)
           {
-            stretchySounds = !stretchySounds;
+            if (dragLengthCurrent != dragLengthSteps)
-            ChartEditorAudioHandler.playSound(Paths.sound('chartingSounds/stretch' + (stretchySounds ? '1' : '2') + '_UI'));
+            {
+              stretchySounds = !stretchySounds;
+              ChartEditorAudioHandler.playSound(Paths.sound('chartingSounds/stretch' + (stretchySounds ? '1' : '2') + '_UI'));
 
-            dragLengthCurrent = dragLengthSteps;
+              dragLengthCurrent = dragLengthSteps;
+            }
+
+            gridGhostHoldNote.visible = true;
+            gridGhostHoldNote.noteData = gridGhostNote.noteData;
+            gridGhostHoldNote.noteDirection = gridGhostNote.noteData.getDirection();
+
+            gridGhostHoldNote.setHeightDirectly(dragLengthPixels);
+
+            gridGhostHoldNote.updateHoldNotePosition(renderedHoldNotes);
+          }
+          else
+          {
+            gridGhostHoldNote.visible = false;
           }
-
-          gridGhostHoldNote.visible = true;
-          gridGhostHoldNote.noteData = gridGhostNote.noteData;
-          gridGhostHoldNote.noteDirection = gridGhostNote.noteData.getDirection();
-
-          gridGhostHoldNote.setHeightDirectly(dragLengthPixels);
-
-          gridGhostHoldNote.updateHoldNotePosition(renderedHoldNotes);
-        }
-        else
-        {
-          gridGhostHoldNote.visible = false;
         }
 
         if (FlxG.mouse.justReleased)
@@ -2670,7 +2674,7 @@ class ChartEditorState extends HaxeUIState
           if (cursorColumn == eventColumn)
           {
             if (gridGhostNote != null) gridGhostNote.visible = false;
-            gridGhostHoldNote.visible = false;
+            if (gridGhostHoldNote != null) gridGhostHoldNote.visible = false;
 
             if (gridGhostEvent == null) throw "ERROR: Tried to handle cursor, but gridGhostEvent is null! Check ChartEditorState.buildGrid()";
 
@@ -2730,11 +2734,11 @@ class ChartEditorState extends HaxeUIState
         }
         else
         {
-          if (FlxG.mouse.overlaps(notePreview))
+          if (notePreview != null && FlxG.mouse.overlaps(notePreview))
           {
             targetCursorMode = Pointer;
           }
-          else if (FlxG.mouse.overlaps(gridPlayheadScrollArea))
+          else if (gridPlayheadScrollArea != null && FlxG.mouse.overlaps(gridPlayheadScrollArea))
           {
             targetCursorMode = Pointer;
           }
@@ -3046,18 +3050,35 @@ class ChartEditorState extends HaxeUIState
   {
     if (healthIconsDirty)
     {
-      if (healthIconBF != null) healthIconBF.characterId = currentSongMetadata.playData.characters.player;
+      var charDataBF = CharacterDataParser.fetchCharacterData(currentSongMetadata.playData.characters.player);
-      if (healthIconDad != null) healthIconDad.characterId = currentSongMetadata.playData.characters.opponent;
+      var charDataDad = CharacterDataParser.fetchCharacterData(currentSongMetadata.playData.characters.opponent);
+      if (healthIconBF != null)
+      {
+        healthIconBF.configure(charDataBF?.healthIcon);
+        healthIconBF.size *= 0.5; // Make the icon smaller in Chart Editor.
+        healthIconBF.flipX = !healthIconBF.flipX; // BF faces the other way.
+      }
+      if (healthIconDad != null)
+      {
+        healthIconDad.configure(charDataDad?.healthIcon);
+        healthIconDad.size *= 0.5; // Make the icon smaller in Chart Editor.
+      }
+      healthIconsDirty = false;
     }
 
-    // Right align the BF health icon.
+    // Right align, and visibly center, the BF health icon.
     if (healthIconBF != null)
     {
       // Base X position to the right of the grid.
-      var baseHealthIconXPos:Float = (gridTiledSprite == null) ? (0) : (gridTiledSprite.x + gridTiledSprite.width + 15);
+      healthIconBF.x = (gridTiledSprite == null) ? (0) : (gridTiledSprite.x + gridTiledSprite.width + 45 - (healthIconBF.width / 2));
-      // Will be 0 when not bopping. When bopping, will increase to push the icon left.
+      healthIconBF.y = (gridTiledSprite == null) ? (0) : (MENU_BAR_HEIGHT + GRID_TOP_PAD + 30 - (healthIconBF.height / 2));
-      var healthIconOffset:Float = healthIconBF.width - (HealthIcon.HEALTH_ICON_SIZE * 0.5);
+    }
-      healthIconBF.x = baseHealthIconXPos - healthIconOffset;
+
+    // Visibly center the Dad health icon.
+    if (healthIconDad != null)
+    {
+      healthIconDad.x = (gridTiledSprite == null) ? (0) : (gridTiledSprite.x - 45 - (healthIconDad.width / 2));
+      healthIconDad.y = (gridTiledSprite == null) ? (0) : (MENU_BAR_HEIGHT + GRID_TOP_PAD + 30 - (healthIconDad.height / 2));
     }
   }
 
@@ -3684,49 +3705,41 @@ class ChartEditorState extends HaxeUIState
     var inputStage:Null<DropDown> = toolbox.findComponent('inputStage', DropDown);
     var stageId:String = currentSongMetadata.playData.stage;
     var stageData:Null<StageData> = StageDataParser.parseStageData(stageId);
-    if (stageData != null)
+    if (inputStage != null)
     {
-      inputStage.value = {id: stageId, text: stageData.name};
+      inputStage.value = (stageData != null) ?
-    }
+        {id: stageId, text: stageData.name} :
-    else
+          {id: "mainStage", text: "Main Stage"};
-    {
-      inputStage.value = {id: "mainStage", text: "Main Stage"};
     }
 
     var inputCharacterPlayer:Null<DropDown> = toolbox.findComponent('inputCharacterPlayer', DropDown);
     var charIdPlayer:String = currentSongMetadata.playData.characters.player;
     var charDataPlayer:Null<CharacterData> = CharacterDataParser.fetchCharacterData(charIdPlayer);
-    if (charDataPlayer != null)
+    if (inputCharacterPlayer != null)
     {
-      inputCharacterPlayer.value = {id: charIdPlayer, text: charDataPlayer.name};
+      inputCharacterPlayer.value = (charDataPlayer != null) ?
-    }
+        {id: charIdPlayer, text: charDataPlayer.name} :
-    else
+          {id: "bf", text: "Boyfriend"};
-    {
-      inputCharacterPlayer.value = {id: "bf", text: "Boyfriend"};
     }
 
     var inputCharacterOpponent:Null<DropDown> = toolbox.findComponent('inputCharacterOpponent', DropDown);
     var charIdOpponent:String = currentSongMetadata.playData.characters.opponent;
     var charDataOpponent:Null<CharacterData> = CharacterDataParser.fetchCharacterData(charIdOpponent);
-    if (charDataOpponent != null)
+    if (inputCharacterOpponent != null)
     {
-      inputCharacterOpponent.value = {id: charIdOpponent, text: charDataOpponent.name};
+      inputCharacterOpponent.value = (charDataOpponent != null) ?
-    }
+        {id: charIdOpponent, text: charDataOpponent.name} :
-    else
+          {id: "dad", text: "Dad"};
-    {
-      inputCharacterOpponent.value = {id: "dad", text: "Dad"};
     }
 
     var inputCharacterGirlfriend:Null<DropDown> = toolbox.findComponent('inputCharacterGirlfriend', DropDown);
     var charIdGirlfriend:String = currentSongMetadata.playData.characters.girlfriend;
     var charDataGirlfriend:Null<CharacterData> = CharacterDataParser.fetchCharacterData(charIdGirlfriend);
-    if (charDataGirlfriend != null)
+    if (inputCharacterGirlfriend != null)
     {
-      inputCharacterGirlfriend.value = {id: charIdGirlfriend, text: charDataGirlfriend.name};
+      inputCharacterGirlfriend.value = (charDataGirlfriend != null) ?
-    }
+        {id: charIdGirlfriend, text: charDataGirlfriend.name} :
-    else
+          {id: "none", text: "None"};
-    {
-      inputCharacterGirlfriend.value = {id: "none", text: "None"};
     }
   }
 
@@ -4032,7 +4045,7 @@ class ChartEditorState extends HaxeUIState
     this.scrollPositionInPixels = value;
 
     // Move the grid sprite to the correct position.
-    if (gridTiledSprite != null)
+    if (gridTiledSprite != null && gridPlayheadScrollArea != null)
     {
       if (isViewDownscroll)
       {
@@ -4182,12 +4195,14 @@ class ChartEditorState extends HaxeUIState
   function moveSongToScrollPosition():Void
   {
     // Update the songPosition in the audio tracks.
-    if (audioInstTrack != null) audioInstTrack.time = scrollPositionInMs + playheadPositionInMs;
+    if (audioInstTrack != null)
+    {
+      audioInstTrack.time = scrollPositionInMs + playheadPositionInMs;
+      // Update the songPosition in the Conductor.
+      Conductor.update(audioInstTrack.time);
+    }
     if (audioVocalTrackGroup != null) audioVocalTrackGroup.time = scrollPositionInMs + playheadPositionInMs;
 
-    // Update the songPosition in the Conductor.
-    Conductor.update(audioInstTrack.time);
-
     // We need to update the note sprites because we changed the scroll position.
     noteDisplayDirty = true;
   }

source/funkin/ui/haxeui/components/FunkinClickLabel.hx

@@ -0,0 +1,30 @@
+package funkin.ui.haxeui.components;
+
+import haxe.ui.components.Label;
+import funkin.input.Cursor;
+import haxe.ui.events.MouseEvent;
+
+/**
+ * A HaxeUI label which:
+ * - Changes the current cursor when hovered over (assume an onClick handler will be added!).
+ */
+class FunkinClickLabel extends Label
+{
+  public function new()
+  {
+    super();
+
+    this.onMouseOver = handleMouseOver;
+    this.onMouseOut = handleMouseOut;
+  }
+
+  private function handleMouseOver(event:MouseEvent)
+  {
+    Cursor.cursorMode = Pointer;
+  }
+
+  private function handleMouseOut(event:MouseEvent)
+  {
+    Cursor.cursorMode = Default;
+  }
+}

source/funkin/util/Constants.hx

@@ -131,6 +131,11 @@ class Constants
    */
   public static final DEFAULT_CHARACTER:String = 'bf';
 
+  /**
+   * Default player character for health icons.
+   */
+  public static final DEFAULT_HEALTH_ICON:String = 'face';
+
   /**
    * Default stage for charts.
    */