fix: /help (#1423)

pumpkin/src/command/tree/format.rs

@@ -40,7 +40,7 @@ fn flatten_require_nodes(nodes: &[Node], children: &[usize]) -> Vec<usize> {
         let node = &nodes[i];
         match &node.node_type {
             NodeType::Require { .. } => {
-                new_children.extend(flatten_require_nodes(nodes, node.children.as_slice()));
+                new_children.extend(flatten_require_nodes(nodes, &node.children));
             }
             _ => new_children.push(i),
         }
@@ -49,57 +49,74 @@ fn flatten_require_nodes(nodes: &[Node], children: &[usize]) -> Vec<usize> {
     new_children
 }
 
+fn write_tree(f: &mut Formatter<'_>, nodes: &[Node], children: &[usize]) -> std::fmt::Result {
+    // Map node indices to actual nodes
+    // NOTE: We assume that Require nodes have already been "flattened" out
+    let nodes_iter = children.iter().map(|&idx| &nodes[idx]);
+
+    // Check if there is a sibling of type ExecuteLeaf
+    // If there is, arguments on the current level are optional and will be printed surrounded by square brackets
+    let argument_is_optional = nodes_iter
+        .clone()
+        .any(|node| matches!(node.node_type, NodeType::ExecuteLeaf { .. }));
+
+    let visible_nodes: Vec<&Node> = nodes_iter
+        .clone()
+        .filter(|node| node.is_visible())
+        .collect();
+
+    if visible_nodes.is_empty() {
+        return Ok(());
+    }
+
+    let multiple_visible_nodes = visible_nodes.len() > 1;
+
+    write!(f, " ")?;
+
+    if argument_is_optional {
+        write!(f, "[")?;
+    }
+
+    if multiple_visible_nodes {
+        write!(f, "(")?;
+    }
+
+    for (idx, &node) in visible_nodes.iter().enumerate() {
+        // Print usage of this node
+        write!(f, "{node}")?;
+
+        // Recursively print usage of it's children
+        write_tree(f, nodes, &node.children)?;
+
+        if multiple_visible_nodes && idx != visible_nodes.len() - 1 {
+            write!(f, " | ")?;
+        }
+    }
+
+    if multiple_visible_nodes {
+        write!(f, ")")?;
+    }
+
+    if argument_is_optional {
+        write!(f, "]")?;
+    }
+
+    Ok(())
+}
+
 impl Display for CommandTree {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(f, "/{}", self.names[0])?;
+        // TODO: Help of commands like /bossbar becomes really long
+        //       A possible solution would be to add a check if the first level consists of literals
+        //       only and if so to run the printing as if it were separate commands
 
-        let current_children = &self.children[..];
+        // TODO: Consider adding a max depth to print command usage only up to a certain depth.
+        //       Vanilla seems to do this too (as can be seen with /help effect)
 
-        while !current_children.is_empty() {
-            let flattened = flatten_require_nodes(&self.nodes, current_children);
-
-            let mut visible_iter = flattened
-                .iter()
-                .copied()
-                .filter(|&i| self.nodes[i].is_visible());
-
-            let Some(first_idx) = visible_iter.next() else {
-                break;
-            };
-
-            let second_idx = visible_iter.next();
-
-            f.write_char(' ')?;
-
-            let is_optional = flattened
-                .iter()
-                .any(|&i| matches!(self.nodes[i].node_type, NodeType::ExecuteLeaf { .. }));
-
-            if is_optional {
-                f.write_char('[')?;
-            }
-
-            if let Some(second_idx) = second_idx {
-                f.write_char('(')?;
-                self.nodes[first_idx].fmt(f)?;
-
-                write!(f, " | ")?;
-                self.nodes[second_idx].fmt(f)?;
-
-                for idx in visible_iter {
-                    write!(f, " | ")?;
-                    self.nodes[idx].fmt(f)?;
-                }
-                f.write_char(')')?;
-
-                break;
-            }
-
-            if is_optional {
-                f.write_char(']')?;
-            }
-        }
-
-        Ok(())
+        // Clean up graph by 'flattening' require nodes to their children
+        let flattened = flatten_require_nodes(&self.nodes, &self.children);
+        // Recursively iterate over tree to print help usage of a command
+        write_tree(f, &self.nodes, &flattened)
     }
 }