From 588bc3f46434b9a06deb2030f4fe0c1829a98a59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moritz=20H=C3=B6lting?= <87192362+moritz-hoelting@users.noreply.github.com> Date: Sun, 10 Nov 2024 21:42:53 +0100 Subject: [PATCH] only prefix commands with '$' that actually use macros --- src/datapack/command/execute/conditional.rs | 139 +++++++++++--------- src/datapack/command/execute/mod.rs | 90 +++++++------ src/datapack/command/mod.rs | 95 ++++++------- src/datapack/function.rs | 14 +- src/util/compile.rs | 126 +++++++++++++++++- 5 files changed, 308 insertions(+), 156 deletions(-) diff --git a/src/datapack/command/execute/conditional.rs b/src/datapack/command/execute/conditional.rs index a6ef219..231bc3b 100644 --- a/src/datapack/command/execute/conditional.rs +++ b/src/datapack/command/execute/conditional.rs @@ -7,7 +7,7 @@ use std::{ use crate::{ prelude::Command, util::{ - compile::{CompileOptions, FunctionCompilerState, MutCompilerState}, + compile::{CompileOptions, CompiledCommand, FunctionCompilerState, MutCompilerState}, MacroString, }, }; @@ -16,22 +16,25 @@ use super::Execute; /// Compile an if condition command. /// The first tuple element is a boolean indicating if the prefix should be used for that command. +#[expect(clippy::too_many_arguments)] #[tracing::instrument(skip_all)] pub fn compile_if_cond( cond: &Condition, then: &Execute, el: Option<&Execute>, prefix: &str, + prefix_contains_macros: bool, options: &CompileOptions, global_state: &MutCompilerState, function_state: &FunctionCompilerState, -) -> Vec<(bool, String)> { +) -> Vec { if options.pack_format < 20 { compile_pre_20_format( cond, then, el, prefix, + prefix_contains_macros, options, global_state, function_state, @@ -42,6 +45,7 @@ pub fn compile_if_cond( then, el, prefix, + prefix_contains_macros, options, global_state, function_state, @@ -49,16 +53,18 @@ pub fn compile_if_cond( } } -#[allow(clippy::too_many_lines)] +#[expect(clippy::too_many_lines, clippy::too_many_arguments)] fn compile_pre_20_format( cond: &Condition, then: &Execute, el: Option<&Execute>, prefix: &str, + prefix_contains_macros: bool, options: &CompileOptions, global_state: &MutCompilerState, function_state: &FunctionCompilerState, -) -> Vec<(bool, String)> { +) -> Vec { + let contains_macro = prefix_contains_macros || cond.contains_macro(); let then_count = then.get_count(options); let str_cond = cond.clone().compile(options, global_state, function_state); @@ -93,14 +99,21 @@ fn compile_pre_20_format( .iter() .map(|s| { if allows_prefix { - (true, "run ".to_string() + s) + s.clone().apply_prefix("run ") } else { - (false, s.clone()) + s.clone() } }) .collect() } else { - then.compile_internal(String::new(), false, options, global_state, function_state) + then.compile_internal( + String::new(), + false, + contains_macro, + options, + global_state, + function_state, + ) }; // if the conditions have multiple parts joined by a disjunction, commands need to be grouped let each_or_cmd = (str_cond.len() > 1).then(|| { @@ -112,10 +125,10 @@ fn compile_pre_20_format( format!("data modify storage shulkerbox:cond {success_uid} set value true"), combine_conditions_commands( str_cond.clone(), - &[( - true, - format!("run data modify storage shulkerbox:cond {success_uid} set value true"), - )], + &[CompiledCommand::new(format!( + "run data modify storage shulkerbox:cond {success_uid} set value true" + )) + .or_contains_macros(contains_macro)], ), ) }); @@ -148,6 +161,7 @@ fn compile_pre_20_format( let el = el.compile_internal( String::new(), else_cond.len() > 1, + contains_macro, options, global_state, function_state, @@ -162,10 +176,10 @@ fn compile_pre_20_format( tracing::error!("No success_uid found for each_or_cmd, using default"); "if_success" }); - Some(( - false, - format!("data remove storage shulkerbox:cond {success_uid}"), - )) + Some( + CompiledCommand::new(format!("data remove storage shulkerbox:cond {success_uid}")) + .with_forbid_prefix(true), + ) } else { None }; @@ -178,26 +192,22 @@ fn compile_pre_20_format( .chain(then_commands) .chain(el_commands) .chain(reset_success_storage) - .map(|(use_prefix, cmd)| { - let cmd = if use_prefix { - prefix.to_string() + &cmd - } else { - cmd - }; - (use_prefix, cmd) - }) + .map(|cmd| cmd.apply_prefix(prefix)) .collect() } +#[expect(clippy::too_many_arguments)] fn compile_since_20_format( cond: &Condition, then: &Execute, el: Option<&Execute>, prefix: &str, + prefix_contains_macros: bool, options: &CompileOptions, global_state: &MutCompilerState, function_state: &FunctionCompilerState, -) -> Vec<(bool, String)> { +) -> Vec { + let contains_macros = prefix_contains_macros || cond.contains_macro(); let then_count = then.get_count(options); let str_cond = cond @@ -216,12 +226,14 @@ fn compile_since_20_format( function_state, ); let group = Command::Group(group_cmds); - let allows_prefix = !group.forbid_prefix(); - group - .compile(options, global_state, function_state) - .into_iter() - .map(|s| (allows_prefix, s)) - .collect() + let cmds = group.compile(options, global_state, function_state); + if contains_macros { + cmds.into_iter() + .map(|cmd| cmd.or_contains_macros(true)) + .collect() + } else { + cmds + } } else if then_count > 1 { let then_cmd = match then.clone() { Execute::Run(cmd) => vec![*cmd], @@ -239,30 +251,30 @@ fn compile_since_20_format( }; combine_conditions_commands_concat(str_cond, &then_cmd) .into_iter() - .map(|cmd| { - ( - cmd.forbid_prefix(), - cmd.compile(options, global_state, function_state), - ) - }) - .flat_map(|(forbid_prefix, cmds)| { - cmds.into_iter() - .map(move |cmd| (!forbid_prefix, prefix.to_string() + &cmd)) + .flat_map(|cmd| { + cmd.compile(options, global_state, function_state) + .into_iter() + .map(move |compiled_cmd| { + compiled_cmd + .apply_prefix(prefix) + .or_contains_macros(contains_macros) + }) }) .collect() } else { str_cond .into_iter() .flat_map(|cond| { - then.compile_internal(String::new(), false, options, global_state, function_state) - .into_iter() - .map(move |(require_prefix, cmd)| { - if require_prefix { - (true, prefix.to_string() + &cond.compile() + " " + &cmd) - } else { - (false, cmd) - } - }) + then.compile_internal( + String::new(), + false, + contains_macros, + options, + global_state, + function_state, + ) + .into_iter() + .map(move |cmd| cmd.apply_prefix(prefix.to_string() + &cond.compile() + " ")) }) .collect() } @@ -270,19 +282,15 @@ fn compile_since_20_format( fn combine_conditions_commands( conditions: Vec, - commands: &[(bool, String)], -) -> Vec<(bool, String)> { + commands: &[CompiledCommand], +) -> Vec { conditions .into_iter() .flat_map(|cond| { - commands.iter().map(move |(use_prefix, cmd)| { + let prefix = cond + " "; + commands.iter().map(move |cmd| { // combine the condition with the command if it uses a prefix - let cmd = if *use_prefix { - cond.clone() + " " + cmd - } else { - cmd.clone() - }; - (*use_prefix, cmd) + cmd.clone().apply_prefix(&prefix) }) }) .collect() @@ -652,18 +660,21 @@ mod tests { .into_iter() .map(str::to_string) .collect(); - let commands = &[(true, "1".to_string()), (false, "2".to_string())]; + let commands = &[ + CompiledCommand::new("1".to_string()), + CompiledCommand::new("2".to_string()).with_forbid_prefix(true), + ]; let combined = combine_conditions_commands(conditions, commands); assert_eq!( combined, vec![ - (true, "a 1".to_string()), - (false, "2".to_string()), - (true, "b 1".to_string()), - (false, "2".to_string()), - (true, "c 1".to_string()), - (false, "2".to_string()) + CompiledCommand::new("a 1".to_string()), + CompiledCommand::new("2".to_string()).with_forbid_prefix(true), + CompiledCommand::new("b 1".to_string()), + CompiledCommand::new("2".to_string()).with_forbid_prefix(true), + CompiledCommand::new("c 1".to_string()), + CompiledCommand::new("2".to_string()).with_forbid_prefix(true) ] ); } diff --git a/src/datapack/command/execute/mod.rs b/src/datapack/command/execute/mod.rs index 0bc6e03..4ac218b 100644 --- a/src/datapack/command/execute/mod.rs +++ b/src/datapack/command/execute/mod.rs @@ -2,7 +2,7 @@ use std::{collections::HashSet, ops::RangeInclusive, string::ToString}; use super::Command; use crate::util::{ - compile::{CompileOptions, FunctionCompilerState, MutCompilerState}, + compile::{CompileOptions, CompiledCommand, FunctionCompilerState, MutCompilerState}, ExtendableQueue, MacroString, }; @@ -39,7 +39,7 @@ impl Execute { options: &CompileOptions, global_state: &MutCompilerState, function_state: &FunctionCompilerState, - ) -> Vec { + ) -> Vec { // Directly compile the command if it is a run command, skipping the execute part // Otherwise, compile the execute command using internal function match self { @@ -48,30 +48,29 @@ impl Execute { .iter() .flat_map(|c| c.compile(options, global_state, function_state)) .collect(), - _ => self - .compile_internal( - String::from("execute "), - false, - options, - global_state, - function_state, - ) - .into_iter() - .map(|(_, cmd)| cmd) - .collect(), + _ => self.compile_internal( + String::from("execute "), + false, + false, + options, + global_state, + function_state, + ), } } /// Compile the execute command into strings with the given prefix. /// Each first tuple element is a boolean indicating if the prefix should be used for that command. + #[expect(clippy::too_many_lines)] fn compile_internal( &self, prefix: String, require_grouping: bool, + prefix_contains_macros: bool, options: &CompileOptions, global_state: &MutCompilerState, function_state: &FunctionCompilerState, - ) -> Vec<(bool, String)> { + ) -> Vec { match self { Self::Align(arg, next) | Self::Anchored(arg, next) @@ -89,6 +88,7 @@ impl Execute { arg = arg.compile() ), require_grouping, + prefix_contains_macros || arg.contains_macro(), options, global_state, function_state, @@ -99,6 +99,7 @@ impl Execute { selector = selector.compile() ), require_grouping, + prefix_contains_macros || selector.contains_macro(), options, global_state, function_state, @@ -108,6 +109,7 @@ impl Execute { then.as_ref(), el.as_deref(), &prefix, + prefix_contains_macros, options, global_state, function_state, @@ -119,6 +121,7 @@ impl Execute { arg = arg.compile() ), true, + prefix_contains_macros || arg.contains_macro(), options, global_state, function_state, @@ -127,6 +130,7 @@ impl Execute { Command::Execute(ex) => ex.compile_internal( prefix, require_grouping, + prefix_contains_macros, options, global_state, function_state, @@ -134,36 +138,38 @@ impl Execute { command => command .compile(options, global_state, function_state) .into_iter() - .map(|c| map_run_cmd(command.forbid_prefix(), c, &prefix)) + .map(|c| { + map_run_cmd(command.forbid_prefix(), c, &prefix) + .or_contains_macros(prefix_contains_macros) + }) .collect(), }, Self::Runs(commands) if !require_grouping => commands .iter() - .flat_map(|c| { - let forbid_prefix = c.forbid_prefix(); - match c { - Command::Execute(ex) => ex.compile_internal( - prefix.clone(), - require_grouping, - options, - global_state, - function_state, - ), - command => command - .compile(options, global_state, function_state) - .into_iter() - .map(move |c| (!forbid_prefix, c)) - .collect(), - } + .flat_map(|c| match c { + Command::Execute(ex) => ex.compile_internal( + prefix.clone(), + require_grouping, + prefix_contains_macros, + options, + global_state, + function_state, + ), + command => command.compile(options, global_state, function_state), + }) + .map(|cmd| { + map_run_cmd(false, cmd, &prefix).or_contains_macros(prefix_contains_macros) }) - .map(|(require_prefix, c)| map_run_cmd(!require_prefix, c, &prefix)) .collect(), Self::Runs(commands) => { let group = Command::Group(commands.clone()); group .compile(options, global_state, function_state) .into_iter() - .map(|c| map_run_cmd(group.forbid_prefix(), c, &prefix)) + .map(|c| { + map_run_cmd(group.forbid_prefix(), c, &prefix) + .or_contains_macros(prefix_contains_macros) + }) .collect() } } @@ -179,6 +185,7 @@ impl Execute { self.compile_internal( String::new(), false, + false, options, &global_state, &function_state, @@ -313,12 +320,11 @@ where /// Combine command parts, respecting if the second part is a comment /// The first tuple element is a boolean indicating if the prefix should be used -fn map_run_cmd(forbid_prefix: bool, cmd: String, prefix: &str) -> (bool, String) { - if forbid_prefix || cmd.is_empty() || cmd.chars().all(char::is_whitespace) { - (false, cmd) - } else { - (true, prefix.to_string() + "run " + &cmd) - } +fn map_run_cmd(forbid_prefix: bool, cmd: CompiledCommand, prefix: &str) -> CompiledCommand { + let forbid_prefix = + forbid_prefix || cmd.as_str().is_empty() || cmd.as_str().chars().all(char::is_whitespace); + cmd.or_forbid_prefix(forbid_prefix) + .apply_prefix(prefix.to_string() + "run ") } #[cfg(test)] @@ -343,7 +349,9 @@ mod tests { assert_eq!( compiled, - vec!["execute as @a if block ~ ~-1 ~ minecraft:stone run say hi".to_string()] + vec![CompiledCommand::new( + "execute as @a if block ~ ~-1 ~ minecraft:stone run say hi" + )] ); let direct = Execute::Run(Box::new("say direct".into())).compile( @@ -352,6 +360,6 @@ mod tests { &FunctionCompilerState::default(), ); - assert_eq!(direct, vec!["say direct".to_string()]); + assert_eq!(direct, vec![CompiledCommand::new("say direct")]); } } diff --git a/src/datapack/command/mod.rs b/src/datapack/command/mod.rs index 99defe1..6e2e028 100644 --- a/src/datapack/command/mod.rs +++ b/src/datapack/command/mod.rs @@ -15,7 +15,7 @@ use super::Function; use crate::{ prelude::Datapack, util::{ - compile::{CompileOptions, FunctionCompilerState, MutCompilerState}, + compile::{CompileOptions, CompiledCommand, FunctionCompilerState, MutCompilerState}, MacroString, }, }; @@ -53,14 +53,18 @@ impl Command { options: &CompileOptions, global_state: &MutCompilerState, function_state: &FunctionCompilerState, - ) -> Vec { + ) -> Vec { match self { - Self::Raw(command) => vec![command.clone()], - Self::UsesMacro(command) => vec![command.compile()], + Self::Raw(command) => vec![CompiledCommand::new(command.clone())], + Self::UsesMacro(command) => { + vec![CompiledCommand::new(command.compile()).with_contains_macros(true)] + } Self::Debug(message) => compile_debug(message, options), Self::Execute(ex) => ex.compile(options, global_state, function_state), Self::Group(commands) => compile_group(commands, options, global_state, function_state), - Self::Comment(comment) => vec!["#".to_string() + comment], + Self::Comment(comment) => { + vec![CompiledCommand::new("#".to_string() + comment).with_forbid_prefix(true)] + } Self::Concat(a, b) => { let a = a.compile(options, global_state, function_state); let b = b.compile(options, global_state, function_state); @@ -72,7 +76,9 @@ impl Command { } else if b.is_empty() { a.clone() } else { - a.clone() + b + b.clone() + .apply_prefix(a.as_str()) + .or_forbid_prefix(a.forbids_prefix()) } }) }) @@ -169,12 +175,12 @@ impl From<&mut Function> for Command { } } -fn compile_debug(message: &MacroString, option: &CompileOptions) -> Vec { +fn compile_debug(message: &MacroString, option: &CompileOptions) -> Vec { if option.debug { - vec![format!( + vec![CompiledCommand::new(format!( r#"tellraw @a [{{"text":"[","color":"dark_blue"}},{{"text":"DEBUG","color":"dark_green","hoverEvent":{{"action":"show_text","value":[{{"text":"Debug message generated by Shulkerbox"}},{{"text":"\nSet debug to 'false' to disable"}}]}}}},{{"text":"] ","color":"dark_blue"}},{{"text":"{}","color":"black"}}]"#, message.compile() - )] + ))] } else { Vec::new() } @@ -186,51 +192,52 @@ fn compile_group( options: &CompileOptions, global_state: &MutCompilerState, function_state: &FunctionCompilerState, -) -> Vec { +) -> Vec { let command_count = commands .iter() .map(|cmd| cmd.get_count(options)) .sum::(); // only create a function if there are more than one command - if command_count > 1 { - let uid = function_state.request_uid(); - let pass_macros = group_contains_macro(commands); + match command_count { + 0 => Vec::new(), + 1 => commands[0].compile(options, global_state, function_state), + _ => { + let uid = function_state.request_uid(); + let pass_macros = group_contains_macro(commands); - // calculate a hashed path for the function in the `sb` subfolder - let function_path = { - let function_path = function_state.path(); - let function_path = function_path.strip_prefix("sb/").unwrap_or(function_path); + // calculate a hashed path for the function in the `sb` subfolder + let function_path = { + let function_path = function_state.path(); + let function_path = function_path.strip_prefix("sb/").unwrap_or(function_path); - let pre_hash_path = function_path.to_owned() + ":" + &uid.to_string(); - let hash = md5::hash(pre_hash_path).to_hex_lowercase(); + let pre_hash_path = function_path.to_owned() + ":" + &uid.to_string(); + let hash = md5::hash(pre_hash_path).to_hex_lowercase(); - "sb/".to_string() + function_path + "/" + &hash[..16] - }; + "sb/".to_string() + function_path + "/" + &hash[..16] + }; - let namespace = function_state.namespace(); + let namespace = function_state.namespace(); - // create a new function with the commands - let mut function = Function::new(namespace, &function_path); - function.get_commands_mut().extend(commands.iter().cloned()); - function_state.add_function(&function_path, function); + // create a new function with the commands + let mut function = Function::new(namespace, &function_path); + function.get_commands_mut().extend(commands.iter().cloned()); + function_state.add_function(&function_path, function); - let mut function_invocation = format!("function {namespace}:{function_path}"); + let mut function_invocation = format!("function {namespace}:{function_path}"); - if pass_macros { - let macros_block = group_get_macros(commands) - .into_iter() - .map(|m| format!("{m}:$({m})")) - .collect::>() - .join(","); - function_invocation.push_str(&format!(" {{{macros_block}}}")); + if pass_macros { + // WARNING: this seems to be the only way to pass macros to the function called. + // Because everything is passed as a string, it looses one "level" of escaping per pass. + let macros_block = group_get_macros(commands) + .into_iter() + .map(|m| format!(r#"{m}:"$({m})""#)) + .collect::>() + .join(","); + function_invocation.push_str(&format!(" {{{macros_block}}}")); + } + + vec![CompiledCommand::new(function_invocation).with_contains_macros(pass_macros)] } - - vec![function_invocation] - } else { - commands - .iter() - .flat_map(|cmd| cmd.compile(options, global_state, function_state)) - .collect::>() } } @@ -388,12 +395,12 @@ mod tests { assert_eq!( command_a.compile(options, global_state, function_state), - vec!["say Hello, world!".to_string()] + vec![CompiledCommand::new("say Hello, world!")] ); assert_eq!(command_a.get_count(options), 1); assert_eq!( command_b.compile(options, global_state, function_state), - vec!["say foo bar".to_string()] + vec![CompiledCommand::new("say foo bar")] ); assert_eq!(command_b.get_count(options), 1); } @@ -408,7 +415,7 @@ mod tests { assert_eq!( comment.compile(options, global_state, function_state), - vec!["#this is a comment".to_string()] + vec![CompiledCommand::new("#this is a comment").with_forbid_prefix(true)] ); assert_eq!(comment.get_count(options), 0); } diff --git a/src/datapack/function.rs b/src/datapack/function.rs index cce8c4c..e5d627c 100644 --- a/src/datapack/function.rs +++ b/src/datapack/function.rs @@ -68,18 +68,20 @@ impl Function { if c.contains_macro() { cmds.into_iter() .map(|c| { - if c.starts_with('#') { - c + if c.contains_macros() { + let content = format!("${c}"); + c.with_command(content) } else { - format!("${c}") + c } + .to_string() }) - .collect() + .collect::>() } else { - cmds + cmds.into_iter().map(|c| c.to_string()).collect::>() } }) - .collect::>() + .collect::>() .join("\n"); VFile::Text(content) } diff --git a/src/util/compile.rs b/src/util/compile.rs index cc6927a..374aa65 100644 --- a/src/util/compile.rs +++ b/src/util/compile.rs @@ -1,6 +1,6 @@ //! Compile options for the compiler. -use std::sync::Mutex; +use std::{fmt::Display, ops::Deref, sync::Mutex}; use getset::Getters; @@ -86,3 +86,127 @@ impl FunctionCompilerState { uid } } + +/// Compiled command, ready to be written to a function. +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] +pub struct CompiledCommand { + /// The command string. + command: String, + /// Whether the command is not allowed to be prefixed. + forbid_prefix: bool, + /// Whether the command contains a macro. + contains_macros: bool, +} + +impl CompiledCommand { + /// Create a new compiled command. + #[must_use] + pub fn new(command: S) -> Self + where + S: Into, + { + Self { + command: command.into(), + forbid_prefix: false, + contains_macros: false, + } + } + + /// Get the command string. + #[must_use] + pub fn as_str(&self) -> &str { + &self.command + } + + /// Set the command string. + #[must_use] + pub fn with_command(mut self, command: String) -> Self { + self.command = command; + self + } + + /// Set whether the command is forbidden to be prefixed. + #[must_use] + pub fn with_forbid_prefix(mut self, forbid_prefix: bool) -> Self { + self.forbid_prefix = forbid_prefix; + self + } + + /// Set whether the command contains a macro. + #[must_use] + pub fn with_contains_macros(mut self, contains_macros: bool) -> Self { + self.contains_macros = contains_macros; + self + } + + /// Get whether the command is forbidden to be prefixed. + #[must_use] + pub fn forbids_prefix(&self) -> bool { + self.forbid_prefix + } + + /// Get whether the command contains a macro. + #[must_use] + pub fn contains_macros(&self) -> bool { + self.contains_macros + } + + /// Apply a prefix to the command (if allowed). + #[must_use] + pub fn apply_prefix(mut self, prefix: S) -> Self + where + S: Into, + { + if !self.forbid_prefix { + self.command = prefix.into() + &self.command; + } + self + } + + /// Combine current forbid prefix status with the input. + #[must_use] + pub fn or_forbid_prefix(mut self, forbid_prefix: bool) -> Self { + self.forbid_prefix = self.forbid_prefix || forbid_prefix; + self + } + + /// Combine current contains macro status with the input. + #[must_use] + pub fn or_contains_macros(mut self, contains_macros: bool) -> Self { + self.contains_macros = self.contains_macros || contains_macros; + self + } +} + +impl Deref for CompiledCommand { + type Target = String; + + fn deref(&self) -> &Self::Target { + &self.command + } +} + +impl Display for CompiledCommand { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.command) + } +} + +impl From for String { + fn from(compiled_command: CompiledCommand) -> Self { + compiled_command.command + } +} + +impl From for CompiledCommand { + fn from(value: String) -> Self { + Self::new(value) + } +} + +impl From<&str> for CompiledCommand { + fn from(value: &str) -> Self { + Self::new(value.to_string()) + } +}