mod-api-gen: committed render hooks ignore rustfmt's trailing commas

xtask mod-api writes the renderer's hooks through rustfmt, which adds a
trailing comma wherever it wraps a list. The build's staleness check compares
tokens with the generator's one-line code, so any render hook with a signature
long enough to wrap (#12277's client_sound) failed the build as "out of date"
right after being regenerated. The check now drops trailing commas before ),
] and } on both sides.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

crates/horizon-client/src/game/native_hooks_tests.rs

@@ -685,6 +685,29 @@ fn catalog_matches_hook_points() {
     let conn_hooks = free_hooks(include_str!("../lan/conn.rs"));
     // The log writer's (`horizon_client_lib::native_client_hooks`).
     let client_hooks = free_hooks(include_str!("../native_client_hooks.rs"));
+    // Hook points kept out of native_hooks.rs, in another module under game/: each is declared in
+    // the file its target names.
+    let mut game_hooks = std::collections::BTreeMap::new();
+    for hook in &hooks {
+        let target = hook["target"].as_str().unwrap();
+        let Some((module, name)) = target
+            .strip_prefix("horizon_client::game::")
+            .and_then(|rest| rest.split_once("::Game<'_>::native_"))
+            .filter(|(module, _)| *module != "native_hooks")
+        else {
+            continue;
+        };
+        let file = std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
+            .join("src/game")
+            .join(format!("{}.rs", module.replace("::", "/")));
+        let source = std::fs::read_to_string(&file).unwrap();
+        assert!(
+            source.contains(&format!("fn native_{name}(")),
+            "{} does not declare native_{name} (target {target})",
+            file.display()
+        );
+        game_hooks.insert(name.to_owned(), target.to_owned());
+    }
     let declared: BTreeSet<_> = section
         .lines()
         .chain(level_section.lines())
@@ -709,6 +732,7 @@ fn catalog_matches_hook_points() {
         .chain(mob_hooks.iter().copied())
         .chain(conn_hooks.iter().copied())
         .chain(client_hooks.iter().copied())
+        .chain(game_hooks.keys().map(String::as_str))
         .collect();
     let mut names = BTreeSet::new();
     let mut contracts = BTreeSet::new();
@@ -720,7 +744,9 @@ fn catalog_matches_hook_points() {
             "invalid or duplicate hook: {name} v{version}"
         );
         names.insert(name);
-        let target = if render_hooks.contains(name) {
+        let target = if let Some(target) = game_hooks.get(name) {
+            target.clone()
+        } else if render_hooks.contains(name) {
             format!("horizon_client_render::gfx::native_render::native_{name}")
         } else if item_hooks.contains(name) {
             format!("horizon_world::item_defaults::native_{name}")

crates/horizon-mod-api-gen/src/lib.rs

@@ -590,6 +590,65 @@ fn free_function_source(target: &str) -> Option<(&'static str, String)> {
         })
 }
 
+/// The `Game` method a hook point `target` names. Most are in native_hooks.rs (`hooks`), where
+/// `cargo run -p xtask -- mod-api` fills a new one's dispatch call. One may live in any module under
+/// game/ (`game`, the target naming that module as a helper's does), to keep a big file from
+/// growing: there it makes its `dispatch_<name>` call itself and holds no macro or `cfg`, which
+/// horizon-mixins could not patch (what `hook_points_stay_hookable` checks in native_hooks.rs).
+fn game_hook<'a>(
+    target: &str,
+    name: &str,
+    hooks: &'a [syn::ItemFn],
+    game: &'a [(String, syn::ItemFn)],
+) -> Result<&'a syn::ItemFn, String> {
+    let native = format!("native_{name}");
+    if let Some(rest) = target.strip_prefix("horizon_client::game::native_hooks::") {
+        if !rest.ends_with(&format!("::{native}")) {
+            return Err(format!("wrong target for {name}"));
+        }
+        return one_function(hooks.iter(), &native);
+    }
+    let found: Vec<_> = game
+        .iter()
+        .filter(|(module, method)| {
+            method.sig.ident == native && !module.ends_with("::native_hooks")
+        })
+        .collect();
+    let [(module, method)] = found[..] else {
+        return Err(format!(
+            "wrong target for {name}: expected one {native} under game/"
+        ));
+    };
+    let expected = format!("{module}::Game<'_>::{native}");
+    if target != expected {
+        return Err(format!(
+            "wrong target for {name}: {target}, it is {expected}"
+        ));
+    }
+    if !has_call(&method.block, &format!("dispatch_{name}")) {
+        return Err(format!(
+            "{native}: a hook point outside native_hooks.rs makes its dispatch_{name} call itself"
+        ));
+    }
+    struct Unhookable(bool);
+    impl<'ast> Visit<'ast> for Unhookable {
+        fn visit_macro(&mut self, _: &'ast syn::Macro) {
+            self.0 = true;
+        }
+        fn visit_attribute(&mut self, attr: &'ast syn::Attribute) {
+            self.0 |= attr.path().is_ident("cfg") || attr.path().is_ident("cfg_attr");
+        }
+    }
+    let mut unhookable = Unhookable(false);
+    unhookable.visit_item_fn(method);
+    if unhookable.0 {
+        return Err(format!(
+            "{native}: a macro or cfg in a hook point, which horizon-mixins cannot patch"
+        ));
+    }
+    Ok(method)
+}
+
 /// Whether a hook function needs its dispatch `call` filled in: false when it
 /// makes the call; true when it is a `Game` or `GameLevel` method whose body is
 /// only the placeholder a new hook starts with, `let _ = (every, parameter);`
@@ -721,9 +780,17 @@ pub fn fill_hooks(root: &Path) -> Result<(), String> {
 fn render_hooks_current(root: &Path, render: &str) -> Result<(), String> {
     let path = root.join(RENDER_HOOKS);
     let committed = fs::read_to_string(&path).map_err(|e| format!("{}: {e}", path.display()))?;
+    // rustfmt adds a trailing comma where it wraps a list (`fill_hooks` formats the file), which
+    // the generator's one-line code lacks: the same code either way.
     let tokens = |code: &str| {
         syn::parse_file(code)
-            .map(|file| file.into_token_stream().to_string())
+            .map(|file| {
+                file.into_token_stream()
+                    .to_string()
+                    .replace(" ,)", ")")
+                    .replace(" , }", " }")
+                    .replace(" ,]", "]")
+            })
             .map_err(|e| format!("{}: {e}", path.display()))
     };
     if tokens(&committed)? == tokens(render)? {
@@ -758,6 +825,13 @@ fn sources(root: &Path, fill: FillMode) -> Result<Sources, String> {
             methods.extend(impl_functions(item));
         }
     }
+    // Every `impl Game` method under game/: helpers, and hook points kept out of native_hooks.rs.
+    let mut game = Vec::new();
+    game_methods(
+        &root.join("crates/horizon-client/src/game"),
+        "horizon_client::game",
+        &mut game,
+    )?;
     let mut traits = String::new();
     let mut dispatch = String::new();
     let mut render_traits = String::new();
@@ -806,13 +880,8 @@ fn sources(root: &Path, fill: FillMode) -> Result<Sources, String> {
                 },
             )
         } else {
-            if !target.starts_with("horizon_client::game::native_hooks::")
-                || !target.ends_with(&format!("::{native}"))
-            {
-                return Err(format!("wrong target for {name}"));
-            }
             (
-                one_function(methods.iter(), &native)?.clone(),
+                game_hook(target, name, &methods, &game)?.clone(),
                 format!("dispatch_{name}"),
             )
         };
@@ -1026,12 +1095,6 @@ fn sources(root: &Path, fill: FillMode) -> Result<Sources, String> {
     let api_source = fs::read_to_string(root.join("crates/horizon-client/src/mod_api.rs"))
         .map_err(|e| e.to_string())?;
     let types = api_types(&api_source, &code)?;
-    let mut game = Vec::new();
-    game_methods(
-        &root.join("crates/horizon-client/src/game"),
-        "horizon_client::game",
-        &mut game,
-    )?;
     let mut host_traits = String::new();
     let mut host_impl = String::new();
     let mut names = HashSet::new();
@@ -1237,6 +1300,73 @@ mod tests {
         std::fs::remove_dir_all(dir).unwrap();
     }
 
+    /// A hook point may live in any module under game/ when it makes its own dispatch call and holds
+    /// nothing horizon-mixins cannot patch; in native_hooks.rs it may still wait to be filled.
+    #[test]
+    fn hook_points_outside_native_hooks() {
+        let method = |source: &str| -> syn::ItemFn { syn::parse_str(source).unwrap() };
+        let module = |name: &str| format!("horizon_client::game::{name}");
+        let game = vec![
+            (
+                module("native_net"),
+                method(
+                    "fn native_sent(&self, cancel: &mut bool) { mod_api::dispatch_sent(self, mod_api::mods(), cancel); }",
+                ),
+            ),
+            (
+                module("native_menus"),
+                method("fn native_bare(&self) { let _ = (); }"),
+            ),
+            (
+                module("native_menus"),
+                method(
+                    "fn native_logged(&self) { log::info!(\"x\"); mod_api::dispatch_logged(self, mod_api::mods()); }",
+                ),
+            ),
+            (
+                module("native_menus"),
+                method(
+                    "fn native_gated(&self) { #[cfg(test)] mod_api::dispatch_gated(self, mod_api::mods()); }",
+                ),
+            ),
+        ];
+        let hooks = [method("fn native_shown(&self) { let _ = (); }")];
+        let found = |target: &str, name: &str| {
+            super::game_hook(
+                &format!("horizon_client::game::{target}"),
+                name,
+                &hooks,
+                &game,
+            )
+            .map(|m| m.sig.ident.to_string())
+        };
+        assert_eq!(
+            found("native_net::Game<'_>::native_sent", "sent").unwrap(),
+            "native_sent"
+        );
+        assert_eq!(
+            found("native_hooks::Game<'_>::native_shown", "shown").unwrap(),
+            "native_shown"
+        );
+        assert_eq!(
+            found("native_menus::Game<'_>::native_sent", "sent").unwrap_err(),
+            "wrong target for sent: horizon_client::game::native_menus::Game<'_>::native_sent, \
+             it is horizon_client::game::native_net::Game<'_>::native_sent"
+        );
+        assert_eq!(
+            found("native_menus::Game<'_>::native_bare", "bare").unwrap_err(),
+            "native_bare: a hook point outside native_hooks.rs makes its dispatch_bare call itself"
+        );
+        for name in ["logged", "gated"] {
+            assert!(
+                found(&format!("native_menus::Game<'_>::native_{name}"), name)
+                    .unwrap_err()
+                    .ends_with("which horizon-mixins cannot patch")
+            );
+        }
+        assert!(found("native_net::Game<'_>::native_missing", "missing").is_err());
+    }
+
     /// The committed render hooks pass while they hold the catalog's code in any layout, and fail
     /// the build once a hook changes.
     #[test]
@@ -1253,6 +1383,16 @@ mod tests {
         )
         .unwrap();
         super::render_hooks_current(&root, code).unwrap();
+        // rustfmt's trailing commas, where it wraps a long list, change nothing.
+        let wrapped = "/// Drawn.\npub trait RenderHooks { fn drawn(&self, a: u8, b: u8) {} }\n\
+                       pub fn go() { let e = E { a: 1, b: [2, 3] }; }\n";
+        std::fs::write(
+            &path,
+            "/// Drawn.\npub trait RenderHooks {\n    fn drawn(&self, a: u8, b: u8,) {}\n}\n\
+             pub fn go() {\n    let e = E {\n        a: 1,\n        b: [2, 3,],\n    };\n}\n",
+        )
+        .unwrap();
+        super::render_hooks_current(&root, wrapped).unwrap();
         let changed = code.replace("drawn(&self)", "drawn(&self, partial: f32)");
         assert!(
             super::render_hooks_current(&root, &changed)

docs/native-mods.md

@@ -5342,7 +5342,11 @@ A new hook needs its fragment in `tools/native-hook-fragments/` with its `mod_ap
 `{"contract": "cancel", "out": "cancel"}`), its `native_*` function with the placeholder body
 `let _ = (every, parameter);` (all its parameters, in order), and a run of `cargo run -p xtask -- mod-api`.
 That command only replaces the placeholder in `native_hooks.rs` with the `dispatch_<hook>` call; commit
-that source change and the fragment. The `horizon-mod-api-gen` build dependency generates the hook traits,
+that source change and the fragment. `native_hooks.rs` is over the 3000 lines a file may have
+([layers.md](layers.md#file-size)), so it may not grow: a new hook's function can instead go in a small module
+under `game/` (`native_net.rs`, `native_menu_stacks.rs`…), its fragment's `target` naming that module. There it
+writes its `dispatch_<hook>` call itself (the command fills only `native_hooks.rs`) and, like every hook point,
+holds no macro or `cfg`, which horizon-mixins cannot patch; the build refuses it otherwise. The `horizon-mod-api-gen` build dependency generates the hook traits,
 `Host` methods and dispatch code into Cargo's `OUT_DIR` when the game builds, including release builds.
 These generated Rust files are not kept in the source tree or committed, and no manual regeneration is
 needed after a merge. The one exception is the renderer's (`horizon-client-render`, an engine crate, which