Fix note count calculation

src/router/custom_song.rs

@@ -2,8 +2,7 @@
 // in-process symphonia + vorbis machinery
 pub mod audio;
 mod chart;
-// One-time startup regroup of charts stored before the spawn-group pairing rule; called from
-// run_server, no-op when the feature is disabled or every chart is already correctly grouped
+// Idempotent startup repairs for stored custom charts and catalog metadata.
 pub mod migrate;
 mod package;
 
@@ -1569,6 +1568,64 @@ mod tests {
         assert_eq!(database::get_revision(), revision + 1);
     }
 
+    #[test]
+    fn startup_migration_recounts_same_lane_checkpoints() {
+        let _lock = crate::runtime::lock_test_data_path();
+        let mut beatmap = array![];
+        for i in 0..13 {
+            beatmap.push(object!{
+                "timing_sec": 0.5 + i as f64 * 0.05, "position": 5,
+                "notes_level": 500, "effect": if i == 12 { 14 } else { 11 },
+                "effect_value": 0.0
+            }).unwrap();
+        }
+        let mut fields = HashMap::new();
+        field(&mut fields, "name", "Repeated Checkpoint Count");
+        field(&mut fields, "artist", "Count Test");
+        field(&mut fields, "attribute", "1");
+        field(&mut fields, "level_number_4", "10");
+        fields.insert(String::from("jacket"), test_png());
+        fields.insert(String::from("audio"), test_ogg_tone(615.0));
+        fields.insert(String::from("chart_4"), jzon::stringify(beatmap).into_bytes());
+        let id = create_song(3333, &fields).unwrap();
+        let path = song_path(id, "chart_4.json");
+        let corrected_bytes = fs::read(&path).unwrap();
+        let corrected_song = database::get_song(id).unwrap();
+        assert_eq!(corrected_song["levels"][0]["full_combo"], 12);
+
+        let mut chart = jzon::parse(&String::from_utf8_lossy(&corrected_bytes)).unwrap();
+        chart["max_combo_count"] = 1.into();
+        let old_bytes = jzon::stringify(chart).into_bytes();
+        fs::write(&path, &old_bytes).unwrap();
+        let mut song = corrected_song.clone();
+        let (md5, size) = asset_meta(&old_bytes);
+        song["levels"][0]["full_combo"] = 1.into();
+        song["levels"][0]["md5"] = md5.into();
+        song["levels"][0]["size"] = size.into();
+        song["mission_combo"] = mission_combo(1);
+        database::update_song(id, &song);
+        let revision = database::get_revision();
+
+        migrate::run();
+        assert_eq!(fs::read(&path).unwrap(), corrected_bytes);
+        assert_eq!(database::get_song(id).unwrap(), corrected_song);
+        assert_eq!(database::get_revision(), revision + 1);
+        migrate::run();
+        assert_eq!(fs::read(&path).unwrap(), corrected_bytes);
+        assert_eq!(database::get_revision(), revision + 1);
+
+        // A stale catalog alone must also recover, without rewriting the chart.
+        song = corrected_song.clone();
+        song["levels"][0]["full_combo"] = 1.into();
+        database::update_song(id, &song);
+        migrate::run();
+        assert_eq!(fs::read(&path).unwrap(), corrected_bytes);
+        assert_eq!(database::get_song(id).unwrap(), corrected_song);
+        assert_eq!(database::get_revision(), revision + 2);
+        migrate::run();
+        assert_eq!(database::get_revision(), revision + 2);
+    }
+
     // The live PLAY cue must never be a loop cue: the client reports a looping playback as
     // forever-playing, and the live's end trigger waits on playback-end, so a looping play cue
     // means the live never ends. New uploads emit is_loop:false, the preview cue keeps looping,

src/router/custom_song/chart.rs

@@ -18,8 +18,8 @@ use std::collections::HashMap;
 //   IsSliderLongMarker  chained, child SAME line, parent different    -> a slide ending in a hold
 //   IsDistanceMarker    has both a parent and a child                 -> a middle segment
 // So a hold is a chain that stays in its lane and a slide is a chain that moves across lanes;
-// both are type 1. This is also how the client counts combo (NoteData.CalcMaxCombo: a note whose
-// child shares its line does not count, its tail does).
+// both are type 1. Combo follows LiveStatusSystem.PushInputResult, not the editor-only
+// NoteData.CalcMaxCombo helper, which undercounts repeated same-lane checkpoints.
 //
 // Mapping rules:
 // - line = position - 1 (both are right-to-left)
@@ -65,8 +65,8 @@ use std::collections::HashMap;
 //   ForceGroupId against the other chunk's GroupId and links the lane-closest pair).
 // - notes[0] is ALWAYS the dummy header (id 0, num 100, type 0) - the client
 //   deserializes it verbatim.
-// - max_combo_count = all real notes EXCEPT hold heads whose tail is on the same
-//   line (the game counts a same-lane hold as one combo for the chain)
+// - max_combo_count excludes a same-lane hold's root and a cross-lane slide settling
+//   into a hold. Further same-lane checkpoints do count (see max_combo_count below).
 
 // Two notes are SIMULTANEOUS (one spawn cluster) when their times agree to within this.
 // Uploaded timings are decimal literals, so notes an author meant to be simultaneous parse to
@@ -143,6 +143,42 @@ pub fn first_note_time(chart: &JsonValue) -> Option<f64> {
         }))
 }
 
+// LiveStatusSystem.PushInputResult skips combo for a non-slider long root or
+// IsSliderLongMarker. LiveInputResultControl maps those results to their child.
+// All other points count, including repeated checkpoints in a stationary slide.
+// Validate the links before repairing stored data; never guess a broken chart's count.
+pub fn max_combo_count(chart: &JsonValue) -> Option<i64> {
+    let notes = &chart["notes"];
+    if !notes.is_array() || notes.len() < 2 || notes[0]["id"] != 0 {
+        return None;
+    }
+    let mut by_id = HashMap::new();
+    for note in notes.members().skip(1) {
+        let id = note["id"].as_i64()?;
+        let line = note["line"].as_i64()?;
+        if id <= 0 || !(0..9).contains(&line) || by_id.insert(id, note).is_some() {
+            return None;
+        }
+    }
+    let mut count = 0;
+    for note in notes.members().skip(1) {
+        let id = note["id"].as_i64()?;
+        let parent_id = note["parent_id"].as_i64()?;
+        let child_id = note["child_id"].as_i64()?;
+        let parent = if parent_id == 0 { None } else { Some(*by_id.get(&parent_id)?) };
+        let child = if child_id == 0 { None } else { Some(*by_id.get(&child_id)?) };
+        if parent_id == id || child_id == id
+            || parent.is_some_and(|p| p["child_id"] != id)
+            || child.is_some_and(|c| c["parent_id"] != id) {
+            return None;
+        }
+        let hold_start = child.is_some_and(|c| c["line"] == note["line"])
+            && parent.is_none_or(|p| p["line"] != note["line"]);
+        if !hold_start { count += 1; }
+    }
+    Some(count)
+}
+
 struct WorkNote {
     time: f64,
     line: i64,
@@ -324,17 +360,9 @@ pub fn transcode(beatmap: &JsonValue) -> Result<(JsonValue, i64), String> {
         "parent_id": 0, "child_id": 0, "child_num": 0, "child_line": 0,
         "force_sync_group_id": 0
     }];
-    let mut max_combo_count = 0;
     for index in order.iter() {
         let note = &work[*index];
 
-        // NoteData.CalcMaxCombo: a note whose child is on the SAME line (a hold) does not count,
-        // its tail does. A cross-lane child (a slide segment) counts normally.
-        match note.child {
-            Some(child) if work[child].line == note.line => {},
-            _ => max_combo_count += 1
-        }
-
         notes.push(object!{
             "id": ids[*index],
             "num": nums[*index],
@@ -349,12 +377,15 @@ pub fn transcode(beatmap: &JsonValue) -> Result<(JsonValue, i64), String> {
         }).unwrap();
     }
 
-    Ok((object!{
+    let mut chart = object!{
         "max_lane": 9,
         "sound_name": "",
-        "max_combo_count": max_combo_count,
+        "max_combo_count": 0,
         "notes": notes
-    }, max_combo_count))
+    };
+    let combo = max_combo_count(&chart).ok_or("Invalid transcoded note links")?;
+    chart["max_combo_count"] = combo.into();
+    Ok((chart, combo))
 }
 
 // Regroups a STORED transcoded chart whose spawn groups predate the pairing rule above: the
@@ -832,6 +863,56 @@ mod tests {
         assert_eq!(combo, 3);
     }
 
+    #[test]
+    fn repeated_same_lane_checkpoints_count_like_the_client() {
+        for final_effect in [11, 14] {
+            let mut beatmap = jzon::array![];
+            for i in 0..13 {
+                beatmap.push(sif_slide(1.0 + i as f64 * 0.1, 5,
+                    if i == 12 { final_effect } else { 11 }, 0.0, 500)).unwrap();
+            }
+            let (chart, combo) = transcode(&beatmap).unwrap();
+            // The head shares the first checkpoint's result; all twelve
+            // non-root points still have their own runtime judgment.
+            assert_eq!(combo, 12);
+            assert_eq!(chart["max_combo_count"], 12);
+            assert_eq!(chart["notes"].len(), 14);
+        }
+    }
+
+    #[test]
+    fn combo_count_preserves_hold_and_cross_lane_rules() {
+        for (positions, expected) in [
+            (vec![5, 5], 1),
+            (vec![5, 5, 5], 2),
+            (vec![5, 5, 6], 2),
+            (vec![4, 5, 5], 2),
+            (vec![4, 5, 5, 5], 3),
+            (vec![4, 5, 6], 3),
+        ] {
+            let mut beatmap = jzon::array![];
+            for (i, position) in positions.iter().enumerate() {
+                beatmap.push(sif_slide(1.0 + i as f64 * 0.1, *position, 11, 0.0, 500)).unwrap();
+            }
+            let (_, combo) = transcode(&beatmap).unwrap();
+            assert_eq!(combo, expected, "chain {:?}", positions);
+        }
+    }
+
+    #[test]
+    fn combo_repair_rejects_invalid_links() {
+        let (chart, _) = transcode(&jzon::array![sif_note(1.0, 5, 3, 1.0)]).unwrap();
+        for (field, value) in [("id", 2), ("parent_id", 999), ("child_id", 999), ("child_id", 1)] {
+            let mut broken = chart.clone();
+            broken["notes"][1][field] = value.into();
+            assert_eq!(max_combo_count(&broken), None, "invalid {}={}", field, value);
+        }
+        let mut broken = chart;
+        broken["notes"][2]["parent_id"] = 0.into();
+        assert_eq!(max_combo_count(&broken), None);
+        assert_eq!(max_combo_count(&object!{}), None);
+    }
+
     #[test]
     fn slide_hold_needs_a_duration() {
         // The effect 3 duration check has to cover note_slide_hold too

src/router/custom_song/migrate.rs

@@ -13,7 +13,8 @@ use crate::runtime::get_data_path;
 //
 // The same pass also corrects the stored catalog values that were fabricated rather than
 // derived from official masterdata (see below): the looping PLAY cue, the per-song score-rank
-// thresholds and the combo-mission targets.
+// thresholds and the combo-mission targets. Combo totals are recalculated using
+// the client's runtime rule, including repeated same-lane slide checkpoints.
 //
 // For each song directory with a catalog row, every chart with an over-shared num is
 // regrouped (chart::regroup), rewritten to disk, and its level's md5/size in the catalog
@@ -68,17 +69,6 @@ pub fn run() {
             changed = true;
             println!("Custom song {}: score rank thresholds reset to the official values", music_id);
         }
-        // Same "hardest difficulty" rule as upload/edit: the last level entry, which both write
-        // in ascending level order
-        if let Some(hardest) = song["levels"].members().last().and_then(|l| l["full_combo"].as_i64()) {
-            let missions = super::mission_combo(hardest);
-            if song["mission_combo"] != missions {
-                song["mission_combo"] = missions;
-                changed = true;
-                println!("Custom song {}: combo missions rescaled to the official 20/40/60/80%", music_id);
-            }
-        }
-
         for level in 1..=LEVEL_COUNT {
             let path = song_path(music_id, &format!("chart_{}.json", level));
             let Ok(bytes) = fs::read(&path) else { continue; };
@@ -86,26 +76,47 @@ pub fn run() {
                 println!("Custom song {} chart {}: not valid JSON, migration skipped", music_id, level);
                 continue;
             };
-            if !chart::regroup(&mut chart_data) {
+            let Some(full_combo) = chart::max_combo_count(&chart_data) else {
+                println!("Custom song {} chart {}: invalid note links, migration skipped", music_id, level);
                 continue;
-            }
-            let new_bytes = jzon::stringify(chart_data);
-            if let Err(e) = fs::write(&path, &new_bytes) {
-                println!("Custom song {} chart {}: rewrite failed ({}), migration skipped", music_id, level, e);
-                continue;
-            }
+            };
+            let regrouped = chart::regroup(&mut chart_data);
+            let recounted = chart_data["max_combo_count"] != full_combo;
+            let (md5, size) = if regrouped || recounted {
+                chart_data["max_combo_count"] = full_combo.into();
+                let new_bytes = jzon::stringify(chart_data);
+                if let Err(e) = fs::write(&path, &new_bytes) {
+                    println!("Custom song {} chart {}: rewrite failed ({}), migration skipped", music_id, level, e);
+                    continue;
+                }
+                changed = true;
+                charts_changed += 1;
+                println!("Custom song {} chart {}: repaired grouping={}, combo={}", music_id, level, regrouped, recounted);
+                asset_meta(new_bytes.as_bytes())
+            } else {
+                asset_meta(&bytes)
+            };
             // The catalog md5/size must follow the served bytes or the client's
             // download-and-verify loop would never accept the asset
-            let (md5, size) = asset_meta(new_bytes.as_bytes());
             for entry in song["levels"].members_mut() {
                 if entry["level"] == level {
+                    changed |= entry["full_combo"] != full_combo
+                        || entry["md5"] != md5 || entry["size"] != size;
+                    entry["full_combo"] = full_combo.into();
                     entry["md5"] = md5.clone().into();
                     entry["size"] = size.into();
                 }
             }
-            changed = true;
-            charts_changed += 1;
-            println!("Custom song {}: regrouped chart level {} (pre-pairing spawn groups)", music_id, level);
+        }
+
+        // Recompute after chart repair, using the hardest difficulty as upload/edit do.
+        if let Some(hardest) = song["levels"].members().last().and_then(|l| l["full_combo"].as_i64()) {
+            let missions = super::mission_combo(hardest);
+            if song["mission_combo"] != missions {
+                song["mission_combo"] = missions;
+                changed = true;
+                println!("Custom song {}: combo missions rescaled to the official 20/40/60/80%", music_id);
+            }
         }
 
         if changed {
@@ -116,6 +127,6 @@ pub fn run() {
 
     if songs_changed > 0 {
         database::bump_revision();
-        println!("Custom song spawn-group migration: rewrote {} chart(s) in {} song(s), catalog revision bumped", charts_changed, songs_changed);
+        println!("Custom song migration: rewrote {} chart(s) in {} song(s), catalog revision bumped", charts_changed, songs_changed);
     }
 }