Release Rojo v7.4.4 (#964)

Backports the release of memofs v0.3.0

.github/workflows/ci.yml

@@ -72,7 +72,7 @@ jobs:
       run: cargo build --locked --verbose
 
   lint:
-    name: Rustfmt, Clippy, & Stylua
+    name: Rustfmt, Clippy, Stylua, & Selene
     runs-on: ubuntu-latest
 
     steps:
@@ -98,6 +98,9 @@ jobs:
     - name: Stylua
       run: stylua --check plugin/src
 
+    - name: Selene
+      run: selene plugin/src
+
     - name: Rustfmt
       run: cargo fmt -- --check
 

.github/workflows/release.yml

@@ -36,7 +36,7 @@ jobs:
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
           trust-check: false
-          version: 'v0.2.6'
+          version: 'v0.3.0'
 
       - name: Build Plugin
         run: rojo build plugin --output Rojo.rbxm

CHANGELOG.md

@@ -2,6 +2,49 @@
 
 ## Unreleased Changes
 
+## [7.4.4] - August 22nd, 2024
+* Fixed issue with reading attributes from `Lighting` in new place files
+* `Instance.Archivable` will now default to `true` when building a project into a binary (`rbxm`/`rbxl`) file rather than `false`.
+
+## [7.4.3] - August 6th, 2024
+* Fixed issue with building binary files introduced in 7.4.2
+* Fixed `value of type nil cannot be converted to number` warning spam in output. [#955]
+
+[#955]: https://github.com/rojo-rbx/rojo/pull/893
+
+## [7.4.2] - July 23, 2024
+* Added Never option to Confirmation ([#893])
+* Fixed removing trailing newlines ([#903])
+* Updated the internal property database, correcting an issue with `SurfaceAppearance.Color` that was reported [here][Surface_Appearance_Color_1] and [here][Surface_Appearance_Color_2] ([#948])
+
+[#893]: https://github.com/rojo-rbx/rojo/pull/893
+[#903]: https://github.com/rojo-rbx/rojo/pull/903
+[#948]: https://github.com/rojo-rbx/rojo/pull/948
+[Surface_Appearance_Color_1]: https://devforum.roblox.com/t/jailbreak-custom-character-turned-shiny-black-no-texture/3075563
+[Surface_Appearance_Color_2]: https://devforum.roblox.com/t/surfaceappearance-not-displaying-correctly/3075588
+
+## [7.4.1] - February 20, 2024
+* Made the `name` field optional on project files ([#870])
+ 	Files named `default.project.json` inherit the name of the folder they're in and all other projects
+    are named as expect (e.g. `foo.project.json` becomes an Instance named `foo`)
+
+    There is no change in behavior if `name` is set.
+* Fixed incorrect results when building model pivots ([#865])
+* Fixed incorrect results when serving model pivots ([#868])
+* Rojo now converts any line endings to LF, preventing spurious diffs when syncing Lua files on Windows ([#854])
+* Fixed Rojo plugin failing to connect when project contains certain unreadable properties ([#848])
+* Fixed various cases where patch visualizer would not display sync failures ([#845], [#844])
+* Fixed http error handling so Rojo can be used in Github Codespaces ([#847])
+
+[#848]: https://github.com/rojo-rbx/rojo/pull/848
+[#845]: https://github.com/rojo-rbx/rojo/pull/845
+[#844]: https://github.com/rojo-rbx/rojo/pull/844
+[#847]: https://github.com/rojo-rbx/rojo/pull/847
+[#854]: https://github.com/rojo-rbx/rojo/pull/854
+[#865]: https://github.com/rojo-rbx/rojo/pull/865
+[#868]: https://github.com/rojo-rbx/rojo/pull/868
+[#870]: https://github.com/rojo-rbx/rojo/pull/870
+
 ## [7.4.0] - January 16, 2024
 * 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])

Cargo.lock

@@ -1073,7 +1073,7 @@ dependencies = [
 
 [[package]]
 name = "memofs"
-version = "0.2.0"
+version = "0.3.0"
 dependencies = [
  "crossbeam-channel",
  "fs-err",
@@ -1586,9 +1586,9 @@ dependencies = [
 
 [[package]]
 name = "rbx_binary"
-version = "0.7.4"
+version = "0.7.7"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6314dd6bf5c21d0598cdb53cf5d241aa643ba41da8b8abf7402b4a35096f03f6"
+checksum = "7b85057e8ff75a1ce99248200c4b3c7b481a3d52f921f1053ecd67921dcc7930"
 dependencies = [
  "log",
  "lz4",
@@ -1601,9 +1601,9 @@ dependencies = [
 
 [[package]]
 name = "rbx_dom_weak"
-version = "2.7.0"
+version = "2.9.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9b67b56bac99849c2e3c57547b036927f71c57cf7f4d900d04e3e4ee774ec316"
+checksum = "fcd2a17d09e46af0805f8b311a926402172b97e8d9388745c9adf8f448901841"
 dependencies = [
  "rbx_types",
  "serde",
@@ -1611,9 +1611,9 @@ dependencies = [
 
 [[package]]
 name = "rbx_reflection"
-version = "4.5.0"
+version = "4.7.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0d41509c991b53a7276a746a795eae2b9204f398164920f61976995b47fe1722"
+checksum = "8118ac6021d700e8debe324af6b40ecfd2cef270a00247849dbdfeebb0802677"
 dependencies = [
  "rbx_types",
  "serde",
@@ -1622,9 +1622,9 @@ dependencies = [
 
 [[package]]
 name = "rbx_reflection_database"
-version = "0.2.10+roblox-607"
+version = "0.2.12+roblox-638"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "12e20c06fa41f7aadc79005c8354f592b2c2f4d0c61e1080ed5718dafc30aea0"
+checksum = "0e29381d675420e841f8c02db5755cbb2545ed3e13f56c539546dc58702b512a"
 dependencies = [
  "lazy_static",
  "rbx_reflection",
@@ -1634,9 +1634,9 @@ dependencies = [
 
 [[package]]
 name = "rbx_types"
-version = "1.8.0"
+version = "1.10.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "7ca23bfd469d067d81ef14f65fe09aeddc25abcf576a889d1a7664fe021cf18c"
+checksum = "e30f49b2a3bb667e4074ba73c2dfb8ca0873f610b448ccf318a240acfdec6c73"
 dependencies = [
  "base64 0.13.1",
  "bitflags 1.3.2",
@@ -1649,9 +1649,9 @@ dependencies = [
 
 [[package]]
 name = "rbx_xml"
-version = "0.13.3"
+version = "0.13.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f8c03f95500961c32340791d1fabd4587f6873bdbff077ecca6ae32db7960dea"
+checksum = "2b14b3027bc9ccd82e2fc854c8bcd25ed58318e570c355bf2cf63df9cdbd5ba8"
 dependencies = [
  "base64 0.13.1",
  "log",
@@ -1831,7 +1831,7 @@ dependencies = [
 
 [[package]]
 name = "rojo"
-version = "7.4.0"
+version = "7.4.4"
 dependencies = [
  "anyhow",
  "backtrace",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rojo"
-version = "7.4.0"
+version = "7.4.4"
 rust-version = "1.70.0"
 authors = ["Lucien Greathouse <me@lpghatguy.com>"]
 description = "Enables professional-grade development tools for Roblox developers"
@@ -40,7 +40,7 @@ name = "build"
 harness = false
 
 [dependencies]
-memofs = { version = "0.2.0", path = "crates/memofs" }
+memofs = { version = "0.3.0", path = "crates/memofs" }
 
 # These dependencies can be uncommented when working on rbx-dom simultaneously
 # rbx_binary = { path = "../rbx-dom/rbx_binary" }
@@ -49,11 +49,11 @@ memofs = { version = "0.2.0", path = "crates/memofs" }
 # rbx_reflection_database = { path = "../rbx-dom/rbx_reflection_database" }
 # rbx_xml = { path = "../rbx-dom/rbx_xml" }
 
-rbx_binary = "0.7.4"
-rbx_dom_weak = "2.7.0"
-rbx_reflection = "4.5.0"
-rbx_reflection_database = "0.2.10"
-rbx_xml = "0.13.3"
+rbx_binary = "0.7.7"
+rbx_dom_weak = "2.9.0"
+rbx_reflection = "4.7.0"
+rbx_reflection_database = "0.2.12"
+rbx_xml = "0.13.5"
 
 anyhow = "1.0.44"
 backtrace = "0.3.61"
@@ -94,7 +94,7 @@ tracy-client = { version = "0.13.2", optional = true }
 winreg = "0.10.1"
 
 [build-dependencies]
-memofs = { version = "0.2.0", path = "crates/memofs" }
+memofs = { version = "0.3.0", path = "crates/memofs" }
 
 embed-resource = "1.6.4"
 anyhow = "1.0.44"

aftman.toml

@@ -1,5 +1,5 @@
 [tools]
-rojo = "rojo-rbx/rojo@7.3.0"
-selene = "Kampfkarren/selene@0.25.0"
+rojo = "rojo-rbx/rojo@7.4.1"
+selene = "Kampfkarren/selene@0.26.1"
 stylua = "JohnnyMorganz/stylua@0.18.2"
 run-in-roblox = "rojo-rbx/run-in-roblox@0.3.0"

crates/memofs/CHANGELOG.md

@@ -1,9 +1,13 @@
 # memofs Changelog
 
 ## Unreleased Changes
+
+## 0.3.0 (2024-03-15)
 * Changed `StdBackend` file watching component to use minimal recursive watches. [#830]
+* Added `Vfs::read_to_string` and `Vfs::read_to_string_lf_normalized` [#854]
 
 [#830]: https://github.com/rojo-rbx/rojo/pull/830
+[#854]: https://github.com/rojo-rbx/rojo/pull/854
 
 ## 0.2.0 (2021-08-23)
 * Updated to `crossbeam-channel` 0.5.1.

crates/memofs/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "memofs"
 description = "Virtual filesystem with configurable backends."
-version = "0.2.0"
+version = "0.3.0"
 authors = ["Lucien Greathouse <me@lpghatguy.com>"]
 edition = "2018"
 readme = "README.md"

crates/memofs/src/lib.rs

@@ -22,9 +22,9 @@ mod noop_backend;
 mod snapshot;
 mod std_backend;
 
-use std::io;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex, MutexGuard};
+use std::{io, str};
 
 pub use in_memory_fs::InMemoryFs;
 pub use noop_backend::NoopBackend;
@@ -155,6 +155,24 @@ impl VfsInner {
         Ok(Arc::new(contents))
     }
 
+    fn read_to_string<P: AsRef<Path>>(&mut self, path: P) -> io::Result<Arc<String>> {
+        let path = path.as_ref();
+        let contents = self.backend.read(path)?;
+
+        if self.watch_enabled {
+            self.backend.watch(path)?;
+        }
+
+        let contents_str = str::from_utf8(&contents).map_err(|_| {
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("File was not valid UTF-8: {}", path.display()),
+            )
+        })?;
+
+        Ok(Arc::new(contents_str.into()))
+    }
+
     fn write<P: AsRef<Path>, C: AsRef<[u8]>>(&mut self, path: P, contents: C) -> io::Result<()> {
         let path = path.as_ref();
         let contents = contents.as_ref();
@@ -258,6 +276,33 @@ impl Vfs {
         self.inner.lock().unwrap().read(path)
     }
 
+    /// Read a file from the VFS (or from the underlying backend if it isn't
+    /// resident) into a string.
+    ///
+    /// Roughly equivalent to [`std::fs::read_to_string`][std::fs::read_to_string].
+    ///
+    /// [std::fs::read_to_string]: https://doc.rust-lang.org/stable/std/fs/fn.read_to_string.html
+    #[inline]
+    pub fn read_to_string<P: AsRef<Path>>(&self, path: P) -> io::Result<Arc<String>> {
+        let path = path.as_ref();
+        self.inner.lock().unwrap().read_to_string(path)
+    }
+
+    /// Read a file from the VFS (or the underlying backend if it isn't
+    /// resident) into a string, and normalize its line endings to LF.
+    ///
+    /// Roughly equivalent to [`std::fs::read_to_string`][std::fs::read_to_string], but also performs
+    /// line ending normalization.
+    ///
+    /// [std::fs::read_to_string]: https://doc.rust-lang.org/stable/std/fs/fn.read_to_string.html
+    #[inline]
+    pub fn read_to_string_lf_normalized<P: AsRef<Path>>(&self, path: P) -> io::Result<Arc<String>> {
+        let path = path.as_ref();
+        let contents = self.inner.lock().unwrap().read_to_string(path)?;
+
+        Ok(contents.replace("\r\n", "\n").into())
+    }
+
     /// Write a file to the VFS and the underlying backend.
     ///
     /// Roughly equivalent to [`std::fs::write`][std::fs::write].
@@ -428,3 +473,23 @@ impl VfsLock<'_> {
         self.inner.commit_event(event)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use crate::{InMemoryFs, Vfs, VfsSnapshot};
+
+    /// https://github.com/rojo-rbx/rojo/issues/899
+    #[test]
+    fn read_to_string_lf_normalized_keeps_trailing_newline() {
+        let mut imfs = InMemoryFs::new();
+        imfs.load_snapshot("test", VfsSnapshot::file("bar\r\nfoo\r\n\r\n"))
+            .unwrap();
+
+        let vfs = Vfs::new(imfs);
+
+        assert_eq!(
+            vfs.read_to_string_lf_normalized("test").unwrap().as_str(),
+            "bar\nfoo\n\n"
+        );
+    }
+}

plugin/Version.txt

@@ -1 +1 @@
-7.4.0
+7.4.4

plugin/http/Error.lua

@@ -3,12 +3,12 @@ Error.__index = Error
 
 Error.Kind = {
 	HttpNotEnabled = {
-		message = "Rojo requires HTTP access, which is not enabled.\n" ..
-			"Check your game settings, located in the 'Home' tab of Studio.",
+		message = "Rojo requires HTTP access, which is not enabled.\n"
+			.. "Check your game settings, located in the 'Home' tab of Studio.",
 	},
 	ConnectFailed = {
-		message = "Couldn't connect to the Rojo server.\n" ..
-			"Make sure the server is running — use 'rojo serve' to run it!",
+		message = "Couldn't connect to the Rojo server.\n"
+			.. "Make sure the server is running — use 'rojo serve' to run it!",
 	},
 	Timeout = {
 		message = "HTTP request timed out.",
@@ -63,4 +63,13 @@ function Error.fromRobloxErrorString(message)
 	return Error.new(Error.Kind.Unknown, message)
 end
 
+function Error.fromResponse(response)
+	local lower = (response.body or ""):lower()
+	if response.code == 408 or response.code == 504 or lower:find("timed? ?out") then
+		return Error.new(Error.Kind.Timeout)
+	end
+
+	return Error.new(Error.Kind.Unknown, string.format("%s: %s", tostring(response.code), tostring(response.body)))
+end
+
 return Error

plugin/http/init.lua

@@ -30,8 +30,13 @@ local function performRequest(requestParams)
 			end)
 
 			if success then
-				Log.trace("Request {} success, status code {}", requestId, response.StatusCode)
-				resolve(HttpResponse.fromRobloxResponse(response))
+				Log.trace("Request {} success, response {:#?}", requestId, response)
+				local httpResponse = HttpResponse.fromRobloxResponse(response)
+				if httpResponse:isSuccess() then
+					resolve(httpResponse)
+				else
+					reject(HttpError.fromResponse(httpResponse))
+				end
 			else
 				Log.trace("Request {} failure: {:?}", requestId, response)
 				reject(HttpError.fromRobloxErrorString(response))

plugin/rbx_dom_lua/EncodedValue.lua

@@ -493,9 +493,32 @@ types = {
 	},
 }
 
+types.OptionalCFrame = {
+	fromPod = function(pod)
+		if pod == nil then
+			return nil
+		else
+			return types.CFrame.fromPod(pod)
+		end
+	end,
+
+	toPod = function(roblox)
+		if roblox == nil then
+			return nil
+		else
+			return types.CFrame.toPod(roblox)
+		end
+	end,
+}
+
 function EncodedValue.decode(encodedValue)
 	local ty, value = next(encodedValue)
 
+	if ty == nil then
+		-- If the encoded pair is empty, assume it is an unoccupied optional value
+		return true, nil
+	end
+
 	local typeImpl = types[ty]
 	if typeImpl == nil then
 		return false, "Couldn't decode value " .. tostring(ty)

plugin/rbx_dom_lua/allValues.json

@@ -370,6 +370,41 @@
     },
     "ty": "NumberSequence"
   },
+  "OptionalCFrame-None": {
+    "value": {
+      "OptionalCFrame": null
+    },
+    "ty": "OptionalCFrame"
+  },
+  "OptionalCFrame-Some": {
+    "value": {
+      "OptionalCFrame": {
+        "position": [
+          0.0,
+          0.0,
+          0.0
+        ],
+        "orientation": [
+          [
+            1.0,
+            0.0,
+            0.0
+          ],
+          [
+            0.0,
+            1.0,
+            0.0
+          ],
+          [
+            0.0,
+            0.0,
+            1.0
+          ]
+        ]
+      }
+    },
+    "ty": "OptionalCFrame"
+  },
   "PhysicalProperties-Custom": {
     "value": {
       "PhysicalProperties": {

plugin/rbx_dom_lua/customProperties.lua

@@ -26,6 +26,21 @@ local TERRAIN_MATERIAL_COLORS = {
 	Enum.Material.Pavement,
 }
 
+local function isAttributeNameValid(attributeName)
+	-- For SetAttribute to succeed, the attribute name must be less than or
+	-- equal to 100 characters...
+	return #attributeName <= 100
+		-- ...and must only contain alphanumeric characters, periods, hyphens,
+		-- underscores, or forward slashes.
+		and attributeName:match("[^%w%.%-_/]") == nil
+end
+
+local function isAttributeNameReserved(attributeName)
+	-- For SetAttribute to succeed, attribute names must not use the RBX
+	-- prefix, which is reserved by Roblox.
+	return attributeName:sub(1, 3) == "RBX"
+end
+
 -- Defines how to read and write properties that aren't directly scriptable.
 --
 -- The reflection database refers to these as having scriptability = "Custom"
@@ -40,26 +55,33 @@ return {
 				local didAllWritesSucceed = true
 
 				for attributeName, attributeValue in pairs(value) do
-					local isNameValid =
-						-- For our SetAttribute to succeed, the attribute name must be
-						-- less than or equal to 100 characters...
-						#attributeName <= 100
-						-- ...must only contain alphanumeric characters, periods, hyphens,
-						-- underscores, or forward slashes...
-						and attributeName:match("[^%w%.%-_/]") == nil
-						-- ... and must not use the RBX prefix, which is reserved by Roblox.
-						and attributeName:sub(1, 3) ~= "RBX"
+					if isAttributeNameReserved(attributeName) then
+						-- If the attribute name is reserved, then we don't
+						-- really care about reporting any failures about
+						-- it.
+						continue
+					end
 
-					if isNameValid then
-						instance:SetAttribute(attributeName, attributeValue)
-					else
+					if not isAttributeNameValid(attributeName) then
 						didAllWritesSucceed = false
-					end
+						continue
 					end
 
-				for key in pairs(existing) do
-					if value[key] == nil then
-						instance:SetAttribute(key, nil)
+					instance:SetAttribute(attributeName, attributeValue)
+				end
+
+				for existingAttributeName in pairs(existing) do
+					if isAttributeNameReserved(existingAttributeName) then
+						continue
+					end
+
+					if not isAttributeNameValid(existingAttributeName) then
+						didAllWritesSucceed = false
+						continue
+					end
+
+					if value[existingAttributeName] == nil then
+						instance:SetAttribute(existingAttributeName, nil)
 					end
 				end
 
@@ -111,6 +133,19 @@ return {
 				return true, instance:ScaleTo(value)
 			end,
 		},
+		WorldPivotData = {
+			read = function(instance)
+				return true, instance.WorldPivot
+			end,
+			write = function(instance, _, value)
+				if value == nil then
+					return true, nil
+				else
+					instance.WorldPivot = value
+					return true
+				end
+			end,
+		},
 	},
 	Terrain = {
 		MaterialColors = {

plugin/src/ApiContext.lua

@@ -185,10 +185,10 @@ function ApiContext:write(patch)
 
 	body = Http.jsonEncode(body)
 
-	return Http.post(url, body):andThen(rejectFailedRequests):andThen(Http.Response.json):andThen(function(body)
-		Log.info("Write response: {:?}", body)
+	return Http.post(url, body):andThen(rejectFailedRequests):andThen(Http.Response.json):andThen(function(responseBody)
+		Log.info("Write response: {:?}", responseBody)
 
-		return body
+		return responseBody
 	end)
 end
 

plugin/src/App/Components/Dropdown.lua

@@ -167,7 +167,7 @@ function Dropdown:render()
 								self.setContentSize(object.AbsoluteContentSize)
 							end,
 						}),
-						Roact.createFragment(optionButtons),
+						Options = Roact.createFragment(optionButtons),
 					}),
 				})
 				else nil,

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

@@ -57,7 +57,7 @@ local function DisplayValue(props)
 				-- 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
+				for _, v in props.value do
 					i += 1
 
 					-- Wrap strings in quotes

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

@@ -97,21 +97,16 @@ function DomLabel:render()
 		-- Line guides help indent depth remain readable
 		local lineGuides = {}
 		for i = 1, props.depth or 0 do
-			table.insert(
-				lineGuides,
-				e("Frame", {
-					Name = "Line_" .. i,
+			lineGuides["Line_" .. i] = e("Frame", {
 				Size = UDim2.new(0, 2, 1, 2),
 				Position = UDim2.new(0, (20 * i) + 15, 0, -1),
 				BorderSizePixel = 0,
 				BackgroundTransparency = props.transparency,
 				BackgroundColor3 = theme.BorderedContainer.BorderColor,
 			})
-			)
 		end
 
 		return e("Frame", {
-			Name = "Change",
 			ClipsDescendants = true,
 			BackgroundColor3 = if props.patchType then theme.Diff[props.patchType] else nil,
 			BorderSizePixel = 0,

plugin/src/App/Components/TextButton.lua

@@ -42,7 +42,7 @@ end
 function TextButton:render()
 	return Theme.with(function(theme)
 		local textSize =
-			TextService:GetTextSize(self.props.text, 18, Enum.Font.GothamSemibold, Vector2.new(math.huge, math.huge))
+			TextService:GetTextSize(self.props.text, 18, Enum.Font.GothamMedium, Vector2.new(math.huge, math.huge))
 
 		local style = self.props.style
 
@@ -83,7 +83,7 @@ function TextButton:render()
 
 			Text = e("TextLabel", {
 				Text = self.props.text,
-				Font = Enum.Font.GothamSemibold,
+				Font = Enum.Font.GothamMedium,
 				TextSize = 18,
 				TextColor3 = bindingUtil.mapLerp(bindingEnabled, theme.Enabled.TextColor, theme.Disabled.TextColor),
 				TextTransparency = self.props.transparency,

plugin/src/App/StatusPages/Confirming.lua

@@ -1,5 +1,3 @@
-local TextService = game:GetService("TextService")
-
 local Rojo = script:FindFirstAncestor("Rojo")
 local Plugin = Rojo.Plugin
 local Packages = Rojo.Packages

plugin/src/App/StatusPages/Settings/init.lua

@@ -26,7 +26,7 @@ local function invertTbl(tbl)
 end
 
 local invertedLevels = invertTbl(Log.Level)
-local confirmationBehaviors = { "Initial", "Always", "Large Changes", "Unlisted PlaceId" }
+local confirmationBehaviors = { "Initial", "Always", "Large Changes", "Unlisted PlaceId", "Never" }
 
 local function Navbar(props)
 	return Theme.with(function(theme)

plugin/src/App/Theme.lua

@@ -19,7 +19,6 @@ local Rojo = script:FindFirstAncestor("Rojo")
 local Packages = Rojo.Packages
 
 local Roact = require(Packages.Roact)
-local Log = require(Packages.Log)
 
 local strict = require(script.Parent.Parent.strict)
 

plugin/src/App/init.lua

@@ -516,6 +516,9 @@ function App:startSession()
 					return "Accept"
 				end
 			end
+		elseif confirmationBehavior == "Never" then
+			Log.trace("Accepting patch without confirmation because behavior is set to Never")
+			return "Accept"
 		end
 
 		-- The datamodel name gets overwritten by Studio, making confirmation of it intrusive

plugin/src/Config.lua

@@ -3,7 +3,8 @@ local strict = require(script.Parent.strict)
 local isDevBuild = script.Parent.Parent:FindFirstChild("ROJO_DEV_BUILD") ~= nil
 
 local Version = script.Parent.Parent.Version
-local major, minor, patch, metadata = Version.Value:match("^(%d+)%.(%d+)%.(%d+)(.*)$")
+local trimmedVersionValue = Version.Value:gsub("^%s+", ""):gsub("%s+$", "")
+local major, minor, patch, metadata = trimmedVersionValue:match("^(%d+)%.(%d+)%.(%d+)(.*)$")
 
 local realVersion = { major, minor, patch, metadata }
 for i = 1, 3 do

plugin/src/InstanceMap.lua

@@ -113,27 +113,29 @@ end
 function InstanceMap:destroyInstance(instance)
 	local id = self.fromInstances[instance]
 
+	local descendants = instance:GetDescendants()
+	instance:Destroy()
+
+	-- After the instance is successfully destroyed,
+	-- we can remove all the id mappings
+
 	if id ~= nil then
 		self:removeId(id)
 	end
 
-	for _, descendantInstance in ipairs(instance:GetDescendants()) do
+	for _, descendantInstance in descendants do
 		self:removeInstance(descendantInstance)
 	end
-
-	instance:Destroy()
 end
 
 function InstanceMap:destroyId(id)
 	local instance = self.fromIds[id]
-	self:removeId(id)
-
 	if instance ~= nil then
-		for _, descendantInstance in ipairs(instance:GetDescendants()) do
-			self:removeInstance(descendantInstance)
-		end
-
-		instance:Destroy()
+		self:destroyInstance(instance)
+	else
+		-- There is no instance with this id, so we can just remove the id
+		-- without worrying about instance destruction
+		self:removeId(id)
 	end
 end
 

plugin/src/PatchTree.lua

@@ -426,22 +426,71 @@ function PatchTree.updateMetadata(tree, patch, instanceMap, unappliedPatch)
 	-- Update isWarning metadata
 	for _, failedChange in unappliedPatch.updated do
 		local node = tree:getNode(failedChange.id)
-		if node then
+		if not node then
+			continue
+		end
+
 		node.isWarning = true
 		Log.trace("Marked node as warning: {} {}", node.id, node.name)
 
-			if node.changeList then
-				for _, change in node.changeList do
-					if failedChange.changedProperties[change[1]] then
-						Log.trace("  Marked property as warning: {}", change[1])
-						if change[4] == nil then
-							change[4] = {}
+		if not node.changeList then
+			continue
 		end
+		for _, change in node.changeList do
+			local property = change[1]
+			local propertyFailedToApply = if property == "Name"
+				then failedChange.changedName ~= nil -- Name is not in changedProperties, so it needs a special case
+				else failedChange.changedProperties[property] ~= nil
+
+			if not propertyFailedToApply then
+				-- This change didn't fail, no need to mark
+				continue
+			end
+			if change[4] == nil then
+				change[4] = { isWarning = true }
+			else
 				change[4].isWarning = true
 			end
+			Log.trace("  Marked property as warning: {}.{}", node.name, property)
 		end
 	end
+	for failedAdditionId in unappliedPatch.added do
+		local node = tree:getNode(failedAdditionId)
+		if not node then
+			continue
 		end
+
+		node.isWarning = true
+		Log.trace("Marked node as warning: {} {}", node.id, node.name)
+
+		if not node.changeList then
+			continue
+		end
+		for _, change in node.changeList do
+			-- Failed addition means that all properties failed to be added
+			if change[4] == nil then
+				change[4] = { isWarning = true }
+			else
+				change[4].isWarning = true
+			end
+			Log.trace("  Marked property as warning: {}.{}", node.name, change[1])
+		end
+	end
+	for _, failedRemovalIdOrInstance in unappliedPatch.removed do
+		local failedRemovalId = if Types.RbxId(failedRemovalIdOrInstance)
+			then failedRemovalIdOrInstance
+			else instanceMap.fromInstances[failedRemovalIdOrInstance]
+		if not failedRemovalId then
+			continue
+		end
+
+		local node = tree:getNode(failedRemovalId)
+		if not node then
+			continue
+		end
+
+		node.isWarning = true
+		Log.trace("Marked node as warning: {} {}", node.id, node.name)
 	end
 
 	-- Update if instances exist

plugin/src/Reconciler/applyPatch.lua

@@ -25,11 +25,16 @@ local function applyPatch(instanceMap, patch)
 	local unappliedPatch = PatchSet.newEmpty()
 
 	for _, removedIdOrInstance in ipairs(patch.removed) do
+		local removeInstanceSuccess = pcall(function()
 			if Types.RbxId(removedIdOrInstance) then
 				instanceMap:destroyId(removedIdOrInstance)
 			else
 				instanceMap:destroyInstance(removedIdOrInstance)
 			end
+		end)
+		if not removeInstanceSuccess then
+			table.insert(unappliedPatch.removed, removedIdOrInstance)
+		end
 	end
 
 	for id, virtualInstance in pairs(patch.added) do
@@ -170,7 +175,13 @@ local function applyPatch(instanceMap, patch)
 		end
 
 		if update.changedName ~= nil then
+			local setNameSuccess = pcall(function()
 				instance.Name = update.changedName
+			end)
+			if not setNameSuccess then
+				unappliedUpdate.changedName = update.changedName
+				partiallyApplied = true
+			end
 		end
 
 		if update.changedMetadata ~= nil then
@@ -183,15 +194,15 @@ local function applyPatch(instanceMap, patch)
 
 		if update.changedProperties ~= nil then
 			for propertyName, propertyValue in pairs(update.changedProperties) do
-				local ok, decodedValue = decodeValue(propertyValue, instanceMap)
-				if not ok then
+				local decodeSuccess, decodedValue = decodeValue(propertyValue, instanceMap)
+				if not decodeSuccess then
 					unappliedUpdate.changedProperties[propertyName] = propertyValue
 					partiallyApplied = true
 					continue
 				end
 
-				local ok = setProperty(instance, propertyName, decodedValue)
-				if not ok then
+				local setPropertySuccess = setProperty(instance, propertyName, decodedValue)
+				if not setPropertySuccess then
 					unappliedUpdate.changedProperties[propertyName] = propertyValue
 					partiallyApplied = true
 				end

plugin/src/Reconciler/decodeValue.lua

@@ -27,9 +27,9 @@ local function decodeValue(encodedValue, instanceMap)
 		end
 	end
 
-	local ok, decodedValue = RbxDom.EncodedValue.decode(encodedValue)
+	local decodeSuccess, decodedValue = RbxDom.EncodedValue.decode(encodedValue)
 
-	if not ok then
+	if not decodeSuccess then
 		return false,
 			Error.new(Error.CannotDecodeValue, {
 				encodedValue = encodedValue,

plugin/src/Reconciler/diff.lua

@@ -147,13 +147,13 @@ local function diff(instanceMap, virtualInstances, rootId)
 
 		local changedProperties = {}
 		for propertyName, virtualValue in pairs(virtualInstance.Properties) do
-			local ok, existingValueOrErr = getProperty(instance, propertyName)
+			local getProperySuccess, existingValueOrErr = getProperty(instance, propertyName)
 
-			if ok then
+			if getProperySuccess then
 				local existingValue = existingValueOrErr
-				local ok, decodedValue = decodeValue(virtualValue, instanceMap)
+				local decodeSuccess, decodedValue = decodeValue(virtualValue, instanceMap)
 
-				if ok then
+				if decodeSuccess then
 					if not trueEquals(existingValue, decodedValue) then
 						Log.debug(
 							"{}.{} changed from '{}' to '{}'",
@@ -165,7 +165,6 @@ local function diff(instanceMap, virtualInstances, rootId)
 						changedProperties[propertyName] = virtualValue
 					end
 				else
-					local propertyType = next(virtualValue)
 					Log.warn(
 						"Failed to decode property {}.{}. Encoded property was: {:#?}",
 						virtualInstance.ClassName,
@@ -178,10 +177,8 @@ local function diff(instanceMap, virtualInstances, rootId)
 
 				if err.kind == Error.UnknownProperty then
 					Log.trace("Skipping unknown property {}.{}", err.details.className, err.details.propertyName)
-				elseif err.kind == Error.UnreadableProperty then
-					Log.trace("Skipping unreadable property {}.{}", err.details.className, err.details.propertyName)
 				else
-					return false, err
+					Log.trace("Skipping unreadable property {}.{}", err.details.className, err.details.propertyName)
 				end
 			end
 		end
@@ -220,9 +217,9 @@ local function diff(instanceMap, virtualInstances, rootId)
 					table.insert(patch.removed, childInstance)
 				end
 			else
-				local ok, err = diffInternal(childId)
+				local diffSuccess, err = diffInternal(childId)
 
-				if not ok then
+				if not diffSuccess then
 					return false, err
 				end
 			end
@@ -243,9 +240,9 @@ local function diff(instanceMap, virtualInstances, rootId)
 		return true
 	end
 
-	local ok, err = diffInternal(rootId)
+	local diffSuccess, err = diffInternal(rootId)
 
-	if not ok then
+	if not diffSuccess then
 		return false, err
 	end
 

plugin/src/Reconciler/hydrate.lua

@@ -31,13 +31,13 @@ local function hydrate(instanceMap, virtualInstances, rootId, rootInstance)
 				-- We guard accessing Name and ClassName in order to avoid
 				-- tripping over children of DataModel that Rojo won't have
 				-- permissions to access at all.
-				local ok, name, className = pcall(function()
+				local accessSuccess, name, className = pcall(function()
 					return childInstance.Name, childInstance.ClassName
 				end)
 
 				-- This rule is very conservative and could be loosened in the
 				-- future, or more heuristics could be introduced.
-				if ok and name == virtualChild.Name and className == virtualChild.ClassName then
+				if accessSuccess and name == virtualChild.Name and className == virtualChild.ClassName then
 					isExistingChildVisited[childIndex] = true
 					hydrate(instanceMap, virtualInstances, childId, childInstance)
 					break

plugin/src/Reconciler/reify.lua

@@ -53,9 +53,9 @@ function reifyInner(instanceMap, virtualInstances, id, parentInstance, unapplied
 	-- Instance.new can fail if we're passing in something that can't be
 	-- created, like a service, something enabled with a feature flag, or
 	-- something that requires higher security than we have.
-	local ok, instance = pcall(Instance.new, virtualInstance.ClassName)
+	local createSuccess, instance = pcall(Instance.new, virtualInstance.ClassName)
 
-	if not ok then
+	if not createSuccess then
 		addAllToPatch(unappliedPatch, virtualInstances, id)
 		return
 	end
@@ -80,14 +80,14 @@ function reifyInner(instanceMap, virtualInstances, id, parentInstance, unapplied
 			continue
 		end
 
-		local ok, value = decodeValue(virtualValue, instanceMap)
-		if not ok then
+		local decodeSuccess, value = decodeValue(virtualValue, instanceMap)
+		if not decodeSuccess then
 			unappliedProperties[propertyName] = virtualValue
 			continue
 		end
 
-		local ok = setProperty(instance, propertyName, value)
-		if not ok then
+		local setPropertySuccess = setProperty(instance, propertyName, value)
+		if not setPropertySuccess then
 			unappliedProperties[propertyName] = virtualValue
 		end
 	end
@@ -148,8 +148,8 @@ function applyDeferredRefs(instanceMap, deferredRefs, unappliedPatch)
 			continue
 		end
 
-		local ok = setProperty(entry.instance, entry.propertyName, targetInstance)
-		if not ok then
+		local setPropertySuccess = setProperty(entry.instance, entry.propertyName, targetInstance)
+		if not setPropertySuccess then
 			markFailed(entry.id, entry.propertyName, entry.virtualValue)
 		end
 	end

plugin/src/Reconciler/reify.spec.lua

@@ -3,7 +3,6 @@ return function()
 
 	local PatchSet = require(script.Parent.Parent.PatchSet)
 	local InstanceMap = require(script.Parent.Parent.InstanceMap)
-	local Error = require(script.Parent.Error)
 
 	local function isEmpty(table)
 		return next(table) == nil, "Table was not empty"

plugin/src/Reconciler/setProperty.lua

@@ -7,7 +7,7 @@ local Log = require(Packages.Log)
 local RbxDom = require(Packages.RbxDom)
 local Error = require(script.Parent.Error)
 
-local function setProperty(instance, propertyName, value)
+local function setProperty(instance: Instance, propertyName: string, value: unknown): boolean
 	local descriptor = RbxDom.findCanonicalPropertyDescriptor(instance.ClassName, propertyName)
 
 	-- We can skip unknown properties; they're not likely reflected to Lua.
@@ -28,9 +28,16 @@ local function setProperty(instance, propertyName, value)
 			})
 	end
 
-	local ok, err = descriptor:write(instance, value)
+	if value == nil then
+		if descriptor.dataType == "Float32" or descriptor.dataType == "Float64" then
+			Log.trace("Skipping nil {} property {}.{}", descriptor.dataType, instance.ClassName, propertyName)
+			return true
+		end
+	end
 
-	if not ok then
+	local writeSuccess, err = descriptor:write(instance, value)
+
+	if not writeSuccess then
 		if err.kind == RbxDom.Error.Kind.Roblox and err.extra:find("lacking permission") then
 			return false,
 				Error.new(Error.LackingPropertyPermissions, {

plugin/src/ServeSession.lua

@@ -21,8 +21,8 @@ local Status = strict("Session.Status", {
 	Disconnected = "Disconnected",
 })
 
-local function debugPatch(patch)
-	return Fmt.debugify(patch, function(patch, output)
+local function debugPatch(object)
+	return Fmt.debugify(object, function(patch, output)
 		output:writeLine("Patch {{")
 		output:indent()
 
@@ -197,7 +197,7 @@ function ServeSession:__onActiveScriptChanged(activeScript)
 		local existingParent = activeScript.Parent
 		activeScript.Parent = nil
 
-		for i = 1, 3 do
+		for _ = 1, 3 do
 			RunService.Heartbeat:Wait()
 		end
 
@@ -251,7 +251,10 @@ function ServeSession:__initialSync(serverInfo)
 
 		if userDecision == "Abort" then
 			return Promise.reject("Aborted Rojo sync operation")
-		elseif userDecision == "Reject" and self.__twoWaySync then
+		elseif userDecision == "Reject" then
+			if not self.__twoWaySync then
+				return Promise.reject("Cannot reject sync operation without two-way sync enabled")
+			end
 			-- The user wants their studio DOM to write back to their Rojo DOM
 			-- so we will reverse the patch and send it back
 
@@ -268,7 +271,7 @@ function ServeSession:__initialSync(serverInfo)
 				table.insert(inversePatch.updated, update)
 			end
 			-- Add the removed instances back to Rojo
-			-- selene:allow(empty_if, unused_variable)
+			-- selene:allow(empty_if, unused_variable, empty_loop)
 			for _, instance in catchUpPatch.removed do
 				-- TODO: Generate ID for our instance and add it to inversePatch.added
 			end
@@ -277,7 +280,7 @@ function ServeSession:__initialSync(serverInfo)
 				table.insert(inversePatch.removed, id)
 			end
 
-			self.__apiContext:write(inversePatch)
+			return self.__apiContext:write(inversePatch)
 		elseif userDecision == "Accept" then
 			local unappliedPatch = self.__reconciler:applyPatch(catchUpPatch)
 
@@ -287,6 +290,10 @@ function ServeSession:__initialSync(serverInfo)
 					PatchSet.humanSummary(self.__instanceMap, unappliedPatch)
 				)
 			end
+
+			return Promise.resolve()
+		else
+			return Promise.reject("Invalid user decision: " .. userDecision)
 		end
 	end)
 end

plugin/src/createSignal.lua

@@ -1,53 +0,0 @@
---[[
-	Create a new signal that can be connected to, disconnected from, and fired.
-
-	Usage:
-
-		local signal = createSignal()
-		local disconnect = signal:connect(function(...)
-			print("fired:", ...)
-		end)
-
-		signal:fire("a", "b", "c")
-		disconnect()
-
-	Avoids mutating listeners list directly to prevent iterator invalidation if
-	a listener is disconnected while the signal is firing.
-]]
-local function createSignal()
-	local listeners = {}
-
-	local function connect(newListener)
-		local nextListeners = {}
-		for listener in pairs(listeners) do
-			nextListeners[listener] = true
-		end
-
-		nextListeners[newListener] = true
-		listeners = nextListeners
-
-		return function()
-			local nextListeners = {}
-			for listener in pairs(listeners) do
-				if listener ~= newListener then
-					nextListeners[listener] = true
-				end
-			end
-
-			listeners = nextListeners
-		end
-	end
-
-	local function fire(...)
-		for listener in pairs(listeners) do
-			listener(...)
-		end
-	end
-
-	return {
-		connect = connect,
-		fire = fire,
-	}
-end
-
-return createSignal

rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_default_project.snap

@@ -0,0 +1,22 @@
+---
+source: tests/tests/build.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">top-level</string>
+    </Properties>
+    <Item class="Folder" referent="1">
+      <Properties>
+        <string name="Name">second-level</string>
+      </Properties>
+      <Item class="IntValue" referent="2">
+        <Properties>
+          <string name="Name">third-level</string>
+          <int64 name="Value">1337</int64>
+        </Properties>
+      </Item>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_project.snap

@@ -0,0 +1,22 @@
+---
+source: tests/tests/build.rs
+expression: contents
+---
+<roblox version="4">
+  <Item class="Folder" referent="0">
+    <Properties>
+      <string name="Name">no_name_project</string>
+    </Properties>
+    <Item class="Folder" referent="1">
+      <Properties>
+        <string name="Name">second-level</string>
+      </Properties>
+      <Item class="BoolValue" referent="2">
+        <Properties>
+          <string name="Name">bool_value</string>
+          <bool name="Value">true</bool>
+        </Properties>
+      </Item>
+    </Item>
+  </Item>
+</roblox>

rojo-test/build-test-snapshots/end_to_end__tests__build__no_name_top_level_project.snap

@@ -0,0 +1,13 @@
+---
+source: tests/tests/build.rs
+assertion_line: 104
+expression: contents
+---
+<roblox version="4">
+  <Item class="StringValue" referent="0">
+    <Properties>
+      <string name="Name">no_name_top_level_project</string>
+      <string name="Value">If this isn't named `no_name_top_level_project`, something went wrong!</string>
+    </Properties>
+  </Item>
+</roblox>

rojo-test/build-test-snapshots/end_to_end__tests__build__rbxmx_in_folder.snap

@@ -1,7 +1,6 @@
 ---
 source: tests/tests/build.rs
 expression: contents
-
 ---
 <roblox version="4">
   <Item class="Folder" referent="0">
@@ -25,6 +24,7 @@ expression: contents
           <R21>0</R21>
           <R22>1</R22>
         </CoordinateFrame>
+        <bool name="NeedsPivotMigration">false</bool>
         <Ref name="PrimaryPart">null</Ref>
         <BinaryString name="Tags"></BinaryString>
       </Properties>

rojo-test/build-test-snapshots/end_to_end__tests__build__unresolved_values.snap

@@ -1,7 +1,6 @@
 ---
 source: tests/tests/build.rs
 expression: contents
-
 ---
 <roblox version="4">
   <Item class="DataModel" referent="0">
@@ -22,6 +21,7 @@ expression: contents
     <Item class="Workspace" referent="2">
       <Properties>
         <string name="Name">Workspace</string>
+        <bool name="NeedsPivotMigration">false</bool>
       </Properties>
       <Item class="BoolValue" referent="3">
         <Properties>

rojo-test/build-tests/no_name_default_project/default.project.json

@@ -0,0 +1,9 @@
+{
+  "name": "top-level",
+  "tree": {
+    "$className": "Folder",
+    "second-level": {
+      "$path": "src"
+    }
+  }
+}

rojo-test/build-tests/no_name_default_project/src/third-level/default.project.json

@@ -0,0 +1,8 @@
+{
+  "tree": {
+    "$className": "IntValue",
+    "$properties": {
+      "Value": 1337
+    }
+  }
+}

rojo-test/build-tests/no_name_project/default.project.json

@@ -0,0 +1,9 @@
+{
+  "name": "no_name_project",
+  "tree": {
+    "$className": "Folder",
+    "second-level": {
+      "$path": "src"
+    }
+  }
+}

rojo-test/build-tests/no_name_project/src/bool_value.project.json

@@ -0,0 +1,8 @@
+{
+  "tree": {
+    "$className": "BoolValue",
+    "$properties": {
+      "Value": true
+    }
+  }
+}

rojo-test/build-tests/no_name_top_level_project/default.project.json

@@ -0,0 +1,8 @@
+{
+  "tree": {
+    "$className": "StringValue",
+    "$properties": {
+      "Value": "If this isn't named `no_name_top_level_project`, something went wrong!"
+    }
+  }
+}

rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_json_model_all-2.snap

@@ -1,7 +1,6 @@
 ---
 source: tests/tests/serve.rs
 expression: "read_response.intern_and_redact(&mut redactions, root_id)"
-
 ---
 instances:
   id-2:
@@ -22,7 +21,9 @@ instances:
       ignoreUnknownInstances: false
     Name: test
     Parent: id-2
-    Properties: {}
+    Properties:
+      NeedsPivotMigration:
+        Bool: false
 messageCursor: 1
 sessionId: id-1
 

rojo-test/serve-test-snapshots/end_to_end__tests__serve__empty_json_model_subscribe.snap

@@ -1,7 +1,6 @@
 ---
 source: tests/tests/serve.rs
 expression: "subscribe_response.intern_and_redact(&mut redactions, ())"
-
 ---
 messageCursor: 1
 messages:
@@ -14,7 +13,9 @@ messages:
           ignoreUnknownInstances: false
         Name: test
         Parent: id-2
-        Properties: {}
+        Properties:
+          NeedsPivotMigration:
+            Bool: false
     removed: []
     updated: []
 sessionId: id-1

rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_all.snap

@@ -0,0 +1,39 @@
+---
+source: tests/tests/serve.rs
+expression: "read_response.intern_and_redact(&mut redactions, root_id)"
+---
+instances:
+  id-2:
+    Children:
+      - id-3
+    ClassName: Folder
+    Id: id-2
+    Metadata:
+      ignoreUnknownInstances: true
+    Name: top-level
+    Parent: "00000000000000000000000000000000"
+    Properties: {}
+  id-3:
+    Children:
+      - id-4
+    ClassName: Folder
+    Id: id-3
+    Metadata:
+      ignoreUnknownInstances: false
+    Name: second-level
+    Parent: id-2
+    Properties: {}
+  id-4:
+    Children: []
+    ClassName: IntValue
+    Id: id-4
+    Metadata:
+      ignoreUnknownInstances: true
+    Name: third-level
+    Parent: id-3
+    Properties:
+      Value:
+        Int64: 1337
+messageCursor: 0
+sessionId: id-1
+

rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_default_project_info.snap

@@ -0,0 +1,14 @@
+---
+source: tests/tests/serve.rs
+assertion_line: 316
+expression: redactions.redacted_yaml(info)
+---
+expectedPlaceIds: ~
+gameId: ~
+placeId: ~
+projectName: top-level
+protocolVersion: 4
+rootInstanceId: id-2
+serverVersion: "[server-version]"
+sessionId: id-1
+

rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_all.snap

@@ -0,0 +1,40 @@
+---
+source: tests/tests/serve.rs
+assertion_line: 338
+expression: "read_response.intern_and_redact(&mut redactions, root_id)"
+---
+instances:
+  id-2:
+    Children:
+      - id-3
+    ClassName: Folder
+    Id: id-2
+    Metadata:
+      ignoreUnknownInstances: true
+    Name: no_name_project
+    Parent: "00000000000000000000000000000000"
+    Properties: {}
+  id-3:
+    Children:
+      - id-4
+    ClassName: Folder
+    Id: id-3
+    Metadata:
+      ignoreUnknownInstances: false
+    Name: second-level
+    Parent: id-2
+    Properties: {}
+  id-4:
+    Children: []
+    ClassName: BoolValue
+    Id: id-4
+    Metadata:
+      ignoreUnknownInstances: true
+    Name: bool_value
+    Parent: id-3
+    Properties:
+      Value:
+        Bool: true
+messageCursor: 0
+sessionId: id-1
+

rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_project_info.snap

@@ -0,0 +1,14 @@
+---
+source: tests/tests/serve.rs
+assertion_line: 335
+expression: redactions.redacted_yaml(info)
+---
+expectedPlaceIds: ~
+gameId: ~
+placeId: ~
+projectName: no_name_project
+protocolVersion: 4
+rootInstanceId: id-2
+serverVersion: "[server-version]"
+sessionId: id-1
+

rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_all-2.snap

@@ -0,0 +1,19 @@
+---
+source: tests/tests/serve.rs
+expression: "read_response.intern_and_redact(&mut redactions, root_id)"
+---
+instances:
+  id-2:
+    Children: []
+    ClassName: StringValue
+    Id: id-2
+    Metadata:
+      ignoreUnknownInstances: true
+    Name: no_name_top_level_project
+    Parent: "00000000000000000000000000000000"
+    Properties:
+      Value:
+        String: "If this isn't named `no_name_top_level_project`, something went wrong!"
+messageCursor: 0
+sessionId: id-1
+

rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_all.snap

@@ -0,0 +1,20 @@
+---
+source: tests/tests/serve.rs
+assertion_line: 306
+expression: "read_response.intern_and_redact(&mut redactions, root_id)"
+---
+instances:
+  id-2:
+    Children: []
+    ClassName: StringValue
+    Id: id-2
+    Metadata:
+      ignoreUnknownInstances: true
+    Name: no_name_top_level_project
+    Parent: "00000000000000000000000000000000"
+    Properties:
+      Value:
+        String: "If this isn't named `no_name_top_level_project`, something went wrong!"
+messageCursor: 0
+sessionId: id-1
+

rojo-test/serve-test-snapshots/end_to_end__tests__serve__no_name_top_level_project_info.snap

@@ -0,0 +1,14 @@
+---
+source: tests/tests/serve.rs
+assertion_line: 300
+expression: redactions.redacted_yaml(info)
+---
+expectedPlaceIds: ~
+gameId: ~
+placeId: ~
+projectName: no_name_top_level_project
+protocolVersion: 4
+rootInstanceId: id-2
+serverVersion: "[server-version]"
+sessionId: id-1
+

rojo-test/serve-tests/no_name_default_project/default.project.json

@@ -0,0 +1,9 @@
+{
+  "name": "top-level",
+  "tree": {
+    "$className": "Folder",
+    "second-level": {
+      "$path": "src"
+    }
+  }
+}

rojo-test/serve-tests/no_name_default_project/src/third-level/default.project.json

@@ -0,0 +1,8 @@
+{
+  "tree": {
+    "$className": "IntValue",
+    "$properties": {
+      "Value": 1337
+    }
+  }
+}

rojo-test/serve-tests/no_name_project/default.project.json

@@ -0,0 +1,9 @@
+{
+  "name": "no_name_project",
+  "tree": {
+    "$className": "Folder",
+    "second-level": {
+      "$path": "src"
+    }
+  }
+}

rojo-test/serve-tests/no_name_project/src/bool_value.project.json

@@ -0,0 +1,8 @@
+{
+  "tree": {
+    "$className": "BoolValue",
+    "$properties": {
+      "Value": true
+    }
+  }
+}

rojo-test/serve-tests/no_name_top_level_project/default.project.json

@@ -0,0 +1,8 @@
+{
+  "tree": {
+    "$className": "StringValue",
+    "$properties": {
+      "Value": "If this isn't named `no_name_top_level_project`, something went wrong!"
+    }
+  }
+}

src/cli/fmt_project.rs

@@ -2,6 +2,7 @@ use std::path::PathBuf;
 
 use anyhow::Context;
 use clap::Parser;
+use memofs::Vfs;
 
 use crate::project::Project;
 
@@ -17,8 +18,11 @@ pub struct FmtProjectCommand {
 
 impl FmtProjectCommand {
     pub fn run(self) -> anyhow::Result<()> {
+        let vfs = Vfs::new_default();
+        vfs.set_watch_enabled(false);
+
         let base_path = resolve_path(&self.project);
-        let project = Project::load_fuzzy(&base_path)?
+        let project = Project::load_fuzzy(&vfs, &base_path)?
             .context("A project file is required to run 'rojo fmt-project'")?;
 
         let serialized = serde_json::to_string_pretty(&project)

src/project.rs

@@ -1,10 +1,12 @@
 use std::{
     collections::{BTreeMap, HashMap, HashSet},
+    ffi::OsStr,
     fs, io,
     net::IpAddr,
     path::{Path, PathBuf},
 };
 
+use memofs::Vfs;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
@@ -19,6 +21,14 @@ pub struct ProjectError(#[from] Error);
 
 #[derive(Debug, Error)]
 enum Error {
+    #[error("The folder for the provided project cannot be used as a project name: {}\n\
+Consider setting the `name` field on this project.", .path.display())]
+    FolderNameInvalid { path: PathBuf },
+
+    #[error("The file name of the provided project cannot be used as a project name: {}.\n\
+Consider setting the `name` field on this project.", .path.display())]
+    ProjectNameInvalid { path: PathBuf },
+
     #[error(transparent)]
     Io {
         #[from]
@@ -39,7 +49,7 @@ enum Error {
 #[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct Project {
     /// The name of the top-level instance described by the project.
-    pub name: String,
+    pub name: Option<String>,
 
     /// The tree of instances described by this project. Projects always
     /// describe at least one instance.
@@ -129,43 +139,91 @@ impl Project {
         }
     }
 
-    pub fn load_from_slice(
-        contents: &[u8],
-        project_file_location: &Path,
-    ) -> Result<Self, ProjectError> {
-        let mut project: Self = serde_json::from_slice(contents).map_err(|source| Error::Json {
-            source,
-            path: project_file_location.to_owned(),
+    /// Sets the name of a project. The order it handles is as follows:
+    ///
+    /// - If the project is a `default.project.json`, uses the folder's name
+    /// - If a fallback is specified, uses that blindly
+    /// - Otherwise, loops through sync rules (including the default ones!) and
+    ///   uses the name of the first one that matches and is a project file
+    fn set_file_name(&mut self, fallback: Option<&str>) -> Result<(), Error> {
+        let file_name = self
+            .file_location
+            .file_name()
+            .and_then(OsStr::to_str)
+            .ok_or_else(|| Error::ProjectNameInvalid {
+                path: self.file_location.clone(),
             })?;
 
-        project.file_location = project_file_location.to_path_buf();
+        // If you're editing this to be generic, make sure you also alter the
+        // snapshot middleware to support generic init paths.
+        if file_name == PROJECT_FILENAME {
+            let folder_name = self.folder_location().file_name().and_then(OsStr::to_str);
+            if let Some(folder_name) = folder_name {
+                self.name = Some(folder_name.to_string());
+            } else {
+                return Err(Error::FolderNameInvalid {
+                    path: self.file_location.clone(),
+                });
+            }
+        } else if let Some(fallback) = fallback {
+            self.name = Some(fallback.to_string());
+        } else {
+            unimplemented!(
+                "7.4.X branch will hopefully never have a case where fallback isn't provided to set_file_name"
+            );
+        }
+
+        Ok(())
+    }
+
+    /// Loads a Project file from the provided contents with its source set as
+    /// the provided location.
+    fn load_from_slice(
+        contents: &[u8],
+        project_file_location: PathBuf,
+        fallback_name: Option<&str>,
+    ) -> Result<Self, Error> {
+        let mut project: Self = serde_json::from_slice(contents).map_err(|source| Error::Json {
+            source,
+            path: project_file_location.clone(),
+        })?;
+        project.file_location = project_file_location;
         project.check_compatibility();
+        if project.name.is_none() {
+            project.set_file_name(fallback_name)?;
+        }
+
         Ok(project)
     }
 
-    pub fn load_fuzzy(fuzzy_project_location: &Path) -> Result<Option<Self>, ProjectError> {
+    /// Loads a Project from a path. This will find the project if it refers to
+    /// a `.project.json` file or if it refers to a directory that contains a
+    /// file named `default.project.json`.
+    pub fn load_fuzzy(
+        vfs: &Vfs,
+        fuzzy_project_location: &Path,
+    ) -> Result<Option<Self>, ProjectError> {
         if let Some(project_path) = Self::locate(fuzzy_project_location) {
-            let project = Self::load_exact(&project_path)?;
-
-            Ok(Some(project))
+            let contents = vfs.read(&project_path).map_err(Error::from)?;
+            Ok(Some(Self::load_from_slice(&contents, project_path, None)?))
         } else {
             Ok(None)
         }
     }
 
-    fn load_exact(project_file_location: &Path) -> Result<Self, Error> {
-        let contents = fs::read_to_string(project_file_location)?;
-
-        let mut project: Project =
-            serde_json::from_str(&contents).map_err(|source| Error::Json {
-                source,
-                path: project_file_location.to_owned(),
-            })?;
-
-        project.file_location = project_file_location.to_path_buf();
-        project.check_compatibility();
-
-        Ok(project)
+    /// Loads a Project from a path.
+    pub fn load_exact(
+        vfs: &Vfs,
+        project_file_location: &Path,
+        fallback_name: Option<&str>,
+    ) -> Result<Self, ProjectError> {
+        let project_path = project_file_location.to_path_buf();
+        let contents = vfs.read(&project_path).map_err(Error::from)?;
+        Ok(Self::load_from_slice(
+            &contents,
+            project_path,
+            fallback_name,
+        )?)
     }
 
     /// Checks if there are any compatibility issues with this project file and

src/serve_session.rs

@@ -9,7 +9,6 @@ use std::{
 };
 
 use crossbeam_channel::Sender;
-use memofs::IoResultExt;
 use memofs::Vfs;
 use thiserror::Error;
 
@@ -110,9 +109,9 @@ impl ServeSession {
 
         log::debug!("Loading project file from {}", project_path.display());
 
-        let root_project = match vfs.read(&project_path).with_not_found()? {
-            Some(contents) => Project::load_from_slice(&contents, &project_path)?,
-            None => {
+        let root_project = match Project::load_exact(&vfs, &project_path, None) {
+            Ok(project) => project,
+            Err(_) => {
                 return Err(ServeSessionError::NoProjectFound {
                     path: project_path.to_path_buf(),
                 });
@@ -190,7 +189,10 @@ impl ServeSession {
     }
 
     pub fn project_name(&self) -> &str {
-        &self.root_project.name
+        self.root_project
+            .name
+            .as_ref()
+            .expect("all top-level projects must have their name set")
     }
 
     pub fn project_port(&self) -> Option<u16> {

src/snapshot/tree.rs

@@ -87,10 +87,28 @@ impl RojoTree {
     }
 
     pub fn insert_instance(&mut self, parent_ref: Ref, snapshot: InstanceSnapshot) -> Ref {
+        // !!!!!!!!!! UGLY HACK !!!!!!!!!!
+        //
+        // This is a set of special cases working around a more general problem upstream
+        // in rbx-dom that causes pivots to not build to file correctly, described in
+        // github.com/rojo-rbx/rojo/issues/628.
+        //
+        // We need to insert the NeedsPivotMigration property with a value of false on
+        // every instance that inherits from Model for pivots to build correctly.
+        let hack_needs_pivot_migration = match snapshot.class_name.as_ref() {
+            "Model" | "Actor" | "Tool" | "HopperBin" | "Flag" | "WorldModel" | "Workspace"
+                if !snapshot.properties.contains_key("NeedsPivotMigration") =>
+            {
+                vec![("NeedsPivotMigration", Variant::Bool(false))]
+            }
+            _ => Vec::new(),
+        };
+
         let builder = InstanceBuilder::empty()
             .with_class(snapshot.class_name.into_owned())
             .with_name(snapshot.name.into_owned())
-            .with_properties(snapshot.properties);
+            .with_properties(snapshot.properties)
+            .with_properties(hack_needs_pivot_migration);
 
         let referent = self.inner.insert(parent_ref, builder);
         self.insert_metadata(referent, snapshot.metadata);

src/snapshot_middleware/lua.rs

@@ -1,6 +1,5 @@
-use std::{collections::HashMap, path::Path, str};
+use std::{collections::HashMap, path::Path};
 
-use anyhow::Context;
 use memofs::{IoResultExt, Vfs};
 use rbx_dom_weak::types::Enum;
 
@@ -58,10 +57,8 @@ pub fn snapshot_lua(
         (_, ScriptType::Module) => ("ModuleScript", None),
     };
 
-    let contents = vfs.read(path)?;
-    let contents_str = str::from_utf8(&contents)
-        .with_context(|| format!("File was not valid UTF-8: {}", path.display()))?
-        .to_owned();
+    let contents = vfs.read_to_string_lf_normalized(path)?;
+    let contents_str = contents.as_str();
 
     let mut properties = HashMap::with_capacity(2);
     properties.insert("Source".to_owned(), contents_str.into());

src/snapshot_middleware/project.rs

@@ -1,4 +1,4 @@
-use std::{borrow::Cow, collections::HashMap, path::Path};
+use std::{borrow::Cow, collections::HashMap, ffi::OsStr, path::Path};
 
 use anyhow::{bail, Context};
 use memofs::Vfs;
@@ -19,9 +19,39 @@ pub fn snapshot_project(
     vfs: &Vfs,
     path: &Path,
 ) -> anyhow::Result<Option<InstanceSnapshot>> {
-    let project = Project::load_from_slice(&vfs.read(path)?, path)
+    let fallback_name = match path.file_name().and_then(OsStr::to_str) {
+        Some("default.project.json") => path
+            .parent()
+            .and_then(Path::file_name)
+            .and_then(OsStr::to_str),
+        Some(name) => name.strip_suffix(".project.json"),
+        None => anyhow::bail!(
+            "project file does not have valid utf-8 name: {}",
+            path.display()
+        ),
+    };
+    let project = Project::load_exact(vfs, path, fallback_name)
         .with_context(|| format!("File was not a valid Rojo project: {}", path.display()))?;
 
+    // This is not how I would normally do this, but this is a temporary
+    // implementation. The one in 7.5+ is better.
+    let project_name = project.name.as_deref().unwrap_or_else(|| {
+        let file_name = path
+            .file_name()
+            .and_then(|s| s.to_str())
+            .expect("project file names should be valid UTF-8");
+        if file_name == "default.project.json" {
+            path.parent()
+                .and_then(Path::file_name)
+                .and_then(|s| s.to_str())
+                .expect("default.project.json should be inside a folder with a valid UTF-8 name")
+        } else {
+            file_name
+                .strip_suffix(".project.json")
+                .expect("project file names should end with .project.json")
+        }
+    });
+
     let mut context = context.clone();
 
     let rules = project.glob_ignore_paths.iter().map(|glob| PathIgnoreRule {
@@ -37,7 +67,7 @@ pub fn snapshot_project(
             .unwrap(),
     );
 
-    match snapshot_project_node(&context, path, &project.name, &project.tree, vfs, None)? {
+    match snapshot_project_node(&context, path, project_name, &project.tree, vfs, None)? {
         Some(found_snapshot) => {
             let mut snapshot = found_snapshot;
             // Setting the instigating source to the project file path is a little
@@ -669,4 +699,36 @@ mod test {
 
         insta::assert_yaml_snapshot!(instance_snapshot);
     }
+
+    #[test]
+    fn no_name_project() {
+        let _ = env_logger::try_init();
+
+        let mut imfs = InMemoryFs::new();
+        imfs.load_snapshot(
+            "/no_name_project",
+            VfsSnapshot::dir(hashmap! {
+                "default.project.json" => VfsSnapshot::file(r#"
+                    {
+                        "tree": {
+                            "$className": "Model"
+                        }
+                    }
+                "#),
+            }),
+        )
+        .unwrap();
+
+        let vfs = Vfs::new(imfs);
+
+        let instance_snapshot = snapshot_project(
+            &InstanceContext::default(),
+            &vfs,
+            Path::new("/no_name_project/default.project.json"),
+        )
+        .expect("snapshot error")
+        .expect("snapshot returned no instances");
+
+        insta::assert_yaml_snapshot!(instance_snapshot);
+    }
 }

src/snapshot_middleware/snapshots/librojo__snapshot_middleware__project__test__no_name_project.snap

@@ -0,0 +1,18 @@
+---
+source: src/snapshot_middleware/project.rs
+expression: instance_snapshot
+---
+snapshot_id: "00000000000000000000000000000000"
+metadata:
+  ignore_unknown_instances: true
+  instigating_source:
+    Path: /no_name_project/default.project.json
+  relevant_paths:
+    - /no_name_project/default.project.json
+  context:
+    emit_legacy_scripts: true
+name: no_name_project
+class_name: Model
+properties: {}
+children: []
+

src/snapshot_middleware/txt.rs

@@ -1,6 +1,5 @@
-use std::{path::Path, str};
+use std::path::Path;
 
-use anyhow::Context;
 use maplit::hashmap;
 use memofs::{IoResultExt, Vfs};
 
@@ -14,11 +13,8 @@ pub fn snapshot_txt(
     path: &Path,
 ) -> anyhow::Result<Option<InstanceSnapshot>> {
     let name = path.file_name_trim_end(".txt")?;
-
-    let contents = vfs.read(path)?;
-    let contents_str = str::from_utf8(&contents)
-        .with_context(|| format!("File was not valid UTF-8: {}", path.display()))?
-        .to_owned();
+    let contents = vfs.read_to_string(path)?;
+    let contents_str = contents.as_str();
 
     let properties = hashmap! {
         "Value".to_owned() => contents_str.into(),

tests/rojo_test/serve_util.rs

@@ -87,7 +87,7 @@ impl TestServeSession {
         let port_string = port.to_string();
 
         let rojo_process = Command::new(ROJO_PATH)
-            .args(&[
+            .args([
                 "serve",
                 project_path.to_str().unwrap(),
                 "--port",
@@ -145,14 +145,14 @@ impl TestServeSession {
 
     pub fn get_api_rojo(&self) -> Result<ServerInfoResponse, reqwest::Error> {
         let url = format!("http://localhost:{}/api/rojo", self.port);
-        let body = reqwest::blocking::get(&url)?.text()?;
+        let body = reqwest::blocking::get(url)?.text()?;
 
         Ok(serde_json::from_str(&body).expect("Server returned malformed response"))
     }
 
     pub fn get_api_read(&self, id: Ref) -> Result<ReadResponse, reqwest::Error> {
         let url = format!("http://localhost:{}/api/read/{}", self.port, id);
-        let body = reqwest::blocking::get(&url)?.text()?;
+        let body = reqwest::blocking::get(url)?.text()?;
 
         Ok(serde_json::from_str(&body).expect("Server returned malformed response"))
     }
@@ -163,7 +163,7 @@ impl TestServeSession {
     ) -> Result<SubscribeResponse<'static>, reqwest::Error> {
         let url = format!("http://localhost:{}/api/subscribe/{}", self.port, cursor);
 
-        reqwest::blocking::get(&url)?.json()
+        reqwest::blocking::get(url)?.json()
     }
 }
 

tests/tests/build.rs

@@ -59,6 +59,9 @@ gen_build_tests! {
     txt_in_folder,
     unresolved_values,
     weldconstraint,
+    no_name_default_project,
+    no_name_project,
+    no_name_top_level_project,
 }
 
 fn run_build_test(test_name: &str) {
@@ -70,7 +73,7 @@ fn run_build_test(test_name: &str) {
     let output_path = output_dir.path().join(format!("{}.rbxmx", test_name));
 
     let output = Command::new(ROJO_PATH)
-        .args(&[
+        .args([
             "build",
             input_path.to_str().unwrap(),
             "-o",

tests/tests/serve.rs

@@ -255,3 +255,68 @@ fn add_optional_folder() {
         );
     });
 }
+
+#[test]
+fn no_name_default_project() {
+    run_serve_test("no_name_default_project", |session, mut redactions| {
+        let info = session.get_api_rojo().unwrap();
+        let root_id = info.root_instance_id;
+
+        assert_yaml_snapshot!(
+            "no_name_default_project_info",
+            redactions.redacted_yaml(info)
+        );
+
+        let read_response = session.get_api_read(root_id).unwrap();
+        assert_yaml_snapshot!(
+            "no_name_default_project_all",
+            read_response.intern_and_redact(&mut redactions, root_id)
+        );
+    });
+}
+
+#[test]
+fn no_name_project() {
+    run_serve_test("no_name_project", |session, mut redactions| {
+        let info = session.get_api_rojo().unwrap();
+        let root_id = info.root_instance_id;
+
+        assert_yaml_snapshot!("no_name_project_info", redactions.redacted_yaml(info));
+
+        let read_response = session.get_api_read(root_id).unwrap();
+        assert_yaml_snapshot!(
+            "no_name_project_all",
+            read_response.intern_and_redact(&mut redactions, root_id)
+        );
+    });
+}
+
+#[test]
+fn no_name_top_level_project() {
+    run_serve_test("no_name_top_level_project", |session, mut redactions| {
+        let info = session.get_api_rojo().unwrap();
+        let root_id = info.root_instance_id;
+        assert_yaml_snapshot!(
+            "no_name_top_level_project_info",
+            redactions.redacted_yaml(info)
+        );
+        let read_response = session.get_api_read(root_id).unwrap();
+        assert_yaml_snapshot!(
+            "no_name_top_level_project_all",
+            read_response.intern_and_redact(&mut redactions, root_id)
+        );
+
+        let project_path = session.path().join("default.project.json");
+        let mut project_contents = fs::read_to_string(&project_path).unwrap();
+        project_contents.push('\n');
+        fs::write(&project_path, project_contents).unwrap();
+
+        // The cursor shouldn't be changing so this snapshot is fine for testing
+        // the response.
+        let read_response = session.get_api_read(root_id).unwrap();
+        assert_yaml_snapshot!(
+            "no_name_top_level_project_all-2",
+            read_response.intern_and_redact(&mut redactions, root_id)
+        );
+    });
+}