feather-rs / lieutenant

Command dispatcher for Rust based on Mojang's Brigadier
Apache License 2.0
2 stars 4 forks source link

Perform major refactoring #10

Closed caelunshun closed 4 years ago

caelunshun commented 4 years ago

This PR resolves a number of problems I've had with the library.

Additionally, I added some documentation here and there.

Defman commented 4 years ago

Removing async would be a bad idea, how would we handle async parsers, checkers, and providers? This would make Io impossible in parsers, checkers, and providers and it is a must.

caelunshun commented 4 years ago

Provided types, parsed arguments, etc. can implement Future, and then the command itself can spawn a task to complete those Futures.

Defman commented 4 years ago

But that would not allow for branching, based on the results since errors returned by exec is not treated the same as for parsers and checkers?

caelunshun commented 4 years ago

For example:

struct DbAccess;

impl Future for DbAccess {
    type Output = u32;
    fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<Self::Output> {
        // some asynchronous computation
        Poll::Ready(10)
    }
}
caelunshun commented 4 years ago

Yes, it will allow for branching as per usual. Example:

#[command(usage = "access_db")]
fn cmd(provided_u32: DbAccess) {
    task::spawn(async move {
        match provided_u32.await {
           1 => println!("one"),
           x => println!("{}", x),
        }
    });
}
Defman commented 4 years ago

We would then on feather side do something like this, to resume dispatching.

let command = "access_db";
let nodes = Vec::new(); // Somehow need to intilize nodes with the root nodes children
let errors = Vec::new();
while !nodes.is_empty() {
    match dispatcher.dispatch(&mut nodes, &mut errors, command) {
        Ok(future) => match future.await {
            Ok(ok) => break, // Maybe do something with the result
            Err(err) => errors.push(err),
        },
    }
}
// Handle erros

I would like to add that Minecraft commands are async by nature, they are dispatched by chat which is an async event and has nothing to do with the main game loop. Many commands do however affect the world or game loop and as such has to merge into it.

Defman commented 4 years ago

I think we should implement the commands dispatcher as an NFA. Each step in the NFA would be a parser and each state may be final(has an exec) or non-final. This is very similar to what we currently have.

The final product would be regex but with support for custom patterns(parsers), these may be IO-bound/async, since the pattern could be based on a file. Regex is an NFA but is often translated into a DFA for performance, it would be infeasible for us to do the same due to our custom patterns(parsers) may be dynamic and we would have to build it for each call and the corresponding DFA could have infinite size.

Our current setup where each nodes checker parses the input and when we execute the commands we parse the entire input a second-time could result in inconsistencies, ie the player leaves in the meantime (async). We can get around this by the checkers determining if the input meets the minimum requirement for the parser to work. Async parsed arguments should then return a future that can be polled from the exec.

#[usage = "command <target>"]
fn command(target: PlayerProfile) -> ... {
  tasks::spawn(async {
    let target = target.await?;
  }
}

If the poll of the target fails we should resume our the dispatching from the state which the exec is attached.

caelunshun commented 4 years ago

Will merge this as we decided on Discord it's a good common baseline for what's next.