diff --git a/main/lib/helgoboss-learn b/main/lib/helgoboss-learn index 41f3f1320..0c3a1cd04 160000 --- a/main/lib/helgoboss-learn +++ b/main/lib/helgoboss-learn @@ -1 +1 @@ -Subproject commit 41f3f1320953645a9795b723f04d207c72a40d6f +Subproject commit 0c3a1cd04da93387e0dc52612b0e562ab42bd961 diff --git a/main/src/application/unit_model.rs b/main/src/application/unit_model.rs index d18309fbc..7321faef8 100644 --- a/main/src/application/unit_model.rs +++ b/main/src/application/unit_model.rs @@ -2992,5 +2992,5 @@ fn compile_common_lua( Ok(val) })?; env.set("require", require)?; - lua.compile_and_execute(compartment.as_ref(), code, env) + lua.compile_and_execute(compartment.to_string(), code, env) } diff --git a/main/src/domain/lua_module_container.rs b/main/src/domain/lua_module_container.rs index 771ad40a9..57de1d9f2 100644 --- a/main/src/domain/lua_module_container.rs +++ b/main/src/domain/lua_module_container.rs @@ -5,8 +5,10 @@ use camino::Utf8Path; use include_dir::Dir; use mlua::{Function, Lua, Value}; use std::borrow::Cow; +use std::cell::RefCell; use std::fs; use std::path::PathBuf; +use std::rc::Rc; /// Allows executing Lua code as a module that may require other modules. pub struct LuaModuleContainer { @@ -45,21 +47,56 @@ where pub fn execute_as_module<'lua>( &self, lua: &'lua Lua, - name: &str, + normalized_path: Option, + display_name: String, code: &str, ) -> anyhow::Result> { - execute_as_module(name, code, self.finder.clone(), lua) + execute_as_module( + lua, + normalized_path, + display_name, + code, + self.finder.clone(), + SharedAccumulator::default(), + ) } } +#[derive(Default)] +struct Accumulator { + required_modules_stack: Vec, +} + +impl Accumulator { + /// The given module must be normalized, i.e. it should contain the extension. + pub fn push_module(&mut self, normalized_path: String) -> anyhow::Result<()> { + let stack = &mut self.required_modules_stack; + tracing::debug!(msg = "Pushing module onto stack", %normalized_path, ?stack); + if stack.iter().any(|path| path == &normalized_path) { + bail!("Detected cyclic Lua module dependency: {normalized_path}"); + } + stack.push(normalized_path); + Ok(()) + } + + pub fn pop_module(&mut self) { + let stack = &mut self.required_modules_stack; + tracing::debug!(msg = "Popping top module from stack", ?stack); + stack.pop(); + } +} + +type SharedAccumulator = Rc>; + fn find_and_execute_module<'lua>( - finder: impl LuaModuleFinder + Clone + 'static, lua: &'lua Lua, - path: &str, + finder: impl LuaModuleFinder + Clone + 'static, + accumulator: SharedAccumulator, + required_path: &str, ) -> anyhow::Result> { // Validate let root_info = || format!("\n\nModule root path: {}", finder.module_root_path()); - let path = Utf8Path::new(path); + let path = Utf8Path::new(required_path); if path.is_absolute() { bail!("Required paths must not start with a slash. They are always relative to the preset sub directory.{}", root_info()); } @@ -83,16 +120,25 @@ fn find_and_execute_module<'lua>( return Ok(Value::Table(table)); } // Find module and get its source - let source = if path.extension().is_some() { + let (normalized_path, source) = if path + .extension() + .is_some_and(|ext| matches!(ext, "luau" | "lua")) + { // Extension given. Just get file directly. - finder + let source = finder .find_source_by_path(path.as_str()) - .with_context(|| format!("Couldn't find Lua module [{path}].{}", root_info()))? + .with_context(|| format!("Couldn't find Lua module [{path}].{}", root_info()))?; + (path.to_string(), source) } else { // No extension given. Try ".luau" and ".lua". ["luau", "lua"] .into_iter() - .find_map(|ext| finder.find_source_by_path(path.with_extension(ext).as_str())) + .find_map(|ext| { + let path_with_extension = format!("{path}.{ext}"); + tracing::debug!(msg = "Finding module by path...", %path_with_extension); + let source = finder.find_source_by_path(&path_with_extension)?; + Some((path_with_extension, source)) + }) .with_context(|| { format!( "Couldn't find Lua module [{path}]. Tried with extension \".lua\" and \".luau\".{}", root_info() @@ -100,7 +146,14 @@ fn find_and_execute_module<'lua>( })? }; // Execute module - execute_as_module(path.as_str(), source.as_ref(), Ok(finder), lua) + execute_as_module( + lua, + Some(normalized_path.clone()), + normalized_path, + source.as_ref(), + Ok(finder), + accumulator, + ) } pub fn lua_module_path_without_ext(path: &str) -> &str { @@ -110,21 +163,30 @@ pub fn lua_module_path_without_ext(path: &str) -> &str { } fn execute_as_module<'lua>( - name: &str, + lua: &'lua Lua, + normalized_path: Option, + display_name: String, code: &str, finder: Result, - lua: &'lua Lua, + accumulator: SharedAccumulator, ) -> anyhow::Result> { let env = create_fresh_environment(lua, true)?; - let require = create_require_function(finder, lua)?; + let require = create_require_function(lua, finder, accumulator.clone())?; env.set("require", require)?; - let value = compile_and_execute(lua, name, code, env) - .with_context(|| format!("Couldn't compile and execute Lua module {name}"))?; + let pop_later = if let Some(p) = normalized_path { + accumulator.borrow_mut().push_module(p)?; + true + } else { + false + }; + let value = compile_and_execute(lua, display_name.clone(), code, env) + .with_context(|| format!("Couldn't compile and execute Lua module {display_name}"))?; + if pop_later { + accumulator.borrow_mut().pop_module(); + } Ok(value) - // TODO-high CONTINUE It doesn't seem to be straightforward to save the Values of the - // already loaded modules because of lifetime challenges. Not a big deal, no need - // to cache. But we should at least track what has already been loaded / maintain - // some kind of stack in order to fail on cycles. + // TODO-medium-performance Instead of just detecting cycles, we could cache the module execution result and return + // it whenever it's queried again. // match self.modules.entry(path) { // Entry::Occupied(e) => Ok(e.into_mut()), // Entry::Vacant(e) => { @@ -143,13 +205,15 @@ fn execute_as_module<'lua>( } fn create_require_function<'lua>( - finder: Result, lua: &'lua Lua, + finder: Result, + accumulator: SharedAccumulator, ) -> anyhow::Result> { - let require = lua.create_function_mut(move |lua, path: String| { + let require = lua.create_function_mut(move |lua, required_path: String| { let finder = finder.clone().map_err(mlua::Error::runtime)?; - let value = find_and_execute_module(finder.clone(), lua, &path) - .map_err(|e| mlua::Error::runtime(format!("{e:#}")))?; + let value = + find_and_execute_module(lua, finder.clone(), accumulator.clone(), &required_path) + .map_err(|e| mlua::Error::runtime(format!("{e:#}")))?; Ok(value) })?; Ok(require) @@ -204,7 +268,9 @@ impl LuaModuleFinder for FsDirLuaModuleFinder { return None; } let absolute_path = self.dir.join(path); + tracing::debug!(msg = "find_source_by_path", ?absolute_path); let content = fs::read_to_string(absolute_path).ok()?; + tracing::debug!(msg = "find_source_by_path successful"); Some(content.into()) } } diff --git a/main/src/domain/lua_support.rs b/main/src/domain/lua_support.rs index bf0fb849e..7174df6fa 100644 --- a/main/src/domain/lua_support.rs +++ b/main/src/domain/lua_support.rs @@ -53,11 +53,11 @@ impl SafeLua { /// Compiles and executes the given code in one go (shouldn't be used for repeated execution!). pub fn compile_and_execute<'a>( &'a self, - name: &str, + display_name: String, code: &str, env: Table<'a>, ) -> anyhow::Result> { - compile_and_execute(&self.0, name, code, env) + compile_and_execute(&self.0, display_name, code, env) } /// Creates a fresh environment for this Lua state. @@ -96,13 +96,13 @@ pub fn create_fresh_environment(lua: &Lua, allow_side_effects: bool) -> anyhow:: /// Compiles and executes the given code in one go (shouldn't be used for repeated execution!). pub fn compile_and_execute<'a>( lua: &'a Lua, - name: &str, + display_name: String, code: &str, env: Table<'a>, ) -> anyhow::Result> { let lua_chunk = lua .load(code) - .set_name(name) + .set_name(display_name) .set_mode(ChunkMode::Text) .set_environment(env); let value = lua_chunk.eval().map_err(|e| match e { diff --git a/main/src/infrastructure/data/preset.rs b/main/src/infrastructure/data/preset.rs index 482cc041b..369d5ad1a 100644 --- a/main/src/infrastructure/data/preset.rs +++ b/main/src/infrastructure/data/preset.rs @@ -155,6 +155,8 @@ pub struct PresetInfo { #[derive(Clone, Eq, PartialEq, Debug)] pub struct CommonPresetInfo { pub id: String, + /// Path relative to the namespace root including extension. + pub normalized_relative_path: String, pub file_type: PresetFileType, pub origin: PresetOrigin, pub meta_data: CommonPresetMetaData, @@ -198,7 +200,10 @@ impl fmt::Display for PresetOrigin { } struct PresetBasics { + /// Path relative to the **preset** root, without extension. id: String, + /// Path relative to the **preset namespace** root, including extension. + relative_path_within_namespace: String, file_type: PresetFileType, } @@ -226,7 +231,15 @@ impl PresetBasics { relative_dir_path.to_string_lossy().replace('\\', "/"); format!("{prefix}{relative_dir_path_with_slashes}/{id_leaf}") }; - let basics = Self { id, file_type }; + let relative_path_within_namespace: PathBuf = + relative_file_path.components().skip(1).collect(); + let basics = Self { + id, + relative_path_within_namespace: relative_path_within_namespace + .to_string_lossy() + .to_string(), + file_type, + }; Some(basics) } } @@ -410,9 +423,7 @@ impl FileBasedCompartmentPresetManager { preset_info: &PresetInfo, ) -> anyhow::Result { let file_content: Cow = match &preset_info.common.origin { - PresetOrigin::User { - absolute_file_path: absolute_path, - } => fs::read_to_string(absolute_path) + PresetOrigin::User { absolute_file_path } => fs::read_to_string(absolute_file_path) .map_err(|_| { anyhow!( "Couldn't read preset file \"{}\" anymore.", @@ -440,10 +451,8 @@ impl FileBasedCompartmentPresetManager { PresetFileType::Lua => { let lua = SafeLua::new()?; let script_name = preset_info.common.origin.to_string(); - let module_finder: Result, _> = match &preset_info - .common - .origin - { + let origin = &preset_info.common.origin; + let module_finder: Result, _> = match origin { PresetOrigin::User { absolute_file_path: _, } => { @@ -471,11 +480,12 @@ impl FileBasedCompartmentPresetManager { Ok(Rc::new(IncludedDirLuaModuleFinder::new(module_root))) } }; - let module_container = LuaModuleContainer::new(module_finder); let lua = lua.start_execution_time_limit_countdown()?; + let module_container = LuaModuleContainer::new(module_finder); let value = module_container.execute_as_module( lua.as_ref(), - &script_name, + Some(preset_info.common.normalized_relative_path.clone()), + script_name, &file_content, )?; let compartment_content: realearn_api::persistence::Compartment = @@ -647,6 +657,7 @@ fn load_preset_info( let preset_info = PresetInfo { common: CommonPresetInfo { id: basics.id, + normalized_relative_path: basics.relative_path_within_namespace, file_type: basics.file_type, origin, meta_data: preset_meta_data.common, diff --git a/main/src/infrastructure/ui/import.rs b/main/src/infrastructure/ui/import.rs index 108bb1958..258bbf713 100644 --- a/main/src/infrastructure/ui/import.rs +++ b/main/src/infrastructure/ui/import.rs @@ -410,5 +410,5 @@ fn execute_lua_import_script<'a>( let preset_dir = BackboneShell::realearn_compartment_preset_dir_path(active_compartment); let module_finder = FsDirLuaModuleFinder::new(preset_dir.join(whoami::username())); let module_container = LuaModuleContainer::new(Ok(module_finder)); - module_container.execute_as_module(lua.as_ref(), "Import", code) + module_container.execute_as_module(lua.as_ref(), None, "Import".to_string(), code) } diff --git a/resources/main-presets/factory/playtime_util.luau b/resources/main-presets/factory/playtime_util.luau index 20824e88c..d0c5ae040 100644 --- a/resources/main-presets/factory/playtime_util.luau +++ b/resources/main-presets/factory/playtime_util.luau @@ -43,7 +43,6 @@ function playtime_util.scroll_vertically(amount: number) return scroll("Y", amount) end - function playtime_util.slot_state_text_feedback() return util.partial_mapping { glue = {