Merge branch 'master' of https://github.com/boatbomber/rojo into headless-api

.github/workflows/ci.yml

@@ -12,11 +12,12 @@ on:
 jobs:
   build:
     name: Build and Test
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
 
     strategy:
       matrix:
         rust_version: [stable, 1.70.0]
+        os: [ubuntu-latest, windows-latest, macos-latest]
 
     steps:
     - uses: actions/checkout@v3

CHANGELOG.md

@@ -2,8 +2,12 @@
 
 ## Unreleased Changes
 * Added headless API for Studio companion plugins. ([#631])
+* Improved the visualization for array properties like Tags ([#829])
+* Significantly improved performance of `rojo serve`, `rojo build --watch`, and `rojo sourcemap --watch` on macOS. [#830]
 
 [#631]: https://github.com/rojo-rbx/rojo/pull/631
+[#829]: https://github.com/rojo-rbx/rojo/pull/829
+[#830]: https://github.com/rojo-rbx/rojo/pull/830
 
 ## [7.4.0-rc3] - October 25, 2023
 * Changed `sourcemap --watch` to only generate the sourcemap when it's necessary ([#800])
@@ -142,7 +146,6 @@
 * Add buttons for navigation on the Connected page ([#722])
 
 ### Fixes
-* Significantly improved performance of `rojo serve` and `rojo build` on macOS. [#783]
 * Significantly improved performance of `rojo sourcemap` ([#668])
 * Fixed the diff visualizer of connected sessions. ([#674])
 * Fixed disconnected session activity. ([#675])
@@ -176,7 +179,6 @@
 [#770]: https://github.com/rojo-rbx/rojo/pull/770
 [#771]: https://github.com/rojo-rbx/rojo/pull/771
 [#774]: https://github.com/rojo-rbx/rojo/pull/774
-[#783]: https://github.com/rojo-rbx/rojo/pull/783
 [rbx-dom#299]: https://github.com/rojo-rbx/rbx-dom/pull/299
 [rbx-dom#296]: https://github.com/rojo-rbx/rbx-dom/pull/296
 

Cargo.lock

@@ -1852,7 +1852,6 @@ dependencies = [
  "log",
  "maplit",
  "memofs",
- "notify",
  "num_cpus",
  "opener",
  "paste",

Cargo.toml

@@ -69,7 +69,6 @@ hyper = { version = "0.14.13", features = ["server", "tcp", "http1"] }
 jod-thread = "0.1.2"
 log = "0.4.14"
 maplit = "1.0.2"
-notify = "4.0.17"
 num_cpus = "1.15.0"
 opener = "0.5.0"
 rayon = "1.7.0"

crates/memofs/CHANGELOG.md

@@ -1,7 +1,9 @@
 # memofs Changelog
 
 ## Unreleased Changes
-* Changed the `StdBackend` file watcher to use `PollWatcher` on macOS.
+* Changed `StdBackend` file watching component to use minimal recursive watches. [#830]
+
+[#830]: https://github.com/rojo-rbx/rojo/pull/830
 
 ## 0.2.0 (2021-08-23)
 * Updated to `crossbeam-channel` 0.5.1.

crates/memofs/src/std_backend.rs

@@ -1,38 +1,24 @@
-use std::io;
+use std::path::{Path, PathBuf};
-use std::path::Path;
 use std::sync::mpsc;
 use std::thread;
 use std::time::Duration;
+use std::{collections::HashSet, io};
 
 use crossbeam_channel::Receiver;
-use notify::{DebouncedEvent, RecursiveMode, Watcher};
+use notify::{watcher, DebouncedEvent, RecommendedWatcher, RecursiveMode, Watcher};
-
-#[cfg(target_os = "macos")]
-use notify::PollWatcher;
-#[cfg(not(target_os = "macos"))]
-use notify::{watcher, RecommendedWatcher};
 
 use crate::{DirEntry, Metadata, ReadDir, VfsBackend, VfsEvent};
 
 /// `VfsBackend` that uses `std::fs` and the `notify` crate.
 pub struct StdBackend {
-    // We use PollWatcher on macos because using the KQueue watcher
-    // can cause some gnarly performance problems.
-    #[cfg(target_os = "macos")]
-    watcher: PollWatcher,
-    #[cfg(not(target_os = "macos"))]
     watcher: RecommendedWatcher,
-
     watcher_receiver: Receiver<VfsEvent>,
+    watches: HashSet<PathBuf>,
 }
 
 impl StdBackend {
     pub fn new() -> StdBackend {
         let (notify_tx, notify_rx) = mpsc::channel();
-
-        #[cfg(target_os = "macos")]
-        let watcher = PollWatcher::new(notify_tx, Duration::from_millis(50)).unwrap();
-        #[cfg(not(target_os = "macos"))]
         let watcher = watcher(notify_tx, Duration::from_millis(50)).unwrap();
 
         let (tx, rx) = crossbeam_channel::unbounded();
@@ -63,6 +49,7 @@ impl StdBackend {
         Self {
             watcher,
             watcher_receiver: rx,
+            watches: HashSet::new(),
         }
     }
 }
@@ -112,12 +99,22 @@ impl VfsBackend for StdBackend {
     }
 
     fn watch(&mut self, path: &Path) -> io::Result<()> {
+        if self.watches.contains(path)
+            || path
+                .ancestors()
+                .any(|ancestor| self.watches.contains(ancestor))
+        {
+            Ok(())
+        } else {
+            self.watches.insert(path.to_path_buf());
             self.watcher
-            .watch(path, RecursiveMode::NonRecursive)
+                .watch(path, RecursiveMode::Recursive)
                 .map_err(|inner| io::Error::new(io::ErrorKind::Other, inner))
         }
+    }
 
     fn unwatch(&mut self, path: &Path) -> io::Result<()> {
+        self.watches.remove(path);
         self.watcher
             .unwatch(path)
             .map_err(|inner| io::Error::new(io::ErrorKind::Other, inner))

plugin/src/App/Components/PatchVisualizer/DisplayValue.lua

@@ -53,8 +53,23 @@ local function DisplayValue(props)
 			elseif next(props.value) == nil then
 				-- If it's empty, show empty braces
 				textRepresentation = "{}"
+			elseif next(props.value) == 1 then
+				-- We don't need to support mixed tables, so checking the first key is enough
+				-- to determine if it's a simple array
+				local out, i = table.create(#props.value), 0
+				for k, v in props.value do
+					i += 1
+
+					-- Wrap strings in quotes
+					if type(v) == "string" then
+						v = '"' .. v .. '"'
+					end
+
+					out[i] = tostring(v)
+				end
+				textRepresentation = "{ " .. table.concat(out, ", ") .. " }"
 			else
-				-- If it has children, list them out
+				-- Otherwise, show the table contents as a dictionary
 				local out, i = {}, 0
 				for k, v in pairs(props.value) do
 					i += 1

plugin/src/preloadAssets.lua

@@ -13,7 +13,7 @@ function gatherAssetUrlsRecursive(currentTable, currentUrls)
 		if typeof(value) == "string" then
 			table.insert(currentUrls, value)
 		elseif typeof(value) == "table" then
-			gatherAssetUrlsRecursive(value)
+			gatherAssetUrlsRecursive(value, currentUrls)
 		end
 	end
 

src/change_processor.rs

@@ -1,6 +1,5 @@
 use std::{
     fs,
-    path::PathBuf,
     sync::{Arc, Mutex},
 };
 
@@ -126,14 +125,41 @@ impl JobThreadContext {
         // For a given VFS event, we might have many changes to different parts
         // of the tree. Calculate and apply all of these changes.
         let applied_patches = match event {
-            VfsEvent::Write(path) => {
+            VfsEvent::Create(path) | VfsEvent::Remove(path) | VfsEvent::Write(path) => {
-                if path.is_dir() {
+                let mut tree = self.tree.lock().unwrap();
-                    return;
+                let mut applied_patches = Vec::new();
+
+                // Find the nearest ancestor to this path that has
+                // associated instances in the tree. This helps make sure
+                // that we handle additions correctly, especially if we
+                // receive events for descendants of a large tree being
+                // created all at once.
+                let mut current_path = path.as_path();
+                let affected_ids = loop {
+                    let ids = tree.get_ids_at_path(&current_path);
+
+                    log::trace!("Path {} affects IDs {:?}", current_path.display(), ids);
+
+                    if !ids.is_empty() {
+                        break ids.to_vec();
                     }
-                on_vfs_event(path, &self.tree, &self.vfs)
+
+                    log::trace!("Trying parent path...");
+                    match current_path.parent() {
+                        Some(parent) => current_path = parent,
+                        None => break Vec::new(),
                     }
-            VfsEvent::Create(path) | VfsEvent::Remove(path) => {
+                };
-                on_vfs_event(path, &self.tree, &self.vfs)
+
+                for id in affected_ids {
+                    if let Some(patch) = compute_and_apply_changes(&mut tree, &self.vfs, id) {
+                        if !patch.is_empty() {
+                            applied_patches.push(patch);
+                        }
+                    }
+                }
+
+                applied_patches
             }
             _ => {
                 log::warn!("Unhandled VFS event: {:?}", event);
@@ -236,45 +262,6 @@ impl JobThreadContext {
     }
 }
 
-// Find the nearest ancestor to this path that has
-// associated instances in the tree. This helps make sure
-// that we handle additions correctly, especially if we
-// receive events for descendants of a large tree being
-// created all at once.
-fn on_vfs_event(
-    path: PathBuf,
-    tree: &Arc<Mutex<RojoTree>>,
-    vfs: &Arc<Vfs>,
-) -> Vec<AppliedPatchSet> {
-    let mut tree = tree.lock().unwrap();
-    let mut applied_patches = Vec::new();
-
-    let mut current_path = path.as_path();
-    let affected_ids = loop {
-        let ids = tree.get_ids_at_path(current_path);
-
-        log::trace!("Path {} affects IDs {:?}", current_path.display(), ids);
-
-        if !ids.is_empty() {
-            break ids.to_vec();
-        }
-
-        log::trace!("Trying parent path...");
-        match current_path.parent() {
-            Some(parent) => current_path = parent,
-            None => break Vec::new(),
-        }
-    };
-
-    for id in affected_ids {
-        if let Some(patch) = compute_and_apply_changes(&mut tree, vfs, id) {
-            if !patch.is_empty() {
-                applied_patches.push(patch);
-            }
-        }
-    }
-    applied_patches
-}
 fn compute_and_apply_changes(tree: &mut RojoTree, vfs: &Vfs, id: Ref) -> Option<AppliedPatchSet> {
     let metadata = tree
         .get_metadata(id)

tests/rojo_test/serve_util.rs

@@ -66,7 +66,11 @@ impl TestServeSession {
 
         let source_path = Path::new(SERVE_TESTS_PATH).join(name);
         let dir = tempdir().expect("Couldn't create temporary directory");
-        let project_path = dir.path().join(name);
+        let project_path = dir
+            .path()
+            .canonicalize()
+            .expect("Couldn't canonicalize temporary directory path")
+            .join(name);
 
         let source_is_file = fs::metadata(&source_path).unwrap().is_file();