rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

RFC: Std I/O driven application (aka `amethyst_commands`)

Open azriel91 opened this issue 6 years ago • 7 comments

Issue 999, this is gonna be epic!

Summary

Ability to control an Amethyst application using commands issued through stdin, with human-friendly terminal interaction.

Motivation

Inspecting and manipulating the state1 of an application at run time is a crucial part of development, with at least the following use cases:

  • Determining that the application is behaving as expected.
  • Experimenting with new features.
  • Triggering certain cases.
  • Investigating / troubleshooting unexpected behaviour.
  • Automatically driving the application for integration tests.

A command terminal will greatly reduce the effort to carry out the aforementioned tasks.

1 state here means the runtime values, not amethyst::State

Prior Art

Expand -- copied from #995 (warning: code heavy)

okay, so this post is code heavy, but it's how I've done commands in my game (youtube). It shouldn't force people to use the state machine, since event types are "plug in if you need it".

Crate: stdio_view (probably analogous to amethyst_commands)

  • Reads stdin strings, uses shell_words to parse into separate tokens.
  • Parses the first token into an AppEventVariant to determine which AppEvent the tokens correspond to. On success, it sends a tuple: (AppEventVariant, Vec<String>) (the tokens) to an EventChannel<(AppEventVariant, Vec<String>)>.

Changes if put into Amethyst:

  • StdinSystem would be generic over top level types E and EVariant, which would take in AppEvent and AppEventVariant.

Crate: application_event

  • Contains AppEvent and AppEventVariant.

  • AppEvent is an enum over all custom event types, AppEventVariant is derived from AppEvent, without the fields.

    Example:

    use character_selection_model::CharacterSelectionEvent;
    use map_selection_model::MapSelectionEvent;
    
    #[derive(Clone, Debug, Display, EnumDiscriminants, From, PartialEq)]
    #[strum_discriminants(
        name(AppEventVariant),
        derive(Display, EnumIter, EnumString),
        strum(serialize_all = "snake_case")
    )]
    pub enum AppEvent {
        /// `character_selection` events.
        CharacterSelection(CharacterSelectionEvent),
        /// `map_selection` events.
        MapSelection(MapSelectionEvent),
    }
    

This would be an application specific crate, so it wouldn't go into Amethyst. If I want to have State event control, this will include an additional variant State(StateEvent) from use amethyst_state::StateEvent;, where StateEvent carries the information of what to do (e.g. Pop or Switch).

Crate: stdio_spi

  • StdinMapper is a trait with the following associated types:

    use structopt::StructOpt;
    
    use Result;
    
    /// Maps tokens from stdin to a state specific event.
    pub trait StdinMapper {
        /// Resource needed by the mapper to construct the state specific event.
        ///
        /// Ideally we can have this be the `SystemData` of an ECS system. However, we cannot add
        /// a `Resources: for<'res> SystemData<'res>` trait bound as generic associated types (GATs)
        /// are not yet implemented. See:
        ///
        /// * <https://users.rust-lang.org/t/17444>
        /// * <https://github.com/rust-lang/rust/issues/44265>
        type Resource;
        /// State specific event type that this maps tokens to.
        type Event: Send + Sync + 'static;
        /// Data structure representing the arguments.
        type Args: StructOpt;
        /// Returns the state specific event constructed from stdin tokens.
        ///
        /// # Parameters
        ///
        /// * `tokens`: Tokens received from stdin.
        fn map(resource: &Self::Resource, args: Self::Args) -> Result<Self::Event>;
    }
    

    Args is a T: StructOpt which we can convert the String tokens from before we pass it to the map function. Resource is there because the constructed AppEvent can contain fields that are constructed based on an ECS resource.

  • This crate also provides a generic MapperSystem that reads from EventChannel<(AppEventVariant, Vec<String>)> from the stdio_view crate. If the variant matches the AppEventVariant this system is responsible for, it passes all of the tokens to a T: StdinMapper that understands how to turn them into an AppEvent, given the Resource.

    /// Type to fetch the application event channel.
    type MapperSystemData<'s, SysData> = (
        Read<'s, EventChannel<VariantAndTokens>>,
        Write<'s, EventChannel<AppEvent>>,
        SysData,
    );
    
    impl<'s, M> System<'s> for MapperSystem<M>
    where
        M: StdinMapper + TypeName,
        M::Resource: Default + Send + Sync + 'static,
        AppEvent: From<M::Event>,
    {
        type SystemData = MapperSystemData<'s, Read<'s, M::Resource>>;
    
        fn run(&mut self, (variant_channel, mut app_event_channel, resources): Self::SystemData) {
        // ...
        let args = M::Args::from_iter_safe(tokens.iter())?;
        M::map(&resources, args)
        // ... collect each event
    
        app_event_channel.drain_vec_write(&mut events);
    }
    }
    

Crate: character_selection_stdio (or any other crate that supports stdin -> AppEvent)

  • Implements the stdio_spi.

  • The Args type:

    #[derive(Clone, Debug, PartialEq, StructOpt)]
    pub enum MapSelectionEventArgs {
        /// Select event.
        #[structopt(name = "select")]
        Select {
            /// Slug of the map or random, e.g. "default/eruption", "random".
            #[structopt(short = "s", long = "selection")]
            selection: String,
        },
    }
    
  • The StdinMapper type:

    impl StdinMapper for MapSelectionEventStdinMapper {
        type Resource = MapAssets;         // Read resource from the `World`, I take a `MapHandle` from it
        type Event = MapSelectionEvent;    // Event to map to
        type Args = MapSelectionEventArgs; // Strong typed arguments, rather than the String tokens
    
        fn map(map_assets: &MapAssets, args: Self::Args) -> Result<Self::Event> {
            match args {
                MapSelectionEventArgs::Select { selection } => {
                    Self::map_select_event(map_assets, &selection)
                }
            }
        }
    }
    
  • The bundle, which adds a MapperSystem<MapSelectionEventStdinMapper>:

    builder.add(
        MapperSystem::<MapSelectionEventStdinMapper>::new(AppEventVariant::MapSelection),
        &MapperSystem::<MapSelectionEventStdinMapper>::type_name(),
        &[],
    );
    

Can use it as inspiration to drive the design, or I'm happy to push my code up for the reusable parts (stdio_spi should be usable as is, stdio_view probably needs a re-write).

Detailed Design

TODO: discuss

Alternatives

The amethyst-editor will let you do some of the above tasks (inspecting, manipulating entities and components). It doesn't cater for:

  • Server side inspection (e.g. SSH to an application running on a headless server)
  • Automated tests
  • Easy repeatability (source controlled actions)

azriel91 avatar Oct 06 '18 08:10 azriel91

Previous comments #995:

The amethyst_terminal crate which I will be adding will need to reference the Trans struct. This cannot be done while this struct is in the base crate, as the base crate depends on all other sub crates (cyclic dependency). Then I need to move everything else for the same reason. StateMachine -> (Trans -> State) -> StateEvent and its not possible to go back up the crate hierarchy

– @jojolepro

Why exactly does terminal need to depend on Trans? Can't you make a command system that is generic enough for that ?

– @Rhuagh

we would like to keep the state machine an optional component.

– @torkleyy

re: amethyst_terminal Trans dependency The amethyst_terminal crate is the parser that converts the command string into the concrete types that need to be sent into the EventChannels. For example, the command

$ statechange "MainState"

will try to parse the string as ron data, and if it succeeds, it will push a Trans instance through the global EventChannel.

I'm not sure how I could make that generic enough, considering all commands have a different way of being parsed, some have side effects, etc.

Most commands will be in the amethyst_command crate, because that crate will be access by most other crates like core::TransformSystem to check for "Teleportation" events for example. For the Trans command however, it cannot be inside of amethyst_commands, because that will create a cyclic dependency (almost everything will depend on amethyst_commands, so it cannot depend on anything in return).

The StateMachine will be behind a feature gate. I forgot to include it in the list I made yesterday on discord.

– @jojolepro

StateEvent carries the information of what to do (e.g. Pop or Switch). – @azriel91

That proves why this PR is necessary.

The design you have is effectively what I have on paper. Following rhuagh comments on discord however, the enum containing the different commands will be replaced by something more dynamic. See the opened PR talking about state handle_event made by rhuagh

– @jojolepro

In short, the command crate should be generic enough so different frameworks (e.g. state machine) can register themselves. I like the current structure because it forces us to properly design things and to not couple stuff too much.

– @torkleyy

azriel91 avatar Oct 06 '18 08:10 azriel91

Thanks for opening the issue!

Ideally, the editor and the command crate would operate over the same interface. We should try to get one interface that's useful for the editor, the command crate, scripting, modding, etc. because otherwise this will get very chaotic.

torkleyy avatar Oct 06 '18 08:10 torkleyy

I was planning on having three frontends (or more) for the command stuff:

  • From the terminal (terminal duplex)
  • From the editor (via networking, also adds rpc support)
  • From the in-game ui

AnneKitsune avatar Oct 06 '18 14:10 AnneKitsune

@jojolepro "in-game ui"? You mean the one for editing, right?

Independent of that, it should be generic enough so crates can register themselves instead of depending on all of them.

torkleyy avatar Oct 06 '18 14:10 torkleyy

amethyst_ui? :P

AnneKitsune avatar Oct 06 '18 15:10 AnneKitsune

By making a generic framework where anything can register their commands we also upon up for the user to have terminal commands added from the game logic.

Rhuagh avatar Oct 12 '18 23:10 Rhuagh

Transferring this to the RFC repo.

fhaynes avatar Jan 08 '19 02:01 fhaynes