Preserve modern SNBT literals during data execution

Reuse the admitted modern SNBT decode contract for merge and literal modify.
Add actual Game storage and command-oracle regressions for typed values and
no-change copy callbacks.

Co-Authored-By: Codex gpt-6.1-sol <[email protected]>

crates/horizon-client/src/game/commands/data.rs

@@ -837,13 +837,15 @@ impl Game<'_> {
         }
         self.cmd_data_in(id, ctx, target)
     }
+    /// `DataCommands.mergeData`: literal values keep the modern argument
+    /// parser's decoded Tag contract through execution.
     fn cmd_data_in(&mut self, id: &str, ctx: &Context, target: DataRef) -> R<i32> {
         match id {
             "data.merge" => {
                 let Some(Arg::Str(nbt)) = ctx.get("nbt") else {
                     return Ok(0);
                 };
-                let (nbt, _) = parse_snbt(nbt).map_err(|e| err("argument.nbt.invalid", &[&e]))?;
+                let nbt = horizon_client::command::snbt::parse_fully(nbt)?;
                 let old = self.data_of(&target)?;
                 if horizon_client::nbt_path::too_deep(&nbt, 0) {
                     return Err(err("arguments.nbtpath.too_deep", &[]));
@@ -897,6 +899,8 @@ impl Game<'_> {
     fn system_feedback(&mut self, message: Text) {
         self.feedback(message);
     }
+    /// `DataCommands.manipulateData`: literal sources use the same SNBT
+    /// decode contract as command admission; stored sources are unchanged.
     fn cmd_data_modify(&mut self, ctx: &Context, target: DataRef) -> R<i32> {
         let target_path = self.path_arg(ctx, "targetPath")?;
         let after_modify: Vec<&String> = ctx
@@ -922,11 +926,7 @@ impl Game<'_> {
             let Some(Arg::Str(text)) = ctx.get("value") else {
                 return Ok(0);
             };
-            vec![
-                parse_snbt(text)
-                    .map_err(|e| err("argument.nbt.invalid", &[&e]))?
-                    .0,
-            ]
+            vec![horizon_client::command::snbt::parse_fully(text)?]
         } else {
             let source = self.data_ref(ctx, 1)?;
             let data = self.data_of(&source)?;
@@ -1508,3 +1508,123 @@ mod loot_table_tests {
         assert!(saved.get("Items").is_none());
     }
 }
+
+#[cfg(test)]
+mod modern_literal_tests {
+    use super::*;
+    use crate::game::commands::execute::ExecState;
+    use crate::game::{Audio, Options, Progress, Settings, load_resources};
+
+    #[test]
+    #[ignore = "needs prepared resources in HORIZON_TEST_RESOURCES and scratch homes/cwd"]
+    fn admitted_modern_literals_reach_actual_storage_as_typed_tags() {
+        let root = std::path::PathBuf::from(std::env::var_os("HORIZON_TEST_RESOURCES").unwrap());
+        let settings = Settings::default();
+        let loaded = load_resources(
+            root.clone(),
+            None,
+            &root,
+            &settings,
+            std::time::Instant::now(),
+            false,
+            true,
+            &Progress::new(),
+            &mut |_, _| unreachable!(),
+        )
+        .unwrap();
+        let mut options = Options::parse(std::iter::empty()).unwrap().unwrap();
+        options.resources = root.clone();
+        options.execution_role = crate::game::execution_role::ExecutionRole::IntegratedServer;
+        let mut game = Game::new(
+            &loaded.resources,
+            loaded.pack,
+            loaded.gui.unwrap(),
+            options,
+            settings,
+            Audio::silent(&root),
+            None,
+        )
+        .unwrap();
+        game.build_registries();
+        game.set_allow_commands(true);
+        let run = |g: &mut Game<'_>, line: &str| {
+            g.exec = ExecState::default();
+            let source = g.player_source();
+            g.execute_line(line, source)
+        };
+        let cases = [
+            ("hex", "0x10", "16", Tag::Int(16)),
+            ("underscore", "1_000", "1000", Tag::Int(1000)),
+            ("unicode4", r#""\u0041""#, r#""A""#, Tag::String("A".into())),
+            ("hex2", r#""\x41""#, r#""A""#, Tag::String("A".into())),
+            (
+                "unicode8",
+                r#""\U00000041""#,
+                r#""A""#,
+                Tag::String("A".into()),
+            ),
+            (
+                "trailing",
+                "{n:3,}",
+                "{n:3}",
+                Tag::compound([("n", Tag::Int(3))]),
+            ),
+            ("plain", "plain", r#""plain""#, Tag::String("plain".into())),
+        ];
+        let clock = game.world.game_time;
+        let mut differences = Vec::new();
+        for (name, modern, plain, expected) in &cases {
+            for route in ["merge", "modify"] {
+                let target = format!("h189:{route}_{name}");
+                let reference = format!("h189:ref_{route}_{name}");
+                let seed = ["data merge storage ", &target, r#" {v:"seed"}"#].concat();
+                assert_eq!(run(&mut game, &seed).unwrap(), 1);
+                assert_eq!(
+                    game.score.storage.get(&target).get("v"),
+                    Some(&Tag::String("seed".into()))
+                );
+                let canonical = ["data merge storage ", &reference, " {v:", plain, "}"].concat();
+                assert_eq!(run(&mut game, &canonical).unwrap(), 1);
+                assert_eq!(game.score.storage.get(&reference).get("v"), Some(expected));
+                let command = if route == "merge" {
+                    ["data merge storage ", &target, " {v:", modern, "}"].concat()
+                } else {
+                    ["data modify storage ", &target, " v set value ", modern].concat()
+                };
+                let dispatcher = horizon_client::command::vanilla::game_dispatcher_with_features(
+                    &game.level.enabled_features,
+                );
+                assert!(
+                    dispatcher
+                        .parse(&command, &game.ui.registries)
+                        .executable()
+                        .is_ok(),
+                    "admission: {route}/{name}"
+                );
+                let result = run(&mut game, &command);
+                let actual = game.score.storage.get(&target).get("v").cloned();
+                if result.as_ref().ok() != Some(&1) || actual.as_ref() != Some(expected) {
+                    differences.push(format!("{route}/{name}: result={result:?}, stored={actual:?}, expected={expected:?}"));
+                }
+                // Copy actual canonical stored data back: equality is a real
+                // no-change failure, independent of a formatter or parser stub.
+                let copy = [
+                    "data modify storage ",
+                    &target,
+                    " v set from storage ",
+                    &reference,
+                    " v",
+                ]
+                .concat();
+                if run(&mut game, &copy).is_ok() {
+                    differences.push(format!(
+                        "{route}/{name}: canonical copy changed the stored Tag"
+                    ));
+                }
+                assert_eq!(game.score.storage.get(&target).get("v"), Some(expected));
+            }
+        }
+        assert_eq!(game.world.game_time, clock);
+        assert!(differences.is_empty(), "{}", differences.join("\n"));
+    }
+}

tools/command_exec_corpus.py

@@ -5775,6 +5775,10 @@ ISOLATED_GROUPS = ISOLATED_REINFORCEMENTS | ISOLATED_RECIPES | ISOLATED_WOLF_ANG
 
 
 def groups():
+    try:
+        from .data_literal_corpus import storage_literals
+    except ImportError:
+        from data_literal_corpus import storage_literals
     try:
         from .transmute_patch_corpus import cases as transmute_patches
     except ImportError:
@@ -5840,6 +5844,7 @@ def groups():
 
 
         ('decorated pot data', decorated_pot_data()),
+        ('modern data literal values', storage_literals(cmd)),
         ('comparator output data', comparator_output_data()),
         ('hopper empty slot limits', hopper_empty_slot_limits()),
         ('End clock commands', end_clock_commands()),

tools/data_literal_corpus.py

@@ -0,0 +1,56 @@
+"""H189 storage literal design; no production parser or formatter changes."""
+
+
+def cases():
+    return [
+        ('hex', '0x10', '16', 16),
+        ('underscore', '1_000', '1000', 1000),
+        ('unicode4', r'"\u0041"', '"A"', 1),
+        ('hex2', r'"\x41"', '"A"', 1),
+        ('unicode8', r'"\U00000041"', '"A"', 1),
+        ('trailing', '{n:3,}', '{n:3}', 1),
+        ('plain', 'plain', '"plain"', 5),
+    ]
+
+
+def storage_literals(cmd):
+    ops = [cmd('/tick freeze'),
+           cmd('/scoreboard objectives add h189 dummy', expect_result=1),
+           cmd('/scoreboard players set #before h189 -31', expect_result=-31),
+           cmd('/scoreboard players set #after h189 -17', expect_result=-17),
+           cmd('/execute store result score #before h189 store success score #clock_ok h189 run time query gametime', False),
+           cmd('/scoreboard players get #clock_ok h189', expect_result=1),
+           cmd('/data merge storage h189:admission {v:21}', expect_result=1),
+           cmd('/data get storage h189:admission v', expect_result=21)]
+
+    def stored(command, result, success):
+        return [cmd('/scoreboard players set #r h189 31', expect_result=31),
+                cmd('/scoreboard players set #s h189 17', expect_result=17),
+                cmd('/execute store result score #r h189 store success score #s h189 run ' + command),
+                cmd('/scoreboard players get #r h189', expect_result=result),
+                cmd('/scoreboard players get #s h189', expect_result=success)]
+
+    for name, modern, plain, value in cases():
+        reference = 'h189:ref_' + name
+        ops += [cmd('/data merge storage ' + reference + ' {v:' + plain + '}', expect_result=1),
+                cmd('/data get storage ' + reference + ' v', expect_result=value)]
+        for route in ['merge', 'modify']:
+            target = 'h189:' + route + '_' + name
+            ops += [cmd('/data merge storage ' + target + ' {v:"seed"}', expect_result=1),
+                    cmd('/data get storage ' + target + ' v', expect_result=4)]
+            write = ('data merge storage ' + target + ' {v:' + modern + '}' if route == 'merge'
+                     else 'data modify storage ' + target + ' v set value ' + modern)
+            ops += stored(write, 1, 1)
+            ops += [cmd('/data get storage ' + target + ' v', expect_result=value)]
+            # The real stored Tag must equal the canonical one: copying it
+            # back must be a no-change failure, independently of pretty text.
+            ops += stored('data modify storage ' + target + ' v set from storage ' + reference + ' v', 0, 0)
+            ops += [cmd('/data get storage ' + target + ' v', expect_result=value),
+                    cmd('/data remove storage ' + target + ' v', expect_result=1)]
+        ops += [cmd('/data remove storage ' + reference + ' v', expect_result=1)]
+    ops += [cmd('/data remove storage h189:admission v', expect_result=1),
+            cmd('/execute store result score #after h189 store success score #clock_ok h189 run time query gametime', False),
+            cmd('/scoreboard players get #clock_ok h189', expect_result=1),
+            cmd('/execute if score #before h189 = #after h189', expect_result=1),
+            cmd('/scoreboard objectives remove h189'), cmd('/tick unfreeze')]
+    return ops