amethyst / rfcs

RFCs are documents that contain major plans and decisions for the engine
Apache License 2.0
32 stars 10 forks source link

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

Open azriel91 opened 6 years ago

azriel91 commented 6 years ago

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:

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](https://youtu.be/jpk2MTeWz3w)). 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)` (the tokens) to an `EventChannel<(AppEventVariant, Vec)>`. 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: ```rust 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: ```rust 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: /// /// * /// * 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; } ``` `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)>` 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`. ```rust /// Type to fetch the application event channel. type MapperSystemData<'s, SysData> = ( Read<'s, EventChannel>, Write<'s, EventChannel>, SysData, ); impl<'s, M> System<'s> for MapperSystem where M: StdinMapper + TypeName, M::Resource: Default + Send + Sync + 'static, AppEvent: From, { 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: ```rust #[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: ```rust 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 { match args { MapSelectionEventArgs::Select { selection } => { Self::map_select_event(map_assets, &selection) } } } } ``` * The bundle, which adds a `MapperSystem`: ```rust builder.add( MapperSystem::::new(AppEventVariant::MapSelection), &MapperSystem::::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:

azriel91 commented 6 years ago

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

torkleyy commented 6 years ago

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.

AnneKitsune commented 6 years ago

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

torkleyy commented 6 years ago

@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.

AnneKitsune commented 6 years ago

amethyst_ui? :P

Rhuagh commented 6 years ago

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.

fhaynes commented 5 years ago

Transferring this to the RFC repo.