From 8662d2227ca057f42c9b8f588f8436406362e020 Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sun, 16 Jul 2023 14:59:30 -0700 Subject: [PATCH 1/3] Improve Rojo sync reliability (#739) Uses non-recursive main loop and avoids hanging promises --- plugin/src/ApiContext.lua | 15 ++-------- plugin/src/ServeSession.lua | 55 ++++++++++++++++++++++++------------- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/plugin/src/ApiContext.lua b/plugin/src/ApiContext.lua index 98890d4f6..9a683dd55 100644 --- a/plugin/src/ApiContext.lua +++ b/plugin/src/ApiContext.lua @@ -11,13 +11,6 @@ local validateApiInfo = Types.ifEnabled(Types.ApiInfoResponse) local validateApiRead = Types.ifEnabled(Types.ApiReadResponse) local validateApiSubscribe = Types.ifEnabled(Types.ApiSubscribeResponse) ---[[ - Returns a promise that will never resolve nor reject. -]] -local function hangingPromise() - return Promise.new(function() end) -end - local function rejectFailedRequests(response) if response.code >= 400 then local message = string.format("HTTP %s:\n%s", tostring(response.code), response.body) @@ -212,12 +205,8 @@ function ApiContext:retrieveMessages() local function sendRequest() local request = Http.get(url) :catch(function(err) - if err.type == Http.Error.Kind.Timeout then - if self.__connected then - return sendRequest() - else - return hangingPromise() - end + if err.type == Http.Error.Kind.Timeout and self.__connected then + return sendRequest() end return Promise.reject(err) diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 08dc40411..4e5116c81 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -299,27 +299,44 @@ function ServeSession:__initialSync(serverInfo) end function ServeSession:__mainSyncLoop() - return self.__apiContext:retrieveMessages() - :andThen(function(messages) - Log.trace("Serve session {} retrieved {} messages", tostring(self), #messages) - - for _, message in ipairs(messages) do - local unappliedPatch = self.__reconciler:applyPatch(message) - - if not PatchSet.isEmpty(unappliedPatch) then - Log.warn("Could not apply all changes requested by the Rojo server:\n{}", - PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) - end - - if self.__patchAppliedCallback then - pcall(self.__patchAppliedCallback, message, unappliedPatch) - end + return Promise.new(function(resolve, reject) + while self.__status == Status.Connected do + local success, result = self.__apiContext:retrieveMessages() + :andThen(function(messages) + if self.__status == Status.Disconnected then + -- In the time it took to retrieve messages, we disconnected + -- so we just resolve immediately without patching anything + return + end + + Log.trace("Serve session {} retrieved {} messages", tostring(self), #messages) + + for _, message in messages do + local unappliedPatch = self.__reconciler:applyPatch(message) + + if not PatchSet.isEmpty(unappliedPatch) then + Log.warn("Could not apply all changes requested by the Rojo server:\n{}", + PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) + end + + if self.__patchAppliedCallback then + pcall(self.__patchAppliedCallback, message, unappliedPatch) + end + end + end):await() + + if self.__status == Status.Disconnected then + -- If we are no longer connected after applying, we stop silently + -- without checking for errors as they are no longer relevant + break + elseif success == false then + reject(result) end + end - if self.__status ~= Status.Disconnected then - return self:__mainSyncLoop() - end - end) + -- We are no longer connected, so we resolve the promise + resolve() + end) end function ServeSession:__stopInternal(err) From 49154778234d90aeb47be260c42a6f9d7b62fb1f Mon Sep 17 00:00:00 2001 From: boatbomber Date: Sun, 16 Jul 2023 21:03:06 -0700 Subject: [PATCH 2/3] Expose reconciler hooks on servesession (#741) --- plugin/src/App/init.lua | 2 +- plugin/src/ServeSession.lua | 16 ++++++---------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/plugin/src/App/init.lua b/plugin/src/App/init.lua index 23d0a5069..f4c30f3c2 100644 --- a/plugin/src/App/init.lua +++ b/plugin/src/App/init.lua @@ -374,7 +374,7 @@ function App:startSession() end) end) - serveSession:onPatchApplied(function(patch, unapplied) + serveSession:hookPostcommit(function(patch, _instanceMap, unapplied) local now = os.time() local old = self.state.patchData diff --git a/plugin/src/ServeSession.lua b/plugin/src/ServeSession.lua index 4e5116c81..6a3fcdc4d 100644 --- a/plugin/src/ServeSession.lua +++ b/plugin/src/ServeSession.lua @@ -97,7 +97,6 @@ function ServeSession.new(options) __instanceMap = instanceMap, __changeBatcher = changeBatcher, __statusChangedCallback = nil, - __patchAppliedCallback = nil, __connections = connections, } @@ -129,8 +128,12 @@ function ServeSession:setConfirmCallback(callback) self.__userConfirmCallback = callback end -function ServeSession:onPatchApplied(callback) - self.__patchAppliedCallback = callback +function ServeSession:hookPrecommit(callback) + return self.__reconciler:hookPrecommit(callback) +end + +function ServeSession:hookPostcommit(callback) + return self.__reconciler:hookPostcommit(callback) end function ServeSession:start() @@ -291,9 +294,6 @@ function ServeSession:__initialSync(serverInfo) Log.warn("Could not apply all changes requested by the Rojo server:\n{}", PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) end - if self.__patchAppliedCallback then - pcall(self.__patchAppliedCallback, catchUpPatch, unappliedPatch) - end end end) end @@ -318,10 +318,6 @@ function ServeSession:__mainSyncLoop() Log.warn("Could not apply all changes requested by the Rojo server:\n{}", PatchSet.humanSummary(self.__instanceMap, unappliedPatch)) end - - if self.__patchAppliedCallback then - pcall(self.__patchAppliedCallback, message, unappliedPatch) - end end end):await() From dc17a185ca4b256b31989bfa616e0bc64eba7ede Mon Sep 17 00:00:00 2001 From: Micah <48431591+nezuo@users.noreply.github.com> Date: Mon, 17 Jul 2023 21:15:40 -0500 Subject: [PATCH 3/3] Add plugin build flag (#735) Resolves #715. --- CHANGELOG.md | 2 ++ benches/build.rs | 3 +- src/cli/build.rs | 87 +++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c2e1bbf1..3617eb478 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ * Better settings controls ([#725]) * Rework patch visualizer with many fixes and improvements ([#726]) * Added support for syncing in `.toml` files ([#633]) +* Add `plugin` flag to the `build` command that outputs to the local plugins folder ([#735]) [#668]: https://github.com/rojo-rbx/rojo/pull/668 [#674]: https://github.com/rojo-rbx/rojo/pull/674 @@ -34,6 +35,7 @@ [#725]: https://github.com/rojo-rbx/rojo/pull/725 [#726]: https://github.com/rojo-rbx/rojo/pull/726 [#633]: https://github.com/rojo-rbx/rojo/pull/633 +[#735]: https://github.com/rojo-rbx/rojo/pull/735 ## [7.3.0] - April 22, 2023 * Added `$attributes` to project format. ([#574]) diff --git a/benches/build.rs b/benches/build.rs index 456a5339a..87d33e442 100644 --- a/benches/build.rs +++ b/benches/build.rs @@ -31,11 +31,12 @@ fn bench_build_place(c: &mut Criterion, name: &str, path: &str) { fn place_setup>(input_path: P) -> (TempDir, BuildCommand) { let dir = tempdir().unwrap(); let input = input_path.as_ref().to_path_buf(); - let output = dir.path().join("output.rbxlx"); + let output = Some(dir.path().join("output.rbxlx")); let options = BuildCommand { project: input, watch: false, + plugin: None, output, }; diff --git a/src/cli/build.rs b/src/cli/build.rs index d6c3f3499..8f705d376 100644 --- a/src/cli/build.rs +++ b/src/cli/build.rs @@ -4,10 +4,11 @@ use std::{ path::{Path, PathBuf}, }; -use anyhow::Context; -use clap::Parser; +use anyhow::{bail, Context}; +use clap::{CommandFactory, Parser}; use fs_err::File; use memofs::Vfs; +use roblox_install::RobloxStudio; use tokio::runtime::Runtime; use crate::serve_session::ServeSession; @@ -16,6 +17,8 @@ use super::resolve_path; const UNKNOWN_OUTPUT_KIND_ERR: &str = "Could not detect what kind of file to build. \ Expected output file to end in .rbxl, .rbxlx, .rbxm, or .rbxmx."; +const UNKNOWN_PLUGIN_KIND_ERR: &str = "Could not detect what kind of file to build. \ + Expected plugin file to end in .rbxm or .rbxmx."; /// Generates a model or place file from the Rojo project. #[derive(Debug, Parser)] @@ -28,7 +31,13 @@ pub struct BuildCommand { /// /// Should end in .rbxm, .rbxl, .rbxmx, or .rbxlx. #[clap(long, short)] - pub output: PathBuf, + pub output: Option, + + /// Alternative to the output flag that outputs the result in the local plugins folder. + /// + /// Should end in .rbxm or .rbxl. + #[clap(long, short)] + pub plugin: Option, /// Whether to automatically rebuild when any input files change. #[clap(long)] @@ -37,18 +46,52 @@ pub struct BuildCommand { impl BuildCommand { pub fn run(self) -> anyhow::Result<()> { - let project_path = resolve_path(&self.project); + let (output_path, output_kind) = match (self.output, self.plugin) { + (Some(_), Some(_)) => { + BuildCommand::command() + .error( + clap::ErrorKind::ArgumentConflict, + "the argument '--output ' cannot be used with '--plugin '", + ) + .exit(); + } + (None, None) => { + BuildCommand::command() + .error( + clap::ErrorKind::MissingRequiredArgument, + "one of the following arguments must be provided: \n --output \n --plugin ", + ) + .exit(); + } + (Some(output), None) => { + let output_kind = + OutputKind::from_output_path(&output).context(UNKNOWN_OUTPUT_KIND_ERR)?; + + (output, output_kind) + } + (None, Some(plugin)) => { + if plugin.is_absolute() { + bail!("plugin flag path cannot be absolute.") + } - let output_kind = detect_output_kind(&self.output).context(UNKNOWN_OUTPUT_KIND_ERR)?; + let output_kind = + OutputKind::from_plugin_path(&plugin).context(UNKNOWN_PLUGIN_KIND_ERR)?; + let studio = RobloxStudio::locate()?; + + (studio.plugins_path().join(&plugin), output_kind) + } + }; + + let project_path = resolve_path(&self.project); log::trace!("Constructing in-memory filesystem"); let vfs = Vfs::new_default(); vfs.set_watch_enabled(self.watch); - let session = ServeSession::new(vfs, &project_path)?; + let session = ServeSession::new(vfs, project_path)?; let mut cursor = session.message_queue().cursor(); - write_model(&session, &self.output, output_kind)?; + write_model(&session, &output_path, output_kind)?; if self.watch { let rt = Runtime::new().unwrap(); @@ -58,7 +101,7 @@ impl BuildCommand { let (new_cursor, _patch_set) = rt.block_on(receiver).unwrap(); cursor = new_cursor; - write_model(&session, &self.output, output_kind)?; + write_model(&session, &output_path, output_kind)?; } } @@ -86,15 +129,27 @@ enum OutputKind { Rbxl, } -fn detect_output_kind(output: &Path) -> Option { - let extension = output.extension()?.to_str()?; +impl OutputKind { + fn from_output_path(output: &Path) -> Option { + let extension = output.extension()?.to_str()?; + + match extension { + "rbxlx" => Some(OutputKind::Rbxlx), + "rbxmx" => Some(OutputKind::Rbxmx), + "rbxl" => Some(OutputKind::Rbxl), + "rbxm" => Some(OutputKind::Rbxm), + _ => None, + } + } - match extension { - "rbxlx" => Some(OutputKind::Rbxlx), - "rbxmx" => Some(OutputKind::Rbxmx), - "rbxl" => Some(OutputKind::Rbxl), - "rbxm" => Some(OutputKind::Rbxm), - _ => None, + fn from_plugin_path(output: &Path) -> Option { + let extension = output.extension()?.to_str()?; + + match extension { + "rbxmx" => Some(OutputKind::Rbxmx), + "rbxm" => Some(OutputKind::Rbxm), + _ => None, + } } }