correctly implement nested if-else

src/datapack/command/execute.rs

@@ -1,8 +1,12 @@
 use std::ops::{BitAnd, BitOr, Not};
 
-use crate::util::compile::{CompileOptions, FunctionCompilerState, MutCompilerState};
+use chksum_md5 as md5;
 
 use super::Command;
+use crate::util::{
+    compile::{CompileOptions, FunctionCompilerState, MutCompilerState},
+    ExtendableQueue,
+};
 
 #[allow(missing_docs)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
@@ -48,7 +52,7 @@ impl Execute {
             .collect()
         }
     }
-    #[allow(clippy::too_many_lines)]
+
     fn compile_internal(
         &self,
         prefix: String,
@@ -58,36 +62,19 @@ impl Execute {
         function_state: &FunctionCompilerState,
     ) -> Vec<(bool, String)> {
         match self {
-            Self::Align(align, next) => format_execute(
+            Self::Align(arg, next)
+            | Self::Anchored(arg, next)
+            | Self::As(arg, next)
+            | Self::At(arg, next)
+            | Self::Facing(arg, next)
+            | Self::In(arg, next)
+            | Self::On(arg, next)
+            | Self::Positioned(arg, next)
+            | Self::Rotated(arg, next)
+            | Self::Store(arg, next)
+            | Self::Summon(arg, next) => format_execute(
                 prefix,
-                &format!("align {align} "),
+                &format!("{op} {arg} ", op = self.variant_name()),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::Anchored(anchor, next) => format_execute(
-                prefix,
-                &format!("anchored {anchor} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::As(selector, next) => format_execute(
-                prefix,
-                &format!("as {selector} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::At(selector, next) => format_execute(
-                prefix,
-                &format!("at {selector} "),
                 next,
                 require_grouping,
                 options,
@@ -103,69 +90,6 @@ impl Execute {
                 global_state,
                 function_state,
             ),
-            Self::Facing(facing, next) => format_execute(
-                prefix,
-                &format!("facing {facing} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::In(dim, next) => format_execute(
-                prefix,
-                &format!("in {dim} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::On(dim, next) => format_execute(
-                prefix,
-                &format!("on {dim} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::Positioned(pos, next) => format_execute(
-                prefix,
-                &format!("positioned {pos} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::Rotated(rot, next) => format_execute(
-                prefix,
-                &format!("rotated {rot} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::Store(store, next) => format_execute(
-                prefix,
-                &format!("store {store} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
-            Self::Summon(entity, next) => format_execute(
-                prefix,
-                &format!("summon {entity} "),
-                next,
-                require_grouping,
-                options,
-                global_state,
-                function_state,
-            ),
             Self::If(cond, then, el) => compile_if_cond(
                 cond,
                 then.as_ref(),
@@ -201,8 +125,48 @@ impl Execute {
                 .collect(),
         }
     }
+
+    /// Get the count of the commands the execute command will compile into.
+    #[tracing::instrument(skip(options))]
+    pub(super) fn get_count(&self, options: &CompileOptions) -> usize {
+        let global_state = MutCompilerState::default();
+        let function_state =
+            FunctionCompilerState::new("[INTERNAL]", "[INTERNAL]", ExtendableQueue::default());
+
+        self.compile_internal(
+            String::new(),
+            false,
+            options,
+            &global_state,
+            &function_state,
+        )
+        .len()
     }
 
+    /// Get the variant name of the execute command.
+    #[must_use]
+    pub fn variant_name(&self) -> &str {
+        match self {
+            Self::Align(..) => "align",
+            Self::Anchored(..) => "anchored",
+            Self::As(..) => "as",
+            Self::At(..) => "at",
+            Self::AsAt(..) => "as_at",
+            Self::Facing(..) => "facing",
+            Self::In(..) => "in",
+            Self::On(..) => "on",
+            Self::Positioned(..) => "positioned",
+            Self::Rotated(..) => "rotated",
+            Self::Store(..) => "store",
+            Self::Summon(..) => "summon",
+            Self::If(..) => "if",
+            Self::Run(..) => "run",
+            Self::Runs(..) => "runs",
+        }
+    }
+}
+
+/// Combine command parts, respecting if the second part is a comment
 fn map_run_cmd(cmd: String, prefix: &str) -> (bool, String) {
     if cmd.starts_with('#') {
         (false, cmd)
@@ -211,6 +175,7 @@ fn map_run_cmd(cmd: String, prefix: &str) -> (bool, String) {
     }
 }
 
+/// Format the execute command, compiling the next command
 fn format_execute(
     prefix: String,
     new: &str,
@@ -229,6 +194,7 @@ fn format_execute(
     )
 }
 
+#[tracing::instrument(skip_all)]
 fn compile_if_cond(
     cond: &Condition,
     then: &Execute,
@@ -238,25 +204,28 @@ fn compile_if_cond(
     global_state: &MutCompilerState,
     function_state: &FunctionCompilerState,
 ) -> Vec<(bool, String)> {
-    // TODO: fix conflicting data storage location when nesting if-else conditions
+    let then_count = then.get_count(options);
 
-    let str_then = then.compile_internal(
-        prefix.to_string(),
-        false,
-        options,
-        global_state,
-        function_state,
-    );
     let str_cond = cond.clone().compile(options, global_state, function_state);
-    let require_grouping = el.is_some() || str_then.len() > 1;
+    let require_grouping_uid = (el.is_some() || then_count > 1).then(|| {
-    let then = if require_grouping {
+        let uid = function_state.request_uid();
+        let pre_hash = function_state.path().to_owned() + ":" + &uid.to_string();
+
+        md5::hash(pre_hash).to_hex_lowercase()
+    });
+    #[allow(clippy::option_if_let_else)]
+    let then = if let Some(success_uid) = require_grouping_uid.as_deref() {
         let mut group_cmd = match then.clone() {
             Execute::Run(cmd) => vec![*cmd],
             Execute::Runs(cmds) => cmds,
             ex => vec![Command::Execute(ex)],
         };
         if el.is_some() && str_cond.len() <= 1 {
-            group_cmd.push("data modify storage shulkerbox:cond if_success set value true".into());
+            group_cmd.push(
+                format!("data modify storage shulkerbox:cond {success_uid} set value true")
+                    .as_str()
+                    .into(),
+            );
         }
         Command::Group(group_cmd)
             .compile(options, global_state, function_state)
@@ -266,26 +235,34 @@ fn compile_if_cond(
     } else {
         then.compile_internal(
             String::new(),
-            require_grouping,
+            require_grouping_uid.is_some(),
             options,
             global_state,
             function_state,
         )
     };
     let each_or_cmd = (str_cond.len() > 1).then(|| {
+        let success_uid = require_grouping_uid.as_deref().unwrap_or_else(|| {
+            tracing::error!("No success_uid found for each_or_cmd, using default");
+            "if_success"
+        });
         (
-            "data modify storage shulkerbox:cond if_success set value true",
+            format!("data modify storage shulkerbox:cond {success_uid} set value true"),
             combine_conditions_commands(
                 str_cond.clone(),
                 &[(
                     true,
-                    "run data modify storage shulkerbox:cond if_success set value true".to_string(),
+                    format!("run data modify storage shulkerbox:cond {success_uid} set value true"),
                 )],
             ),
         )
     });
     let successful_cond = if each_or_cmd.is_some() {
-        Condition::Atom("data storage shulkerbox:cond {if_success:1b}".to_string()).compile(
+        let success_uid = require_grouping_uid.as_deref().unwrap_or_else(|| {
+            tracing::error!("No success_uid found for each_or_cmd, using default");
+            "if_success"
+        });
+        Condition::Atom(format!("data storage shulkerbox:cond {{{success_uid}:1b}}")).compile(
             options,
             global_state,
             function_state,
@@ -296,8 +273,12 @@ fn compile_if_cond(
     let then_commands = combine_conditions_commands(successful_cond, &then);
     let el_commands = el
         .map(|el| {
+            let success_uid = require_grouping_uid.as_deref().unwrap_or_else(|| {
+                tracing::error!("No success_uid found for each_or_cmd, using default");
+                "if_success"
+            });
             let else_cond =
-                (!Condition::Atom("data storage shulkerbox:cond {if_success:1b}".to_string()))
+                (!Condition::Atom(format!("data storage shulkerbox:cond {{{success_uid}:1b}}")))
                     .compile(options, global_state, function_state);
             let el = el.compile_internal(
                 String::new(),
@@ -311,9 +292,13 @@ fn compile_if_cond(
         .unwrap_or_default();
 
     let reset_success_storage = if each_or_cmd.is_some() || el.is_some() {
+        let success_uid = require_grouping_uid.as_deref().unwrap_or_else(|| {
+            tracing::error!("No success_uid found for each_or_cmd, using default");
+            "if_success"
+        });
         Some((
             false,
-            "data remove storage shulkerbox:cond if_success".to_string(),
+            format!("data remove storage shulkerbox:cond {success_uid}"),
         ))
     } else {
         None

src/datapack/command/mod.rs

@@ -48,6 +48,18 @@ impl Command {
             Self::Comment(comment) => vec!["#".to_string() + comment],
         }
     }
+
+    /// Get the count of the commands this command will compile into.
+    #[must_use]
+    fn get_count(&self, options: &CompileOptions) -> usize {
+        match self {
+            Self::Comment(_) => 0,
+            Self::Debug(_) => usize::from(options.debug),
+            Self::Raw(cmd) => cmd.split('\n').count(),
+            Self::Execute(ex) => ex.get_count(options),
+            Self::Group(_) => 1,
+        }
+    }
 }
 
 impl From<&str> for Command {
@@ -77,30 +89,25 @@ fn compile_debug(message: &str, option: &CompileOptions) -> Vec<String> {
     }
 }
 
+#[tracing::instrument(skip_all, fields(commands = ?commands))]
 fn compile_group(
     commands: &[Command],
     options: &CompileOptions,
     global_state: &MutCompilerState,
     function_state: &FunctionCompilerState,
 ) -> Vec<String> {
-    let str_commands = commands
+    let command_count = commands
         .iter()
-        .flat_map(|cmd| cmd.compile(options, global_state, function_state))
+        .map(|cmd| cmd.get_count(options))
-        .collect::<Vec<_>>();
+        .sum::<usize>();
-    if str_commands.len() > 1 {
+    if command_count > 1 {
-        let generated_functions = {
+        let uid = function_state.request_uid();
-            let generated_functions = function_state.generated_functions();
-            let amount = generated_functions.get();
-            generated_functions.set(amount + 1);
-
-            amount
-        };
 
         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() + ":" + &generated_functions.to_string();
+            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]
@@ -114,6 +121,9 @@ fn compile_group(
 
         vec![format!("function {namespace}:{function_path}")]
     } else {
-        str_commands
+        commands
+            .iter()
+            .flat_map(|cmd| cmd.compile(options, global_state, function_state))
+            .collect::<Vec<_>>()
     }
 }

src/datapack/mod.rs

@@ -130,17 +130,17 @@ impl Datapack {
                 tick_tag.add_value(tag::TagValue::Simple(function.to_owned()));
             }
             data_folder.add_file(
-                "minecraft/tags/functions/tick.json",
+                "minecraft/tags/function/tick.json",
                 tick_tag.compile_no_state(options).1,
             );
         }
         if !self.load.is_empty() {
             let mut load_tag = tag::Tag::new(tag::TagType::Functions, false);
-            for function in &self.tick {
+            for function in &self.load {
                 load_tag.add_value(tag::TagValue::Simple(function.to_owned()));
             }
             data_folder.add_file(
-                "minecraft/tags/functions/load.json",
+                "minecraft/tags/function/load.json",
                 load_tag.compile_no_state(options).1,
             );
         }

src/datapack/namespace.rs

@@ -101,7 +101,7 @@ impl Namespace {
         while let Some((path, function)) = functions.next() {
             let function_state = FunctionCompilerState::new(&path, &self.name, functions.clone());
             root_folder.add_file(
-                &format!("functions/{path}.mcfunction"),
+                &format!("function/{path}.mcfunction"),
                 function.compile(options, state, &function_state),
             );
         }

src/datapack/tag.rs

@@ -71,12 +71,12 @@ pub enum TagType {
 impl ToString for TagType {
     fn to_string(&self) -> String {
         match self {
-            Self::Blocks => "blocks".to_string(),
+            Self::Blocks => "block".to_string(),
-            Self::Fluids => "fluids".to_string(),
+            Self::Fluids => "fluid".to_string(),
-            Self::Items => "items".to_string(),
+            Self::Items => "item".to_string(),
-            Self::Entities => "entity_types".to_string(),
+            Self::Entities => "entity_type".to_string(),
-            Self::GameEvents => "game_events".to_string(),
+            Self::GameEvents => "game_event".to_string(),
-            Self::Functions => "functions".to_string(),
+            Self::Functions => "function".to_string(),
             Self::Others(path) => path.to_string(),
         }
     }

src/util/compile.rs

@@ -1,6 +1,6 @@
 //! Compile options for the compiler.
 
-use std::{cell::Cell, sync::Mutex};
+use std::sync::Mutex;
 
 use getset::Getters;
 
@@ -32,11 +32,10 @@ pub struct CompilerState {}
 pub type MutCompilerState = Mutex<CompilerState>;
 
 /// State of the compiler for each function that can change during compilation.
-#[derive(Debug, Clone, Getters)]
+#[derive(Debug, Getters)]
 pub struct FunctionCompilerState {
-    /// Number of generated functions in the current function.
+    /// Next unique identifier.
-    #[get = "pub"]
+    uid_counter: Mutex<usize>,
-    generated_functions: Cell<usize>,
     /// Path of the current function.
     #[get = "pub"]
     path: String,
@@ -54,7 +53,7 @@ impl FunctionCompilerState {
     #[must_use]
     pub fn new(path: &str, namespace: &str, functions: FunctionQueue) -> Self {
         Self {
-            generated_functions: Cell::new(0),
+            uid_counter: Mutex::new(0),
             namespace: namespace.to_string(),
             path: path.to_string(),
             functions,
@@ -65,4 +64,12 @@ impl FunctionCompilerState {
     pub fn add_function(&self, name: &str, function: Function) {
         self.functions.push((name.to_string(), function));
     }
+
+    #[must_use]
+    pub fn request_uid(&self) -> usize {
+        let mut guard = self.uid_counter.lock().unwrap();
+        let uid = *guard;
+        *guard += 1;
+        uid
+    }
 }

src/util/extendable_queue.rs

@@ -11,7 +11,7 @@ pub struct ExtendableQueue<T> {
     queue: Arc<RwLock<VecDeque<T>>>,
 }
 
-impl Default for ExtendableQueue<String> {
+impl<T> Default for ExtendableQueue<T> {
     fn default() -> Self {
         Self {
             queue: Arc::new(RwLock::new(VecDeque::new())),