2
\$\begingroup\$

I'm a C/C++ programmer and this is my first Rust program. A heavy simplification of the console system like used in Quake and Source engine games, that lets you set variables and run commands.

I'm aware the parsing algorithm is bad, it's just supposed to be simple. I want to know how I can improve this code to make it less ugly or more idiomatic Rust. I think I'm still writing with a leftover C++ mindset and Rust does not like that. Even for something this simple I had to fight the compiler (see the comment // can't print immediately because of borrowing rules) over wanting to print a console message in the middle of a console function. (I didn't want to duplicate the code in the message function, that'd be ugly and wouldn't work in a console with a more complex message function.)

Usually in my C++ programs I have Console as just a global state namespace, not an object, but that seems like a bad idea to even try in Rust. However with Console as an object, ConCommand callbacks aren't able to use the message method because they have no handle to Console. Every part of a large game would need a handle to Console just for debug messages. I'd also like an easier way of registering newly-created variables/commands with the Console. Is there something better I can do on that front as well?

console.rs:

pub struct ConVar {
    pub name: String,
    pub value: String,
}

pub struct ConCommand {
    pub name: String,
    pub callback: fn(args: &[String])
}

pub struct Console {
    // line history, for rendering if used in a gui program, or a dump lines to file command
    pub history: Vec<String>,
    pub commands: Vec<ConCommand>,
    pub variables: Vec<ConVar>
}

impl Console {
    pub fn new() -> Console {
        Console {
            history: Vec::new(),
            commands: Vec::new(),
            variables: Vec::new()
        }
    }

    pub fn print(&mut self, msg: String) {
        println!("CONSOLE: {}", msg);
        self.history.push(msg);
    }

    pub fn execute(&mut self, args: &[String]) {
        if args.is_empty() {
            return;
        }

        let mut msg = String::new();
        for i in self.variables.iter_mut() {
            if i.name == args[0] {
                if args.len() > 1 {
                    i.value = args[1].clone();
                    return;
                } else {
                    // can't print immediately because of borrowing rules
                    msg = format!("\"{}\" is \"{}\"", i.name, i.value);
                    break;
                }
            }
        }
        if !msg.is_empty() {
            self.print(msg);
            return;
        }

        for i in self.commands.iter_mut() {
            if i.name == args[0] {
                (i.callback)(args);
                return;
            }
        }

        self.print(format!("Unknown command \"{}\"", args[0]));
    }

    pub fn parse(&mut self, str: String) {
        if str.is_empty() {
            return;
        }

        let mut args: Vec<String> = Vec::new();
        for c in str.chars() {
            match c {
                '\n' => { continue; }
                '\r' => { continue; }
                ' ' => {
                    if args.len() > 0 {
                        args.push(String::from(""));
                    }
                }
                _ => {
                    let index = args.len();
                    if index > 0 {
                        if let Some(s) = args.get_mut(index - 1) {
                            s.push(c);
                        } else {
                            args.push(String::from(c));
                        }
                    } else {
                        args.push(String::from(c));
                    }
                }
            }
        }

        args.retain(|s| { !(s.is_empty()) });
        if args.len() == 0 {
            return;
        }

        self.execute(args.as_slice());
    }
}

main.rs:

mod console;

use console::*;
use std::io::Write;

fn echo(args: &[String]) {
    if args.len() < 2 {
        println!("Usage: echo <message>");
        return;
    }
    println!("{}", args[1]);
}

fn main() {
    let mut con = Console::new();

    let cvar = ConVar {
        name: "testvar".to_owned(),
        value: "default".to_owned(),
    };
    con.variables.push(cvar);

    let cmd = ConCommand {
        name: "echo".to_owned(),
        callback: echo,
    };
    con.commands.push(cmd);

    loop {
        print!(">");
        std::io::stdout().flush().unwrap(); // show the prompt
        let mut line = String::new();
        match std::io::stdin().read_line(&mut line) {
            Ok(_) => con.parse(line),
            Err(e) => println!("Err: {}", e)
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$
  1. Use std::collections::HashMap

Your variables and functions are stored in a Vec<ConVar> or Vec<ConCommand>, instead use a HashMap<String, String> and HashMap<String, fn(&[String]>. Then you can readily use the get, insert, and entry apis to simplify your code.

  1. Your parse function duplicates a lot of functionality that already built into Rust.

Your parse function is pretty much equivalent to:

let args: Vec<String> = str.trim().split('_').map(str::to_owned).collect();
  1. can't print immediately because of borrowing rules:

     for i in self.variables.iter_mut() {
         if i.name == args[0] {
             if args.len() > 1 {
                 i.value = args[1].clone();
                 return;
             } else {
                 // can't print immediately because of borrowing rules
                 msg = format!("\"{}\" is \"{}\"", i.name, i.value);
                 break;
             }
         }
     }
    

There are a few options here.

Part of the problem is that the loop is doing very different things depending on the length of args. In particular, one branch modifies variables while the other prints it. If you move the condition the out level, then it resolves the problem:

    if args.len() > 1 {
        for i in self.variables.iter_mut() {
            if i.name == args[0] {
                i.value = args[1].clone();
                return;
            }
        }
    } else {
        for i in self.variables.iter() {
            if i.name == args[0] {
                self.print(format!("\"{}\" is \"{}\"", i.name, i.value));
                return;
            }
        }
    }

This duplicates the loop, but if you use a hashmap that will be minimal.

Alternately, the problem is that Rust doesn't realize that print only modifies history and not variables. This can be resolved by moving history into its own struct.

pub struct Printer {
    pub history: Vec<String>
}

impl Printer {
    fn new() -> Printer {
        Printer {
            history: Vec::new()
        }
    }
    
    fn print(&mut self, msg: String) {
        println!("CONSOLE: {}", msg);
        self.history.push(msg);
    }
}

impl Console {
   pub fn new() -> Console {
       Console {
           printer: Printer::new(),
           commands: Vec::new(),
           variables: Vec::new()
       }
   }
   ...
}

Now you can invoke self.printer.print and Rust will realize that self.variables and self.printer can be mutated separately.

Usually in my C++ programs I have Console as just a global state namespace, not an object, but that seems like a bad idea to even try in Rust. However with Console as an object, ConCommand callbacks aren't able to use the message method because they have no handle to Console

My recommendation is that you create a Game struct. (Assuming this would be part of some larger game). It would represent your whole game and contain all of the state. In particular, anything you might make global in C++ would live in it.

struct Game {
    console: Console,
    ...
}

Then when something needs access to the whole system, you just it a reference to Game. From there it can access any part of the system that it needs. As a generic callback, the ConCommand should take a &mut Game and thus have access to anything it needs.

I'd also like an easier way of registering newly-created variables/commands with the Console. Is there something better I can do on that front as well?

I'm not sure what to advise, since I'm not sure what you find difficult about your current approach.

\$\endgroup\$
2
  • \$\begingroup\$ Why is HashMap a better alternative than ConVar? other than get, insert, and entry. Is there any speed improvement? \$\endgroup\$ – Eka Apr 21 at 6:32
  • 1
    \$\begingroup\$ @Eka, for a large enough number of commands or variables, it will be faster to use a hashmap. However, in this use case, I'm not sure there is enough. The primary reason is just because the interface is nicer if you are doing lookups. \$\endgroup\$ – Winston Ewert Apr 21 at 20:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.