Improve error handling and hints and stuff

src/builtin_parser.rs

@@ -15,7 +15,7 @@ pub(crate) mod lexer;
 pub(crate) mod parser;
 pub(crate) mod runner;
 
-pub use runner::{environment::Environment, unique_rc::*, RunError, Value};
+pub use runner::{environment::Environment, error::RunError, unique_rc::*, Value};
 
 /// Wrapper around `T` that stores a [Span] (A location in the source code)
 #[derive(Debug, Clone)]

src/builtin_parser/runner.rs

@@ -4,9 +4,10 @@ use std::{cell::RefCell, collections::HashMap, rc::Rc};
 
 use environment::Environment;
 
-use crate::command::{CommandHint, CommandHintColor, CommandHints};
+use crate::{command::CommandHints, ui::COMMAND_RESULT_NAME};
 
 use self::{
+    error::RunError,
     reflection::{object_to_dynamic_struct, CreateRegistration, IntoResource},
     unique_rc::{UniqueRc, WeakRef},
 };
@@ -19,8 +20,9 @@ use bevy::{
     prelude::*,
     reflect::{DynamicEnum, Enum, ReflectMut, TypeInfo, TypeRegistration},
 };
-use logos::Span;
+
 pub mod environment;
+pub mod error;
 pub mod reflection;
 pub mod stdlib;
 pub mod unique_rc;
@@ -35,34 +37,6 @@ pub struct EvalParams<'world, 'env, 'reg> {
     registrations: &'reg [&'reg TypeRegistration],
 }
 
-/// An error occuring during the while executing the [`AST`](Ast) of the command.
-#[derive(Debug)]
-pub enum RunError {
-    /// A custom text message. Contains very little contextual information, try to find an existing error instead.
-    Custom {
-        /// The text of the message
-        text: String,
-        span: Span,
-    },
-    VariableNotFound(Span),
-    ExpectedNumberAfterUnaryOperator(Value),
-    InvalidVariantForResource(String, String),
-    CannotIndexValue(Span),
-    FieldNotFoundInStruct(Span),
-    CouldntDereferenceValue(Span),
-    ReferenceToMovedData(Span),
-    VariableMoved(Span),
-    CannotBorrowValue(Span),
-    IncompatibleReflectTypes {
-        expected: String,
-        actual: String,
-    },
-    EnumVariantNotFound {
-        name: String,
-    },
-    CannotMoveOutOfResource(Spanned<String>),
-}
-
 pub fn run(ast: Ast, world: &mut World) {
     // Temporarily remove the [`Environment`] resource to gain
     // mutability without needing a mutable reference.
@@ -118,17 +92,16 @@ pub fn run(ast: Ast, world: &mut World) {
             match value {
                 Ok(Value::None) => {}
                 Ok(value) => match value.try_format(span, world, &registrations) {
-                    Ok(value) => info!(name: "console_result", "> {value}"),
+                    Ok(value) => info!(name: COMMAND_RESULT_NAME, "> {value}"),
                     Err(err) => error!("{err:?}"),
                 },
-                Err(err) => error!("{err:?}"),
+                Err(err) => {
+                    hints.push(err.hints());
+
+                    error!("{}", err.message());
+                }
             }
         }
-        hints.push([CommandHint::new(
-            3..4,
-            CommandHintColor::Error,
-            "woah description",
-        )]);
     }
 
     // Add back the resources
@@ -192,13 +165,15 @@ fn eval_expression(
                         let TypeInfo::Enum(enum_info) = registeration.type_info() else {
                             unreachable!();
                         };
-                        match value_expr.value {
+                        let Spanned { span, value } = *value_expr;
+                        match value {
                             Expression::Variable(variable) => {
                                 if enum_info.contains_variant(&variable) {
                                     let new_enum = DynamicEnum::new(variable, ());
 
                                     dyn_enum.set(Box::new(new_enum)).map_err(|new_enum| {
                                         RunError::IncompatibleReflectTypes {
+                                            span,
                                             expected: dyn_enum.variant_name().to_string(),
                                             actual: new_enum
                                                 .downcast::<DynamicEnum>()
@@ -208,7 +183,10 @@ fn eval_expression(
                                         }
                                     })
                                 } else {
-                                    Err(RunError::EnumVariantNotFound { name: variable })
+                                    Err(RunError::EnumVariantNotFound {
+                                        name: variable,
+                                        span,
+                                    })
                                 }?
                             }
                             Expression::StructObject { name, map } => {
@@ -244,6 +222,7 @@ fn eval_expression(
                         }
                     }
                     _ => {
+                        let span = value_expr.span.clone();
                         let value = eval_expression(
                             *value_expr,
                             EvalParams {
@@ -262,6 +241,7 @@ fn eval_expression(
 
                         reflect.set(value_reflect).map_err(|value_reflect| {
                             RunError::IncompatibleReflectTypes {
+                                span,
                                 expected: reflect.reflect_type_path().to_string(),
                                 actual: value_reflect.reflect_type_path().to_string(),
                             }
@@ -334,19 +314,23 @@ fn eval_expression(
                 registrations,
             },
         ),
-        Expression::UnaryOp(expr) => {
-            let expr = eval_expression(
-                *expr,
+        Expression::UnaryOp(sub_expr) => {
+            let span = sub_expr.span.clone();
+            let value = eval_expression(
+                *sub_expr,
                 EvalParams {
                     world,
                     environment,
                     registrations,
                 },
             )?;
 
-            match expr {
+            match value {
                 Value::Number(number) => Ok(Value::Number(-number)),
-                _ => Err(RunError::ExpectedNumberAfterUnaryOperator(expr)),
+                _ => Err(RunError::ExpectedNumberAfterUnaryOperator(Spanned {
+                    span,
+                    value,
+                })),
             }
         }
         Expression::StructObject { name, map } => {

src/builtin_parser/runner/environment.rs

@@ -7,9 +7,10 @@ use logos::Span;
 
 use super::{
     super::{parser::Expression, Spanned},
+    error::RunError,
     eval_expression, stdlib,
     unique_rc::UniqueRc,
-    EvalParams, RunError, Value,
+    EvalParams, Value,
 };
 
 /// Macro for mass registering functions.

src/builtin_parser/runner/error.rs

@@ -0,0 +1,95 @@
+use std::borrow::Cow;
+
+use logos::Span;
+
+use crate::{
+    builtin_parser::Spanned,
+    command::{CommandHint, CommandHintColor},
+};
+
+use super::Value;
+
+/// An error occuring during the while executing the [`AST`](Ast) of the command.
+#[derive(Debug)]
+pub enum RunError {
+    /// A custom text message. Contains very little contextual information, try to find an existing error instead.
+    Custom {
+        /// The text of the message
+        text: String,
+        span: Span,
+    },
+    VariableNotFound(Span),
+    ExpectedNumberAfterUnaryOperator(Spanned<Value>),
+    InvalidVariantForResource(String, String),
+    CannotIndexValue(Span),
+    FieldNotFoundInStruct(Span),
+    CouldntDereferenceValue(Span),
+    ReferenceToMovedData(Span),
+    VariableMoved(Span),
+    CannotBorrowValue(Span),
+    IncompatibleReflectTypes {
+        expected: String,
+        actual: String,
+        span: Span,
+    },
+    EnumVariantNotFound {
+        name: String,
+        span: Span,
+    },
+    CannotMoveOutOfResource(Spanned<String>),
+}
+
+impl RunError {
+    pub fn spans(&self) -> Vec<Span> {
+        use RunError::*;
+
+        match self {
+            Custom { span, .. } => vec![span.clone()],
+            VariableNotFound(span) => vec![span.clone()],
+            ExpectedNumberAfterUnaryOperator(Spanned { span, .. }) => vec![span.clone()],
+            InvalidVariantForResource(_, _) => todo!(),
+            CannotIndexValue(span) => vec![span.clone()],
+            FieldNotFoundInStruct(span) => vec![span.clone()],
+            CouldntDereferenceValue(span) => vec![span.clone()],
+            ReferenceToMovedData(span) => vec![span.clone()],
+            VariableMoved(span) => vec![span.clone()],
+            CannotBorrowValue(span) => vec![span.clone()],
+            IncompatibleReflectTypes { span, .. } => vec![span.clone()],
+            EnumVariantNotFound { span, .. } => vec![span.clone()],
+            CannotMoveOutOfResource(Spanned { span, .. }) => vec![span.clone()],
+        }
+    }
+    pub fn hints(&self) -> Vec<CommandHint> {
+        self.spans()
+            .into_iter()
+            .map(|span| CommandHint::new(span, CommandHintColor::Error, "todo"))
+            .collect()
+    }
+    pub fn message(&self) -> Cow<'static, str> {
+        use RunError::*;
+
+        match self {
+            Custom { text, .. } => text.clone().into(),
+            VariableNotFound(_) => "Variable not found.".into(),
+            ExpectedNumberAfterUnaryOperator(Spanned { value, .. }) => format!(
+                "Expected a number after unary operator (-) but got {} instead.",
+                value.kind()
+            )
+            .into(),
+            InvalidVariantForResource(_, _) => todo!(),
+            CannotIndexValue(_) => todo!(),
+            FieldNotFoundInStruct(_) => todo!(),
+            CouldntDereferenceValue(_) => todo!(),
+            ReferenceToMovedData(_) => todo!(),
+            VariableMoved(_) => todo!(),
+            CannotBorrowValue(_) => todo!(),
+            IncompatibleReflectTypes {
+                expected,
+                actual,
+                span,
+            } => todo!(),
+            EnumVariantNotFound { name, span } => todo!(),
+            CannotMoveOutOfResource(Spanned { value, .. }) => format!("Cannot move out of resource `{value}`, try borrowing it instead.").into(),
+        }
+    }
+}

src/builtin_parser/runner/stdlib.rs

@@ -2,7 +2,7 @@ use crate::register;
 use bevy::{ecs::world::World, log::info, reflect::TypeRegistration};
 use std::{cell::Ref, ops::Range};
 
-use super::{Environment, RunError, Spanned, Value};
+use super::{error::RunError, Environment, Spanned, Value};
 
 fn print(
     value: Spanned<Value>,

src/builtin_parser/runner/value.rs

@@ -7,7 +7,7 @@ use crate::builtin_parser::{Environment, StrongRef};
 use super::environment::FunctionParam;
 use super::reflection::{CreateRegistration, IntoResource};
 use super::unique_rc::WeakRef;
-use super::{super::Spanned, RunError};
+use super::{super::Spanned, error::RunError};
 
 use bevy::ecs::world::World;
 use bevy::reflect::{
@@ -19,7 +19,6 @@ use logos::Span;
 
 /// A runtime value
 #[derive(Debug)]
-#[non_exhaustive]
 pub enum Value {
     /// Nothing at all
     None,
@@ -131,6 +130,19 @@ impl Value {
             Value::Resource(resource) => Ok(fancy_debug_print(resource, world, registrations)),
         }
     }
+
+    pub fn kind(&self) -> &'static str {
+        match self {
+            Value::None => "nothing",
+            Value::Number(_) => "a number",
+            Value::Boolean(_) => "a boolean",
+            Value::String(_) => "a string",
+            Value::Reference(_) => "a reference",
+            Value::Object(_) => "a object",
+            Value::StructObject { .. } => "a struct object",
+            Value::Resource(_) => "a resource",
+        }
+    }
 }
 /// A massive function that takes in a type registration and the world and then
 /// does all the hard work of printing out the type nicely.

src/command.rs

@@ -29,6 +29,7 @@ impl From<Box<dyn CommandParser>> for DefaultCommandParser {
 }
 
 /// A hint displayed to the user when they make a mistake.
+#[derive(Debug, Clone)]
 pub struct CommandHint {
     /// The color of the hint.
     pub color: CommandHintColor,
@@ -53,6 +54,7 @@ impl CommandHint {
 }
 
 /// The color of a [`CommandHint`], may either be a standard color or a [`Custom`](CommandHintColor::Custom) [`Color`].
+#[derive(Debug, Clone)]
 pub enum CommandHintColor {
     /// An error marks bad code that cannot be recovered from.
     ///
@@ -78,11 +80,30 @@ pub enum CommandHintColor {
 /// A resource where hints (errors/warnings/etc) are stored
 /// to be displayed in the developer console.
 #[derive(Resource, Default, Deref)]
-pub struct CommandHints(Vec<Vec<CommandHint>>);
+pub struct CommandHints {
+    #[deref]
+    hints: Vec<Vec<CommandHint>>,
+    hint_added: bool,
+}
 impl CommandHints {
-    /// Push a list of hints. This should be done once per command call (even if theres no hints).
+    /// Push a list of hints. This should be done once per command call.
     pub fn push(&mut self, hints: impl Into<Vec<CommandHint>>) {
-        self.0.push(hints.into());
+        if self.hint_added {
+            warn!(
+                "Hints were added twice! Hint 1: {:?}, Hint 2: {:?}",
+                self.hints.last(),
+                hints.into()
+            )
+        } else {
+            self.hints.push(hints.into());
+        }
+    }
+    pub(crate) fn reset_hint_added(&mut self) {
+        if self.hint_added {
+            self.hint_added = false;
+        } else {
+            self.push([]);
+        }
     }
 }
 
@@ -97,10 +118,10 @@ impl CommandHints {
 /// pub struct MyCustomParser;
 /// impl CommandParser for MyCustomParser {
 ///     fn parse(&self, command: &str, world: &mut World) {
-///         // The `name: "console_result"` tells the console this is a result from
+///         // The `name: COMMAND_RESULT_NAME` tells the console this is a result from
 ///         // the parser and then formats it accordingly.
 ///         // TODO: figure out better solution for this
-///         info!(name: "console_result", "You just entered the command {command}")
+///         info!(name: COMMAND_RESULT_NAME, "You just entered the command {command}")
 ///     }
 /// }
 /// ```