More note changes I guess

src/router/custom_song/chart.rs

@@ -3,15 +3,40 @@ use jzon::{object, JsonValue};
 // Transcodes a SIF1/NPPS4 beatmap (array of {timing_sec, effect, effect_value, position})
 // into the SIF2 chart JSON the client deserializes into NoteData.
 //
+// SIF1 note effects, from the game's own LiveModel.NoteEffect
+// (m_live/model/note_effect.lua): note_normal 1, note_event 2, note_hold 3, note_bomb_1 4,
+// note_bomb_3 5, note_bomb_5 6, note_bomb_9 7, note_slide 11, note_slide_event 12,
+// note_slide_hold 13, with isHold(e) = e == 3 and isSlide(e) = e >= 11.
+//
+// SIF2 note types, from Aoharu.LiveTimeController.ToMarkerType: 1 tap, 2 flick, 3 skill.
+// Anything outside 1..3 becomes MarkerType.None, so those three are the whole vocabulary.
+//
 // Mapping rules:
 // - line = position - 1 (both are right-to-left)
-// - effect 1 (and 2, the "parallel" marker) -> type 1 (tap)
+// - effect 1 (note_normal) and 2 (note_event) -> type 1 (tap). note_event is an ordinary
+//   note that also fed SIF1's event scoring; it plays identically and SIF2 has no
+//   equivalent, so the distinction is dropped. Simultaneous hits are expressed by sharing
+//   a spawn num, not by the effect.
 // - effect 3 (hold) -> head note (type 1) at timing_sec plus a SYNTHESIZED tail note
 //   (type 1, same line) at timing_sec + effect_value, linked through parent/child ids
-// - effect 4 (star/token) -> type 3
-// - effect 11/12/13 (swing) and anything unknown -> plain type 1 tap for v1.
-//   Slider chains are a later feature.
-// - notes_attribute / notes_level are dropped (SIF2 has no per-note attribute)
+// - effect 4/5/6/7 (bomb_1/3/5/9, SIF1's star notes) -> type 3, SIF2's bomb note. SIF1
+//   varies the blast width per effect; SIF2 has one bomb with one damage value, so the
+//   four collapse into type 3 and only the radius is lost. Previously only bomb_1 mapped
+//   here and the three wider ones arrived as ordinary taps.
+// - effect 11/12 (slide) -> type 2, SIF2's flick. These are SIF1's swipe notes; sending
+//   them as taps made a swipe chart playable as a tap chart.
+// - effect 13 (slide hold) -> a flick HEAD (type 2) plus the same synthesized tail as
+//   effect 3, tail as type 1. SIF1 wants the swipe on entry and a plain release, and SIF2
+//   agrees on both counts: LiveInputControl judges a Flick root through InputType.Flick
+//   (so the head demands a swipe) while InputType.Released rejects a Flick outright
+//   (so a flick tail could never be released). Previously effect 13 lost BOTH halves —
+//   no swipe and no hold, just a lone tap.
+// - effect 0 (random) and anything else unknown -> plain type 1 tap. Every effect the game
+//   actually defines is covered above, so this is only a floor for hand-authored charts.
+// - notes_attribute is dropped (SIF2 has no per-note attribute). notes_level is dropped
+//   too: in SIF1 a notes_level > 1 marks a simultaneous-hit group (notes.lua groups on it
+//   regardless of effect, so it is NOT the slide chain). SIF2's force_sync_group_id is the
+//   equivalent and is left at 0 for now.
 // - ids are sequential from 1 in time order. num is the spawn group: the dummy
 //   header occupies 100, real groups count up from 101, and notes that hit
 //   simultaneously (equal timing_sec, which covers SIF1 effect 2 pairs) share one num.
@@ -28,6 +53,26 @@ struct WorkNote {
     head: Option<usize>
 }
 
+// LiveModel.NoteEffect.isHold, widened to note_slide_hold: both carry a duration in
+// effect_value (notes.lua isTimeOver adds effect_value for note_hold and note_slide_hold alike).
+fn is_hold(effect: i64) -> bool {
+    effect == 3 || effect == 13
+}
+
+// LiveModel.NoteEffect.isSlide
+fn is_slide(effect: i64) -> bool {
+    effect >= 11
+}
+
+// SIF1's star notes: note_bomb_1/3/5/9. The suffix is the blast width — star_icon.lua maps
+// them to 0/1/2/4 extra lanes either side, damaging that spread when the note is missed.
+// SIF2 has a single bomb note with one damage value (LiveInputResultMst._bombLifeDamage,
+// applied by LiveLifeControl.CheckLife for type 3, and drawn with the star mark by
+// MarkerUI), so all four map to type 3 and only the radius is lost.
+fn is_bomb(effect: i64) -> bool {
+    (4..=7).contains(&effect)
+}
+
 fn parse_sif_note(data: &JsonValue, index: usize) -> Result<(f64, i64, f64, i64), String> {
     let timing = data["timing_sec"].as_f64().ok_or(format!("Note {}: missing timing_sec", index))?;
     let effect = data["effect"].as_i64().ok_or(format!("Note {}: missing effect", index))?;
@@ -40,7 +85,7 @@ fn parse_sif_note(data: &JsonValue, index: usize) -> Result<(f64, i64, f64, i64)
     if timing < 0.0 {
         return Err(format!("Note {}: negative timing_sec {}", index, timing));
     }
-    if effect == 3 && effect_value <= 0.0 {
+    if is_hold(effect) && effect_value <= 0.0 {
         return Err(format!("Note {}: hold with effect_value {} (must be > 0)", index, effect_value));
     }
 
@@ -67,10 +112,12 @@ pub fn transcode(beatmap: &JsonValue) -> Result<(JsonValue, i64), String> {
         work.push(WorkNote {
             time: timing,
             line: position - 1,
-            kind: if effect == 4 { 3 } else { 1 },
+            kind: if is_slide(effect) { 2 } else if is_bomb(effect) { 3 } else { 1 },
             head: None
         });
-        if effect == 3 {
+        if is_hold(effect) {
+            // Tail is always a tap: a flick tail is unreleasable (InputType.Released rejects
+            // MarkerType.Flick), so the swipe stays on the head where SIF1 puts it.
             work.push(WorkNote {
                 time: timing + effect_value,
                 line: position - 1,
@@ -233,7 +280,7 @@ mod tests {
             sif_note(2.0, 3, 3, 1.5),  // hold: head at 2.0, tail at 3.5
             sif_note(2.5, 7, 4, 0.0),  // star
             sif_note(3.5, 1, 2, 2.0),  // parallel with the hold tail
-            sif_note(4.0, 9, 11, 0.0)  // swing -> plain tap for v1
+            sif_note(4.0, 9, 11, 0.0)  // slide -> flick
         ];
         let (chart, combo) = transcode(&beatmap).unwrap();
 
@@ -246,13 +293,132 @@ mod tests {
         assert_eq!(chart["notes"][4]["parent_id"], 2);
         // The tail and the parallel tap at 3.5 share a spawn group
         assert_eq!(chart["notes"][4]["num"], chart["notes"][5]["num"].clone());
-        assert_eq!(chart["notes"][6]["type"], 1);
+        assert_eq!(chart["notes"][6]["type"], 2);
         // Ids stay sequential in time order
         for (i, data) in chart["notes"].members().enumerate() {
             assert_eq!(data["id"], i);
         }
     }
 
+    #[test]
+    fn slides_become_flicks() {
+        // note_slide and note_slide_event are both plain swipes
+        let beatmap = jzon::array![
+            sif_note(1.0, 1, 11, 0.0),
+            sif_note(2.0, 9, 12, 0.0)
+        ];
+        let (chart, combo) = transcode(&beatmap).unwrap();
+
+        assert_eq!(combo, 2);
+        assert_eq!(chart["notes"].len(), 3);
+        assert_eq!(chart["notes"][1]["type"], 2);
+        assert_eq!(chart["notes"][2]["type"], 2);
+        // A plain slide is not a hold, so neither gets a tail
+        assert_eq!(chart["notes"][1]["child_id"], 0);
+        assert_eq!(chart["notes"][2]["child_id"], 0);
+    }
+
+    #[test]
+    fn slide_hold_keeps_swipe_and_hold() {
+        // Regression: effect 13 used to lose both halves and arrive as one plain tap.
+        let beatmap = jzon::array![
+            sif_note(1.0, 3, 13, 2.5)
+        ];
+        let (chart, combo) = transcode(&beatmap).unwrap();
+
+        assert_eq!(combo, 1);
+        assert_eq!(chart["notes"].len(), 3);
+        let head = &chart["notes"][1];
+        let tail = &chart["notes"][2];
+        // Swipe on entry, plain release: flick head, tap tail
+        assert_eq!(head["type"], 2);
+        assert_eq!(tail["type"], 1);
+        // ... linked as a hold, exactly like effect 3
+        assert_eq!(head["child_id"], 2);
+        assert_eq!(head["child_line"], 2);
+        assert_eq!(tail["parent_id"], 1);
+        assert_eq!(tail["time"].as_f64().unwrap(), 3.5);
+    }
+
+    #[test]
+    fn slide_hold_needs_a_duration() {
+        // The effect 3 duration check has to cover note_slide_hold too
+        assert!(transcode(&jzon::array![sif_note(1.0, 5, 13, 0.0)]).is_err());
+        assert!(transcode(&jzon::array![sif_note(1.0, 5, 13, -1.0)]).is_err());
+    }
+
+    #[test]
+    fn every_bomb_width_is_a_star_note() {
+        // note_bomb_1/3/5/9. SIF2 has one bomb note, so all four land on type 3; before,
+        // only bomb_1 did and the wider three arrived as ordinary taps.
+        for effect in [4, 5, 6, 7] {
+            let (chart, combo) = transcode(&jzon::array![sif_note(1.0, 5, effect, 0.0)]).unwrap();
+            assert_eq!(chart["notes"][1]["type"], 3, "effect {}", effect);
+            // Bombs are instantaneous — no tail, and they count for combo
+            assert_eq!(chart["notes"].len(), 2, "effect {}", effect);
+            assert_eq!(chart["notes"][1]["child_id"], 0, "effect {}", effect);
+            assert_eq!(combo, 1, "effect {}", effect);
+        }
+    }
+
+    #[test]
+    fn every_defined_effect_maps_to_a_real_note_type() {
+        // The whole LiveModel.NoteEffect vocabulary, and what each must become
+        for (effect, kind) in [(1, 1), (2, 1), (3, 1), (4, 3), (5, 3), (6, 3), (7, 3), (11, 2), (12, 2), (13, 2)] {
+            let (chart, _) = transcode(&jzon::array![sif_note(1.0, 5, effect, 1.0)]).unwrap();
+            assert_eq!(chart["notes"][1]["type"], kind, "effect {}", effect);
+            // Whatever it is, the client must be able to resolve it
+            for data in chart["notes"].members().skip(1) {
+                let t = data["type"].as_i64().unwrap();
+                assert!((1..=3).contains(&t), "effect {} produced unresolvable type {}", effect, t);
+            }
+        }
+    }
+
+    #[test]
+    fn undefined_effects_fall_back_to_taps() {
+        // Not part of NoteEffect; a hand-authored chart must still transcode to something valid
+        for effect in [0, 8, 9, 10] {
+            let (chart, _) = transcode(&jzon::array![sif_note(1.0, 5, effect, 0.0)]).unwrap();
+            assert_eq!(chart["notes"][1]["type"], 1, "effect {}", effect);
+        }
+    }
+
+    // Lifted verbatim from a chart uploaded to the live server (custom song 10008, "Edelied",
+    // exported via /custom_song/download/10008) — a swipe run of note_slide into note_slide_hold,
+    // which is the shape that used to arrive as undifferentiated taps. Note the editor writes a
+    // large arbitrary notes_level (38615 here) rather than SIF1's small group index, which is why
+    // notes_level is not used as a sync group.
+    #[test]
+    fn real_uploaded_swipe_run() {
+        let beatmap = jzon::array![
+            object!{ "timing_sec": 22.0, "effect": 11, "effect_value": 2.0, "notes_attribute": 2, "notes_level": 38615, "position": 9 },
+            object!{ "timing_sec": 22.166666666666668, "effect": 11, "effect_value": 2.0, "notes_attribute": 2, "notes_level": 38615, "position": 8 },
+            object!{ "timing_sec": 22.5, "effect": 13, "effect_value": 0.33333333333333215, "notes_attribute": 2, "notes_level": 38615, "position": 6 }
+        ];
+        let (chart, combo) = transcode(&beatmap).unwrap();
+
+        // 3 source notes + the slide-hold's tail; the same-lane head does not count for combo
+        assert_eq!(chart["notes"].len(), 5);
+        assert_eq!(combo, 3);
+        // Both plain slides are flicks with no tail
+        assert_eq!(chart["notes"][1]["type"], 2);
+        assert_eq!(chart["notes"][1]["line"], 8);
+        assert_eq!(chart["notes"][1]["child_id"], 0);
+        assert_eq!(chart["notes"][2]["type"], 2);
+        assert_eq!(chart["notes"][2]["line"], 7);
+        // The slide-hold is a flick head linked to a tap tail
+        assert_eq!(chart["notes"][3]["type"], 2);
+        assert_eq!(chart["notes"][3]["child_id"], 4);
+        assert_eq!(chart["notes"][4]["type"], 1);
+        assert_eq!(chart["notes"][4]["parent_id"], 3);
+        // Every type must be one ToMarkerType resolves; 0 would silently become MarkerType.None
+        for data in chart["notes"].members().skip(1) {
+            let kind = data["type"].as_i64().unwrap();
+            assert!((1..=3).contains(&kind), "unresolvable type {}", kind);
+        }
+    }
+
     #[test]
     fn rejects_bad_charts() {
         assert!(transcode(&jzon::array![sif_note(1.0, 0, 1, 2.0)]).is_err());