From ed37d1239ffa3ff5656debfdf881333330fd582c Mon Sep 17 00:00:00 2001 From: Soispha Date: Sat, 9 Sep 2023 19:49:56 +0200 Subject: [PATCH] feat(CommandTransferValue): Generalize the API to facilitate multiple languages The code for the `CommandTransferValue` was deeply entrenched in the whole Lua execution code, which is obviously not ideal. In trying to alleviate that, I decided to generalize the API in a way, that makes adding new languages a lot easier. --- .../lua_wrapper/rust_wrapper_functions/mod.rs | 62 +----------- .../command_transfer_value/lua.rs | 94 +++++++++++++++++++ .../command_transfer_value/mod.rs | 60 ++++++++++++ .../type_conversions.rs | 34 +++++++ .../lua_command_manager/mod.rs | 91 +----------------- src/app/command_interface/mod.rs | 10 +- .../event_types/event/handlers/command.rs | 42 ++++++--- src/app/events/event_types/event/mod.rs | 2 +- 8 files changed, 228 insertions(+), 167 deletions(-) create mode 100644 src/app/command_interface/command_transfer_value/lua.rs create mode 100644 src/app/command_interface/command_transfer_value/mod.rs create mode 100644 src/app/command_interface/command_transfer_value/type_conversions.rs diff --git a/language_macros/src/generate/lua_wrapper/rust_wrapper_functions/mod.rs b/language_macros/src/generate/lua_wrapper/rust_wrapper_functions/mod.rs index 71d204b..785b886 100644 --- a/language_macros/src/generate/lua_wrapper/rust_wrapper_functions/mod.rs +++ b/language_macros/src/generate/lua_wrapper/rust_wrapper_functions/mod.rs @@ -126,67 +126,7 @@ fn get_function_body(field: &Field, has_input: bool, output_type: &Option) let function_return = if let Some(_) = output_type { quote! { - let converted_output = lua - .to_value(&output) - .expect("This conversion should (indirectely) be checked at compile time"); - if let mlua::Value::Table(table) = converted_output { - let real_output: mlua::Value = match output { - CommandTransferValue::Nil => table - .get("Nil") - .expect("This should exist"), - CommandTransferValue::Boolean(_) => table - .get("Boolean") - .expect("This should exist"), - CommandTransferValue::Integer(_) => table - .get("Integer") - .expect("This should exist"), - CommandTransferValue::Number(_) => table - .get("Number") - .expect("This should exist"), - CommandTransferValue::String(_) => table - .get("String") - .expect("This should exist"), - CommandTransferValue::Table(_) => { - todo!() - // FIXME(@Soispha): This returns a table with the values wrapped the - // same way the values above are wrapped. That is (from the greet_multiple - // function): - // ```json - // { - // "Table": { - // "UserName1": { - // "Integer": 2 - // } - // } - // } - // ``` - // whilst the output should be: - // ```json - // { - // "UserName1": 2 - // } - // ``` - // That table would need to be unpacked, but this requires some recursive - // function, which seems not very performance oriented. - // - // My first (quick) attempt: - //let mut output_table = lua.create_table().expect("This should work?"); - //let initial_table: mlua::Value = table - // .get("Table") - // .expect("This should exist"); - //while let mlua::Value::Table(table) = initial_table { - // for pair in table.pairs() { - // let (key, value) = pair.expect("This should also work?"); - // output_table.set(key, value); - // } - //} - }, - }; - return Ok(real_output); - } else { - unreachable!("Lua serializes these things always in a table"); - } - + return Ok(output.into_lua(lua).expect("This conversion should always work")); } } else { quote! { diff --git a/src/app/command_interface/command_transfer_value/lua.rs b/src/app/command_interface/command_transfer_value/lua.rs new file mode 100644 index 0000000..e4a75ad --- /dev/null +++ b/src/app/command_interface/command_transfer_value/lua.rs @@ -0,0 +1,94 @@ +use cli_log::{debug, info}; +use mlua::{IntoLua, LuaSerdeExt, Table, Value}; + +use super::CommandTransferValue; + +impl<'lua> IntoLua<'lua> for CommandTransferValue { + fn into_lua(self, lua: &'lua mlua::Lua) -> mlua::Result> { + let converted_output = lua.to_value(&self)?; + return unwrap(converted_output, lua); + } +} +fn unwrap<'lua>( + value_to_unwrap: Value<'lua>, + lua: &'lua mlua::Lua, +) -> mlua::Result> { + fn unwrap_first_level<'lua>(table: Table<'lua>) -> mlua::Result> { + let (_, value): (Value, Value) = table + .pairs() + .next() + .expect("Exactly one item should extist")?; + Ok(value) + } + // That converted value looks somewhat like this (e.g. a String): + // ``` + // { + // ["String"] = "hi", + // } + // ``` + // or like this (e.g. a table): + // ``` + // { + // ["Table"] = { + // ["UserId"] = { + // ["Integer"] = 2, + // }, + // ["UserName"] = { + // ["String"] = "James", + // }, + // }, + // } + // ``` + if let Value::Table(table) = value_to_unwrap { + let value = unwrap_first_level(table)?; + if let Value::Table(wrapped_table) = value { + info!("We've got a wtable! wtable: \n{:#?}", wrapped_table); + // we now have a wrapped table value for example like this: + // ``` + // { + // ["UserId"] = { + // ["Integer"] = 2, + // }, + // ["UserName"] = { + // ["String"] = "James", + // }, + // ["Versions"] = { + // ["Table"] = { + // ["api"] = { + // ["Boolean"] = true, + // }, + // ["interface"] = { + // ["Integer"] = 3, + // }, + // }, + // }, + // } + // ``` + let output_table: Table = lua + .load("{}") + .eval() + .expect("This is static, it should always work"); + + // FIXME(@soispha): This still fails for nested tables (i.e. the table above), as it + // unpacks too much. While unpacking the while loop should stop, when a key is not from + // the CommandTransferValue family (i.e. ["Integer", "Boolean", "String", "Table", + // etc.]) <2023-09-09> + for pair in wrapped_table.pairs::() { + let (key, mut raw_value) = pair?; + while let Value::Table(raw_table) = raw_value { + raw_value = unwrap_first_level(raw_table)?; + } + output_table.set(key, raw_value)?; + } + + return Ok(output_table.into_lua(lua)?); + } else { + info!("We've got a normal output! output: {:#?}", value); + // we had a simple wrapped value, which is already unwrapped, thus it can be + // returned directly + return Ok(value); + } + } else { + unreachable!("The returned table should always only contain one element"); + } +} diff --git a/src/app/command_interface/command_transfer_value/mod.rs b/src/app/command_interface/command_transfer_value/mod.rs new file mode 100644 index 0000000..44e16fc --- /dev/null +++ b/src/app/command_interface/command_transfer_value/mod.rs @@ -0,0 +1,60 @@ +use std::{collections::HashMap, fmt::Display}; + +use serde::{Deserialize, Serialize}; + +pub mod type_conversions; + +// language support +pub mod lua; + +pub type Table = HashMap; + +#[derive(Debug, Serialize, Deserialize, Clone)] +pub enum CommandTransferValue { + /// `nil` or `null` or `undefined`; anything which goes in that group of types. + Nil, + + /// `true` or `false`. + Boolean(bool), + + // A “light userdata” object, equivalent to a raw pointer. + // /* TODO */ LightUserData(LightUserData), + /// An integer number. + Integer(i64), + + /// A floating point number. + Number(f64), + + /// A string + String(String), + + /// A table, dictionary or HashMap + Table(HashMap), + // Reference to a Lua function (or closure). + // /* TODO */ Function(Function), + + // Reference to a Lua thread (or coroutine). + // /* TODO */ Thread(Thread<'lua>), + + // Reference to an userdata object that holds a custom type which implements `UserData`. + // Special builtin userdata types will be represented as other `Value` variants. + // /* TODO */ UserData(AnyUserData), + + // `Error` is a special builtin userdata type. When received from Lua it is implicitly cloned. + // /* TODO */ Error(Error), +} + +impl Display for CommandTransferValue { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + CommandTransferValue::Nil => f.write_str("Nil"), + CommandTransferValue::Boolean(bool) => f.write_str(&format!("{}", bool)), + CommandTransferValue::Integer(int) => f.write_str(&format!("{}", int)), + CommandTransferValue::Number(num) => f.write_str(&format!("{}", num)), + CommandTransferValue::String(str) => f.write_str(&format!("{}", str)), + // TODO(@Soispha): The following line should be a real display call, but how do you + // format a HashMap? + CommandTransferValue::Table(table) => f.write_str(&format!("{:#?}", table)), + } + } +} diff --git a/src/app/command_interface/command_transfer_value/type_conversions.rs b/src/app/command_interface/command_transfer_value/type_conversions.rs new file mode 100644 index 0000000..0d4ea2f --- /dev/null +++ b/src/app/command_interface/command_transfer_value/type_conversions.rs @@ -0,0 +1,34 @@ +use std::collections::HashMap; + +use super::CommandTransferValue; + +impl From for CommandTransferValue { + fn from(s: String) -> Self { + Self::String(s.to_owned()) + } +} +impl From for CommandTransferValue { + fn from(s: f64) -> Self { + Self::Number(s.to_owned()) + } +} +impl From for CommandTransferValue { + fn from(s: i64) -> Self { + Self::Integer(s.to_owned()) + } +} +impl From> for CommandTransferValue { + fn from(s: HashMap) -> Self { + Self::Table(s.to_owned()) + } +} +impl From for CommandTransferValue { + fn from(s: bool) -> Self { + Self::Boolean(s.to_owned()) + } +} +impl From<()> for CommandTransferValue { + fn from(_: ()) -> Self { + Self::Nil + } +} diff --git a/src/app/command_interface/lua_command_manager/mod.rs b/src/app/command_interface/lua_command_manager/mod.rs index 3116f0c..9b5bb24 100644 --- a/src/app/command_interface/lua_command_manager/mod.rs +++ b/src/app/command_interface/lua_command_manager/mod.rs @@ -1,10 +1,9 @@ -use std::{collections::HashMap, fmt::Display, thread}; +use std::thread; use anyhow::{Context, Result}; -use cli_log::{error, info, debug}; +use cli_log::{debug, error, info}; use mlua::{Function, Value}; use once_cell::sync::OnceCell; -use serde::{Deserialize, Serialize}; use tokio::{ runtime::Builder, sync::{mpsc, Mutex}, @@ -17,95 +16,13 @@ use crate::app::{ }; static LUA: OnceCell> = OnceCell::new(); -pub type Table = HashMap; - -#[derive(Debug, Serialize, Deserialize, Clone)] -pub enum CommandTransferValue { - /// `nil` or `null` or `undefined`; anything which goes in that group of types. - Nil, - - /// `true` or `false`. - Boolean(bool), - - // A "light userdata" object, equivalent to a raw pointer. - // /*TODO*/ LightUserData(LightUserData), - - /// An integer number. - Integer(i64), - - /// A floating point number. - Number(f64), - - /// A string - String(String), - - /// A table, directory or HashMap - Table(HashMap), - - // Reference to a Lua function (or closure). - // /* TODO */ Function(Function), - - // Reference to a Lua thread (or coroutine). - // /* TODO */ Thread(Thread<'lua>), - - // Reference to a userdata object that holds a custom type which implements `UserData`. - // Special builtin userdata types will be represented as other `Value` variants. - // /* TODO */ UserData(AnyUserData), - - // `Error` is a special builtin userdata type. When received from Lua it is implicitly cloned. - // /* TODO */ Error(Error), -} - -impl Display for CommandTransferValue { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - CommandTransferValue::Nil => f.write_str("Nil"), - CommandTransferValue::Boolean(bool) => f.write_str(&format!("{}", bool)), - CommandTransferValue::Integer(int) => f.write_str(&format!("{}", int)), - CommandTransferValue::Number(num) => f.write_str(&format!("{}", num)), - CommandTransferValue::String(str) => f.write_str(&format!("{}", str)), - // TODO(@Soispha): The following line should be a real display call, but how do you - // format a HashMap? - CommandTransferValue::Table(table) => f.write_str(&format!("{:#?}", table)), - } - } -} +/// This structure contains the necessary state for running an embedded Lua runtime (i.e. +/// the tread, the Lua memory, etc.). pub struct LuaCommandManager { lua_command_tx: mpsc::Sender, } -impl From for CommandTransferValue { - fn from(s: String) -> Self { - Self::String(s.to_owned()) - } -} -impl From for CommandTransferValue { - fn from(s: f64) -> Self { - Self::Number(s.to_owned()) - } -} -impl From for CommandTransferValue { - fn from(s: i64) -> Self { - Self::Integer(s.to_owned()) - } -} -impl From> for CommandTransferValue { - fn from(s: HashMap) -> Self { - Self::Table(s.to_owned()) - } -} -impl From for CommandTransferValue { - fn from(s: bool) -> Self { - Self::Boolean(s.to_owned()) - } -} -impl From<()> for CommandTransferValue { - fn from(_: ()) -> Self { - Self::Nil - } -} - impl LuaCommandManager { pub async fn execute_code(&self, code: String) { self.lua_command_tx diff --git a/src/app/command_interface/mod.rs b/src/app/command_interface/mod.rs index 0418470..e8cb958 100644 --- a/src/app/command_interface/mod.rs +++ b/src/app/command_interface/mod.rs @@ -1,14 +1,15 @@ // Use `cargo expand app::command_interface` for an overview of the file contents +pub mod command_transfer_value; pub mod lua_command_manager; use language_macros::ci_command_enum; // TODO(@Soispha): Should these paths be moved to the proc macro? // As they are not static, it could be easier for other people, -// if they stay here -use lua_command_manager::CommandTransferValue; -use mlua::LuaSerdeExt; +// if they stay here. +use crate::app::command_interface::command_transfer_value::CommandTransferValue; +use mlua::IntoLua; use crate::app::Event; #[ci_command_enum] @@ -25,7 +26,6 @@ struct Commands { /// Returns a table of greeted users greet_multiple: fn() -> Table, // End debug functions - /// Closes the application exit: fn(), @@ -46,7 +46,7 @@ struct Commands { set_mode_insert: fn(), /// Send a message to the current room - /// The sent message is interpreted literally. + /// The send message is interpreted literally. room_message_send: fn(String), /// Open the help pages at the first occurrence of diff --git a/src/app/events/event_types/event/handlers/command.rs b/src/app/events/event_types/event/handlers/command.rs index de301f9..841db28 100644 --- a/src/app/events/event_types/event/handlers/command.rs +++ b/src/app/events/event_types/event/handlers/command.rs @@ -4,14 +4,18 @@ use anyhow::{Error, Result}; use cli_log::{trace, warn}; use tokio::sync::oneshot; -use crate::{app::{ - command_interface::{ - lua_command_manager::{CommandTransferValue, Table}, - Command, +use crate::{ + app::{ + command_interface::{ + command_transfer_value::{CommandTransferValue, Table}, + Command, + }, + events::event_types::EventStatus, + status::State, + App, }, - events::event_types::EventStatus, - App, status::State, -}, ui::central::InputPosition}; + ui::central::InputPosition, +}; pub async fn handle( app: &mut App<'_>, @@ -20,8 +24,8 @@ pub async fn handle( ) -> Result { // A command can both return _status output_ (what you would normally print to stderr) // and _main output_ (the output which is normally printed to stdout). - // We simulate these by returning the main output to the lua function, and printing the - // status output to a status ui field. + // We simulate these by returning the main output to the Lua function, and printing the + // status output to a status UI field. // // Every function should return some status output to show the user, that something is // happening, while only some functions return some value to the main output, as this @@ -68,7 +72,7 @@ pub async fn handle( } Command::DisplayOutput(output) => { - // TODO(@Soispha): This is only used to show the lua command output to the user. + // TODO(@Soispha): This is only used to show the Lua command output to the user. // Lua commands already receive the output. This should probably be communicated // better, should it? send_status_output!(output); @@ -114,7 +118,7 @@ pub async fn handle( room.send(msg.clone()).await?; send_status_output!("Sent message: `{}`", msg); } else { - // TODO(@Soispha): Should this raise a lua error? It could be very confusing, + // TODO(@Soispha): Should this raise a Lua error? It could be very confusing, // when a user doesn't read the log. warn!("Can't send message: `{}`, as there is no open room!", &msg); } @@ -126,13 +130,25 @@ pub async fn handle( } Command::Print(output) => { // FIXME(@Soispha): This only works with strings, which is a clear downside to the - // original print function. Find a way to just use the original one + // original print function. Find a way to just use the original one. send_main_output!("{}", output); EventStatus::Ok } Command::GreetMultiple => { let mut table: Table = HashMap::new(); - table.insert("UserName1".to_owned(), CommandTransferValue::Integer(2)); + table.insert("UserId".to_owned(), CommandTransferValue::Integer(2)); + table.insert( + "UserName".to_owned(), + CommandTransferValue::String("James".to_owned()), + ); + + let mut second_table: Table = HashMap::new(); + second_table.insert("interface".to_owned(), CommandTransferValue::Integer(3)); + second_table.insert("api".to_owned(), CommandTransferValue::Boolean(true)); + table.insert( + "Versions".to_owned(), + CommandTransferValue::Table(second_table), + ); send_main_output!(table); EventStatus::Ok } diff --git a/src/app/events/event_types/event/mod.rs b/src/app/events/event_types/event/mod.rs index 27bba87..942070a 100644 --- a/src/app/events/event_types/event/mod.rs +++ b/src/app/events/event_types/event/mod.rs @@ -6,7 +6,7 @@ use crossterm::event::Event as CrosstermEvent; use tokio::sync::oneshot; use crate::app::{ - command_interface::{lua_command_manager::CommandTransferValue, Command}, + command_interface::{command_transfer_value::CommandTransferValue, Command}, status::State, App, };