diff --git a/docs/architecture/design.md b/docs/architecture/design.md index b1feee036e..cabe19ff46 100644 --- a/docs/architecture/design.md +++ b/docs/architecture/design.md @@ -5,9 +5,9 @@ sidebar_position: 2 # Nevermore design principles -Nevermore consists of a few hundred packages in a [mono-repo](https://en.wikipedia.org/wiki/Monorepo). These packages are [semantically versioned](https://semver.org/) such that long-term maintaince can be done. Nevermore it trying to provide utility modules, and is not a framework. +Nevermore consists of a few hundred packages in a [mono-repo](https://en.wikipedia.org/wiki/Monorepo). These packages are [semantically versioned](https://semver.org/) such that long-term maintenance can be done. Nevermore is trying to provide utility modules, and is not a framework. -* **Lego blocks** - Nevermore provides utility modules that can combined in a variety of ways +* **Lego blocks** - Nevermore provides utility modules that can be combined in a variety of ways * **Not a framework** - Nevermore works in a variety of other architectures * **Versioned** - Nevermore should be versioned. Nevermore should not break games when changes are made. * **Fast development** - Nevermore should accelerate game development @@ -35,7 +35,7 @@ Library packages tend to be packages that export one or multiple libraries. Thes * [Elo](/api/EloUtils) ### Object utility libraries -These are very similiar to libraries but they tend to export an object, and some supporting objects. These objects are concepts that are useful to learn, and generally exist outside of Roblox (although they may not). These are fundamental building blocks and patterns in Roblox. +These are very similar to libraries but they tend to export an object, and some supporting objects. These objects are concepts that are useful to learn, and generally exist outside of Roblox (although they may not). These are fundamental building blocks and patterns in Roblox. * [Octree](/api/Octree) * [Maid](/api/Maid) @@ -45,7 +45,7 @@ These are very similiar to libraries but they tend to export an object, and some * [Queue](/api/Queue) ### Integration services -There services are primary about providing a contract between two services. +These services are primarily about providing a contract between two services. * [GameConfigService](/api/GameConfigService) * [CameraStackService](/api/CameraStackService) @@ -63,7 +63,7 @@ opinionated about... 2. Consumption of code (plugin, game, et cetera) Code is designed to be copied and pasted as needed, but first and foremost, is designed to empower James's (Quenty's) workflow. For this reason, while Nevermore tries its best to be useful -to as wide of an audience as possible, in many ways document and design notes are lacking because this is not its first purpose. +to as wide of an audience as possible, in many ways documentation and design notes are lacking because this is not its first purpose. ## Loading system diff --git a/docs/architecture/servicebag.md b/docs/architecture/servicebag.md index c5f00afe03..62dacb31d0 100644 --- a/docs/architecture/servicebag.md +++ b/docs/architecture/servicebag.md @@ -9,7 +9,7 @@ Services in Nevermore use [ServiceBag](/api/ServiceBag/) and need to be required through them. ServiceBag provides services and helps with game or plugin initialization, and is like a `game` in Roblox. You can retrieve services from it, and it will ensure the service exists and is initialized. -This will bootstrap any other dependent dependencies. +This will bootstrap any other dependencies. ## tl;dr @@ -107,7 +107,7 @@ serviceBag:Start() :::warning An important detail of ServiceBag is that it does not allow your services to yield in the `:Init()` methods. This is to prevent a service from delaying your -entires game start. If you need to yield, do work in `:Start()` or export your +entire game start. If you need to yield, do work in `:Start()` or export your API calls as promises. See [Cmdr](/api/CmdrService/) for a good example of how this works. ::: @@ -205,7 +205,7 @@ end ## Extras -### Why is understanding ServiceBag is important? +### Why is understanding ServiceBag important? Nevermore tries to be a collection of libraries that can be plugged together, and not exist as a set framework that forces specific design decisions. While @@ -244,7 +244,7 @@ and dependency injection system is a really good idea. ### What ServiceBag tries to achieve ServiceBag does service dependency injection and initialization. These words -may be unfamiliar with you. Dependency injection is the process of retrieving +may be unfamiliar to you. Dependency injection is the process of retrieving dependencies instead of constructing them in an object. Lifecycle management is the process of managing the life of services, which often includes the game. @@ -374,6 +374,6 @@ local function getAnyModule(module) end ``` -It's preferably your systems interop with ServiceBag directly as ServiceBag +It's preferable that your systems interop with ServiceBag directly as ServiceBag provides more control, better testability, and more clarity on where things are coming from. \ No newline at end of file diff --git a/docs/build.md b/docs/build.md index 089f92a3ef..f69395739e 100644 --- a/docs/build.md +++ b/docs/build.md @@ -64,7 +64,7 @@ In general you want to install the following by hand. 2. [Git](https://git-scm.com/downloads) 3. [Aftman](https://github.com/LPGhatguy/aftman) -After than you will want to clone Nevermore to a folder. +After that you will want to clone Nevermore to a folder. ```bash git clone https://github.com/Quenty/NevermoreEngine.git @@ -86,4 +86,4 @@ pnpm install You can then serve a test place ## Why does building need a custom version of Rojo? -Nevermore does not need a custom version of Rojo to be consumed, but it does need one to be built. This custom version of Rojo understands symlinks and turn them into ObjectValues. These symlinks link the packages together and means that a change to a transient dependency, or direct dependency will immediately be shown in the upstream package. +Nevermore does not need a custom version of Rojo to be consumed, but it does need one to be built. This custom version of Rojo understands symlinks and turn them into ObjectValues. These symlinks link the packages together and means that a change to a transitive dependency, or direct dependency will immediately be shown in the upstream package. diff --git a/docs/ides/vscode.md b/docs/ides/vscode.md index 3f2a6b6e33..8bbcfe3623 100644 --- a/docs/ides/vscode.md +++ b/docs/ides/vscode.md @@ -5,7 +5,7 @@ sidebar_position: 1 # Getting started with VSCode -VSCode works with Nevermore relatively easily. We have default extensions.json setup. Follow the general setup tips. These types should generally work for Cursor and other VS-Code based IDEs. +VSCode works with Nevermore relatively easily. We have default extensions.json setup. Follow the general setup tips. These tips should generally work for Cursor and other VS-Code based IDEs. ## Extensions @@ -22,7 +22,7 @@ These will provide snippets, styling, and linking. You currently must use the forked version of luau-lsp. You can use the default extension. -in `settings.json` configure the luau-lsp server to point towards a custom exe path. This should be your Luau-lsp exe path installed via aftman.toml. +In `settings.json`, configure the luau-lsp server to point towards a custom exe path. This should be your Luau-lsp exe path installed via aftman.toml. ```json "luau-lsp.server.path": "/tool-storage/quenty/luau-lsp//luau-lsp.exe", diff --git a/docs/install.md b/docs/install.md index 6ae6ef6796..0326031f66 100644 --- a/docs/install.md +++ b/docs/install.md @@ -38,7 +38,7 @@ npm install -g @quenty/nevermore-cli This will install the current version of Maid and all dependencies into the `node_modules` folder. To upgrade you will want to run `npm upgrade` You should ignore the `node_modules` folder in your source control system. ### What is NPM and why are we using it? -[npm](https://www.npmjs.com/) is a package manager. Nevermore uses npm to manage package versions and install transient dependencies. A transient dependency is a dependency of a dependency (for example, [Blend](/api/Blend) depends upon [Maid](/api/Maid). +[npm](https://www.npmjs.com/) is a package manager. Nevermore uses npm to manage package versions and install transitive dependencies. A transitive dependency is a dependency of a dependency (for example, [Blend](/api/Blend) depends upon [Maid](/api/Maid)). ### How do I install additional packages? The default installation comes with very few packages. This is normal. You can see which packages are installed by looking at the `package.json` file in a text editor. To install additional packages, simply run the following command in a terminal: @@ -135,7 +135,7 @@ local require = require(loader).bootstrapGame(loader.Parent) Assuming you've changed nothing, the path to the replicated modules should be the same as the one used on the server, just indexed under ReplicatedStorage instead. ## Manually installing via NPM for a stand-alone module. -If you want to use Nevermore for more stand-alone or reusable scenarios (where you can't assume that a packages folder will be reused, you can manually bootstrap the components using the loader system. +If you want to use Nevermore for more stand-alone or reusable scenarios (where you can't assume that a packages folder will be reused), you can manually bootstrap the components using the loader system. Ensure that you have [Node.js](https://nodejs.org/en/download/) v14+ installed on your computer. diff --git a/docs/intro.md b/docs/intro.md index ca058ac329..e0e7df19b9 100644 --- a/docs/intro.md +++ b/docs/intro.md @@ -31,7 +31,7 @@ Nevermore has had significant cultural impact. There are some packages this repo * [DataStore](/api/DataStore) - Battle-tested datastore wrapper * [Camera](/api/CameraStackService) - Layered camera system that interops with Roblox's cameras -## Nevermore can by used in many cases +## Nevermore can be used in many cases While Nevermore was originally designed to make games, in general Nevermore is now a collection of utility libraries that can be used in the following. These use cases have been carefully battle tested. Nevermore is in many top games, gamejams, plugins, and other components across Roblox. * **Top Games** - Both built originally with Nevermore, or games that use other systems and frameworks but may want to include Nevermore diff --git a/docs/testing/testing.md b/docs/testing/testing.md index 5786fa0b65..be1dbd7eba 100644 --- a/docs/testing/testing.md +++ b/docs/testing/testing.md @@ -257,4 +257,5 @@ For CI, set `ROBLOSECURITY` as a repository or Codespace secret. The `.github/wo - **Workflows should be thin.** All logic lives in `nevermore-cli` commands — GitHub Actions workflows just call them. This keeps CI debuggable locally. - **Rate limiting** is shared across concurrent workers via the `OpenCloudClient` instance. The `RateLimiter` serializes all Open Cloud API requests (one in-flight at a time) and reads `x-ratelimit-remaining` / `x-ratelimit-reset` headers. - **Post results via CLI**: `nevermore tools post-test-results ` posts or updates a PR comment with test results and writes to the GitHub Actions job summary. Requires `GITHUB_TOKEN` for PR comments; job summaries are written automatically when `GITHUB_STEP_SUMMARY` is set. -- **Job summaries**: Results are automatically written to the [GitHub Actions job summary](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#adding-a-job-summary) when running in CI. This makes results visible on the workflow run summary page, complementing the PR comment. +- **Live comment updates during the run**: When `nevermore batch test` detects a CI environment, it also updates the PR comment as packages transition between phases (throttled to ~10s). The `post-test-results` step still writes the final snapshot, but reviewers see progress without waiting for the full run to finish. In `--aggregated` mode every package shares one execution, so they move through `uploading` → `scheduling` → `executing` in lock-step — that's expected, not a bug. +- **Job summaries**: Results are automatically written to the [GitHub Actions job summary](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-commands#adding-a-job-summary) when running in CI. This makes results visible on the workflow run summary page, complementing the PR comment. The job summary is only written by `post-test-results` (not by the live batch run) to avoid duplicate entries on the workflow summary page. diff --git a/package.json b/package.json index bdc1a51aaa..47f8c9339f 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "build:sourcemap": "rojo sourcemap default.project.json --output sourcemap.json --absolute && npx @quenty/nevermore-cli tools strip-sourcemap-jest", "build:ts": "pnpm -r --filter './tools/**' --filter '!./tools/nevermore-vscode' run build", "format": "stylua --config-path=stylua.toml src games plugins", - "format:ts": "prettier --ignore-path .gitignore --write 'tools/**/*.{ts,tsx,js,jsx}'", + "format:ts": "prettier --ignore-path .gitignore --write \"tools/**/*.{ts,tsx,js,jsx}\"", "lint:luau": "luau-lsp analyze --sourcemap=sourcemap.json --base-luaurc=.luaurc --defs=globalTypes.d.lua --flag:LuauSolverV2=false --ignore=**/node_modules/** --ignore=**/*.story.lua --ignore=**/*.client.lua --ignore=**/*.server.lua src", "lint:moonwave": "npx lerna exec --parallel -- moonwave-extractor extract src", "lint:prettier": "prettier --ignore-path .gitignore --check 'tools/**/*.{ts,tsx,js,jsx}'", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9308c7d842..2686bb3ca4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -743,9 +743,15 @@ importers: src/brine: dependencies: + '@quenty/baseobject': + specifier: workspace:* + version: link:../baseobject '@quenty/bufferencoder': specifier: workspace:* version: link:../buffer-encoder + '@quenty/instanceutils': + specifier: workspace:* + version: link:../instanceutils '@quenty/loader': specifier: workspace:* version: link:../loader @@ -758,18 +764,27 @@ importers: '@quenty/nevermore-test-runner': specifier: workspace:* version: link:../nevermore-test-runner + '@quenty/rx': + specifier: workspace:* + version: link:../rx '@quenty/selectionutils': specifier: workspace:* version: link:../selectionutils '@quenty/servicebag': specifier: workspace:* version: link:../servicebag + '@quenty/steputils': + specifier: workspace:* + version: link:../steputils '@quenty/string': specifier: workspace:* version: link:../string '@quenty/symbol': specifier: workspace:* version: link:../symbol + '@quenty/table': + specifier: workspace:* + version: link:../table '@quenty/viewport': specifier: workspace:* version: link:../viewport diff --git a/readme.md b/readme.md index 95950208ad..c79d8a8cfa 100644 --- a/readme.md +++ b/readme.md @@ -1,7 +1,7 @@

Nevermore

- + Documentation status diff --git a/src/blend/README.md b/src/blend/README.md index 63901c3ee0..821bb54b6e 100644 --- a/src/blend/README.md +++ b/src/blend/README.md @@ -22,7 +22,7 @@ npm install @quenty/blend --save ## Attributes -This system is designed to be very similar to fusion, except that we do not having any global state management, do not rely upon weak references, works with my types, and is built on top of Rx types. +This system is designed to be very similar to fusion, except that we do not have any global state management, do not rely upon weak references, works with my types, and is built on top of Rx types. * No global state * Extensible diff --git a/src/brine/README.md b/src/brine/README.md index 4924e94487..eca0469fb3 100644 --- a/src/brine/README.md +++ b/src/brine/README.md @@ -12,7 +12,7 @@

-Fast and efficient extensible serialiation and deserialization library for Roblox with native instance support out of the box +Fast and efficient extensible serialization and deserialization library for Roblox with native instance support out of the box
View docs →
diff --git a/src/brine/deploy.nevermore.json b/src/brine/deploy.nevermore.json new file mode 100644 index 0000000000..7b14e6e5b3 --- /dev/null +++ b/src/brine/deploy.nevermore.json @@ -0,0 +1,10 @@ +{ + "targets": { + "test": { + "universeId": 9716264427, + "placeId": 115317112879009, + "project": "test/default.project.json", + "scriptTemplate": "test/scripts/Server/ServerMain.server.lua" + } + } +} diff --git a/src/brine/package.json b/src/brine/package.json index 788d477e5d..fd5262f5ec 100644 --- a/src/brine/package.json +++ b/src/brine/package.json @@ -28,15 +28,20 @@ "Quenty" ], "dependencies": { + "@quenty/baseobject": "workspace:*", "@quenty/bufferencoder": "workspace:*", + "@quenty/instanceutils": "workspace:*", "@quenty/loader": "workspace:*", "@quenty/maid": "workspace:*", "@quenty/memoize": "workspace:*", "@quenty/nevermore-test-runner": "workspace:*", + "@quenty/rx": "workspace:*", "@quenty/selectionutils": "workspace:*", "@quenty/servicebag": "workspace:*", + "@quenty/steputils": "workspace:*", "@quenty/string": "workspace:*", "@quenty/symbol": "workspace:*", + "@quenty/table": "workspace:*", "@quenty/viewport": "workspace:*", "@quentystudios/jest-lua": "3.10.0-quenty.2" }, diff --git a/src/brine/src/Shared/Brine.lua b/src/brine/src/Shared/Brine.lua index 893f6395a8..afd0fbc353 100644 --- a/src/brine/src/Shared/Brine.lua +++ b/src/brine/src/Shared/Brine.lua @@ -12,9 +12,13 @@ local EncodingService = game:GetService("EncodingService") local BrineContext = require("BrineContext") local BrineInstanceEncoder = require("BrineInstanceEncoder") +local BrineInstanceReflection = require("BrineInstanceReflection") local BrineOptionUtils = require("BrineOptionUtils") local BrineTypes = require("BrineTypes") local BufferEncoder = require("BufferEncoder") +local Maid = require("Maid") +local Observable = require("Observable") +local RxInstanceUtils = require("RxInstanceUtils") local COMPRESSION_LEVEL = 8 -- 1 is fastest, 22 is slowest @@ -22,21 +26,346 @@ local Brine = {} --[=[ Serializes an instance into a string, with optional references - - @param data Instance - @param options BrineOptions? - @return Brined, References? ]=] function Brine.serialize(data: Instance, options: BrineTypes.BrineOptions?): (BrineTypes.Brined, BrineTypes.References?) local safeOptions = BrineOptionUtils.defaultOptions(options) local context = BrineContext.new(safeOptions) - local intermediate = Brine._toIntermediate(context, data) - local ensuredIntermediate = context:ReplaceInstancesWithSerializedInstances(intermediate) + return Brine._toStream(context, intermediate) +end + +--[=[ + Deserializes a string into an instance, with optional references +]=] +function Brine.deserialize(data: BrineTypes.Brined, options: BrineTypes.BrineOptions?): Instance? + local safeOptions = BrineOptionUtils.defaultOptions(options) + local context = BrineContext.new(safeOptions) + local intermediate = Brine._fromStream(context, data, safeOptions.references) + return Brine._fromIntermediate(context, intermediate) +end + +--[=[ + Observes changes to an instance +]=] +function Brine.observeSerialize( + data: Instance, + options: BrineTypes.BrineOptions? +): Observable.Observable + local safeOptions = BrineOptionUtils.defaultOptions(options) + + return Observable.new(function(sub) + local topMaid = Maid.new() + local context = BrineContext.new(safeOptions) + local replicated: { [Instance]: boolean } = {} + + local function encodeReferences(references: BrineTypes.References?): BrineTypes.References? + if not references then + return nil + end + + local encodedReferences = {} + for index, reference in references do + if typeof(reference) == "Instance" then + encodedReferences[index] = context.InstanceRegistry:FindInstanceId(reference) or reference + end + end + return encodedReferences + end + + local function firePacket(packet: BrineTypes.BrinePacket) + if not sub:IsPending() then + return + end + + local stream, references = Brine._toStream(context, packet) + sub:Fire(stream, encodeReferences(references)) + end + + local function fireFullFrame() + local serialized = Brine._toIntermediate(context, data) + local packet: BrineTypes.FullFramePacket = { + type = "full", + data = serialized, + } + + replicated[data] = true + for _, descendant in data:GetDescendants() do + replicated[descendant] = true + end + firePacket(packet) + end + + local function firePropertyChange(instance: Instance, property: string) + local value = (instance :: any)[property] + local packet: BrineTypes.ChangePacket + if value == nil then + packet = { + type = "change", + instanceId = context.InstanceRegistry:InstanceToId(instance), + clearedProperties = { property }, + } + else + packet = { + type = "change", + instanceId = context.InstanceRegistry:InstanceToId(instance), + properties = { + [property] = value, + }, + } + end + firePacket(packet) + end + + local function fireAttributeChange(instance: Instance, attribute: string) + local value = instance:GetAttribute(attribute) + local packet: BrineTypes.ChangePacket + if value == nil then + packet = { + type = "change", + instanceId = context.InstanceRegistry:InstanceToId(instance), + clearedAttributes = { attribute }, + } + else + packet = { + type = "change", + instanceId = context.InstanceRegistry:InstanceToId(instance), + attributes = { + [attribute] = value, + }, + } + end + firePacket(packet) + end + + local function fireDescendantAdded(instance: Instance) + if replicated[instance] then + return + end + local parent = instance.Parent + if not parent then + return + end + + replicated[instance] = true + for _, descendant in instance:GetDescendants() do + replicated[descendant] = true + end + local serialized = Brine._toIntermediate(context, instance) + local packet: BrineTypes.DescendantTreeAddedPacket = { + type = "descendantAdded", + parentInstanceId = context.InstanceRegistry:InstanceToId(parent), + instanceId = context.InstanceRegistry:InstanceToId(instance), + data = serialized, + } + firePacket(packet) + end + + local function fireDescendantRemoved(instance: Instance) + if not replicated[instance] then + return + end + + replicated[instance] = nil + for _, descendant in instance:GetDescendants() do + replicated[descendant] = nil + end + + local packet: BrineTypes.DescendantTreeRemovingPacket = { + type = "descendantRemoving", + instanceId = context.InstanceRegistry:InstanceToId(instance), + } + firePacket(packet) + end + + local function registerInstanceChanged(maid: Maid.Maid, instance: Instance) + local encodedProperties = + BrineInstanceReflection.getEncodedProperties(instance.ClassName, context.SecurityCapabilities) + if not encodedProperties then + return + end + + maid:GiveTask(instance.AttributeChanged:Connect(function(attribute) + fireAttributeChange(instance, attribute) + end)) + + for _, property in encodedProperties.orderedList do + maid:GiveTask(instance:GetPropertyChangedSignal(property.Name):Connect(function() + firePropertyChange(instance, property.Name) + end)) + end + end + + -- Fire full frame first + fireFullFrame() + registerInstanceChanged(topMaid, data) + + -- Then fire descendants + if safeOptions.includeDescendants then + topMaid:GiveTask(RxInstanceUtils.observeDescendantsBrio(data):Subscribe(function(brio) + if brio:IsDead() then + return + end + + local maid, descendant = brio:ToMaidAndValue() + fireDescendantAdded(descendant) + registerInstanceChanged(maid, descendant) + + maid:GiveTask(descendant:GetPropertyChangedSignal("Parent"):Connect(function() + if descendant:IsDescendantOf(data) then + firePropertyChange(descendant, "Parent") + end + end)) + + maid:GiveTask(function() + if sub:IsPending() then + fireDescendantRemoved(descendant) + end + end) + end)) + end + + return topMaid + end) :: any +end - local stream, references = BufferEncoder.write(ensuredIntermediate, nil, { +--[=[ + Observes changes to an instance, returning deserialized instances +]=] +function Brine.observeDeserialize( + observableStream: Observable.Observable, + options: BrineTypes.BrineOptions? +): Observable.Observable + local safeOptions = BrineOptionUtils.defaultOptions(options) + + return Observable.new(function(sub) + local context = BrineContext.new(safeOptions) + + local topMaid = Maid.new() + local current: Instance? = nil + + local function decodeReferences(references: BrineTypes.References?): BrineTypes.References? + if not references then + return nil + end + + local decodedReferences: BrineTypes.References = {} + for index, reference in references do + if typeof(reference) == "Instance" then + decodedReferences[index] = reference + continue + end + decodedReferences[index] = context.InstanceRegistry:IdToInstance(reference) + end + return decodedReferences + end + + local function handleFullFrame(packet: BrineTypes.FullFramePacket) + local deserialized = Brine._fromIntermediate(context, packet.data) + + if not deserialized then + warn("Failed to deserialize instance") + else + current = deserialized + sub:Fire(current) + end + end + + local function handleChangePacket(packet: BrineTypes.ChangePacket) + if not current then + warn("Received change packet before full frame") + return + end + + local instance = context.InstanceRegistry:IdToInstance(packet.instanceId) + if not instance then + warn("Received change packet for unknown instance ID: " .. packet.instanceId) + return + end + + BrineInstanceEncoder.decodeProperties(context, instance, packet.properties) + BrineInstanceEncoder.decodeAttributes(context, instance, packet.attributes) + BrineInstanceEncoder.decodeChildren(context, instance, packet.children) + BrineInstanceEncoder.decodeTags(context, instance, packet.tags) + BrineInstanceEncoder.clearProperties(context, instance, packet.clearedProperties) + BrineInstanceEncoder.clearAttributes(context, instance, packet.clearedAttributes) + end + + local function handleDescendantAdded(packet: BrineTypes.DescendantTreeAddedPacket) + if not current then + warn("Received descendant added packet before full frame") + return + end + + local parent = context.InstanceRegistry:IdToInstance(packet.parentInstanceId) + if not parent then + warn("Received descendant added packet for unknown parent instance ID: " .. packet.parentInstanceId) + return + end + + if not parent:IsDescendantOf(current) and parent ~= current then + warn("Can only add descendants to current instance") + return + end + + local deserialized = Brine._fromIntermediate(context, packet.data) + if not deserialized then + warn("Failed to deserialize descendant instance") + return + end + + deserialized.Parent = parent + end + + local function handleDescendantRemoving(packet: BrineTypes.DescendantTreeRemovingPacket) + if not current then + warn("Received descendant removing packet before full frame") + return + end + + local instance = context.InstanceRegistry:IdToInstance(packet.instanceId) + if not instance then + warn("Received descendant removing packet for unknown instance ID: " .. packet.instanceId) + return + end + + if not instance:IsDescendantOf(current) then + warn("Can only remove descendants in current instance") + return + end + + context.InstanceRegistry:SetInstanceId(instance, nil) + instance:Destroy() + end + + topMaid:GiveTask(observableStream:Subscribe(function(encoded, encodedReferences) + local references = decodeReferences(encodedReferences) + local packet: BrineTypes.BrinePacket = Brine._fromStream(context, encoded, references) + + if packet.type == "full" then + handleFullFrame(packet) + elseif packet.type == "change" then + handleChangePacket(packet) + elseif packet.type == "descendantAdded" then + handleDescendantAdded(packet) + elseif packet.type == "descendantRemoving" then + handleDescendantRemoving(packet) + else + error("Unknown packet type: " .. tostring(packet.type)) + end + end)) + + return topMaid + end) :: any +end + +function Brine._toStream( + _context: BrineContext.BrineContext, + intermediate: BrineTypes.Intermediate +): (string, BrineTypes.References?) + local stream, references = BufferEncoder.write(intermediate, nil, { allowdeduplication = true, allowreferences = true, + rbxenum_behavior = "compact", }) stream = EncodingService:CompressBuffer(stream, Enum.CompressionAlgorithm.Zstd, COMPRESSION_LEVEL) @@ -44,41 +373,34 @@ function Brine.serialize(data: Instance, options: BrineTypes.BrineOptions?): (Br return buffer.tostring(stream), references end ---[=[ - Deserializes a string into an instance, with optional references - - @param data Brined - @param options BrineOptions? - @return Instance? -]=] -function Brine.deserialize(data: BrineTypes.Brined, options: BrineTypes.BrineOptions?): Instance? - local safeOptions = BrineOptionUtils.defaultOptions(options) - local context = BrineContext.new(safeOptions) +function Brine._fromStream( + _context: BrineContext.BrineContext, + data: BrineTypes.Brined, + references: BrineTypes.References? +): BrineTypes.Intermediate local stream = buffer.fromstring(data) stream = EncodingService:DecompressBuffer(stream, Enum.CompressionAlgorithm.Zstd) - local intermediate = BufferEncoder.read(stream, nil, { allowdeduplication = true, allowreferences = true, - references = safeOptions.references, + references = references, + rbxenum_behavior = "compact", }) - - intermediate = context:ReplaceSerializedInstancesWithInstances(intermediate) - - return Brine._fromIntermediate(context, intermediate) + return intermediate end -function Brine._toIntermediate(context: BrineContext.BrineContext, data: Instance): BrineTypes.Intermediate? +function Brine._toIntermediate(context: BrineContext.BrineContext, data: Instance): BrineTypes.Intermediate local encoded = BrineInstanceEncoder.encodeInstance(context, data) - if encoded == nil then - return nil - end - - return encoded + local ensuredIntermediate = context:ReplaceInstancesWithSerializedInstances(encoded) + context:ClearState() + return ensuredIntermediate end -function Brine._fromIntermediate(context: BrineContext.BrineContext, data: BrineTypes.Intermediate): Instance? - return BrineInstanceEncoder.decodeInstance(context, data) +function Brine._fromIntermediate(context: BrineContext.BrineContext, intermediate: BrineTypes.Intermediate): Instance? + intermediate = context:ReplaceSerializedInstancesWithInstances(intermediate) + local result = BrineInstanceEncoder.decodeInstance(context, intermediate) + context:ClearState() + return result end return Brine diff --git a/src/brine/src/Shared/Brine.spec.lua b/src/brine/src/Shared/Brine.spec.lua new file mode 100644 index 0000000000..86e6c3308c --- /dev/null +++ b/src/brine/src/Shared/Brine.spec.lua @@ -0,0 +1,953 @@ +--!nonstrict +--[[ + @class Brine.spec.lua +]] + +local require = (require :: any)( + game:GetService("ServerScriptService"):FindFirstChild("LoaderUtils", true).Parent + ).bootstrapStory(script) :: typeof(require(script.Parent.loader).load(script)) + +local Brine = require("Brine") +local BrineContext = require("BrineContext") +local BrineOptionUtils = require("BrineOptionUtils") +local Jest = require("Jest") +local Observable = require("Observable") +local StepUtils = require("StepUtils") + +local describe = Jest.Globals.describe +local expect = Jest.Globals.expect +local it = Jest.Globals.it + +describe("Brine.serialize", function() + it("returns a non-empty string", function() + local part = Instance.new("Part") + local serialized = Brine.serialize(part) + + expect(typeof(serialized)).toEqual("string") + expect(#serialized > 0).toEqual(true) + end) + + it("returns deterministic output for equivalent inputs", function() + local first = Instance.new("Folder") + first.Name = "Same" + + local second = Instance.new("Folder") + second.Name = "Same" + + expect((Brine.serialize(first))).toEqual((Brine.serialize(second))) + end) + + it("returns different output for inputs that differ", function() + local a = Instance.new("Folder") + a.Name = "First" + + local b = Instance.new("Folder") + b.Name = "Second" + + expect((Brine.serialize(a))).never.toEqual((Brine.serialize(b))) + end) +end) + +describe("Brine.deserialize", function() + it("returns an instance for a valid serialized payload", function() + local part = Instance.new("Part") + local serialized = Brine.serialize(part) + local result = Brine.deserialize(serialized) + + expect(typeof(result)).toEqual("Instance") + end) + + it("preserves the class of the original instance", function() + local original = Instance.new("Folder") + local result = Brine.deserialize(Brine.serialize(original)) + + expect(result.ClassName).toEqual("Folder") + end) +end) + +describe("Brine.serialize / Brine.deserialize roundtrip", function() + describe("for a Part with non-default properties", function() + local part = Instance.new("Part") + part.Name = "MyPart" + part.Size = Vector3.new(2, 4, 6) + part.Color = Color3.new(1, 0, 0) + part.Transparency = 0.5 + part.Anchored = true + part.CanCollide = false + + local result = Brine.deserialize(Brine.serialize(part)) + + it("preserves the class name", function() + expect(result.ClassName).toEqual("Part") + end) + + it("preserves the Name", function() + expect(result.Name).toEqual("MyPart") + end) + + it("preserves the Size", function() + expect(result.Size).toEqual(Vector3.new(2, 4, 6)) + end) + + it("preserves the Color", function() + expect(result.Color).toEqual(Color3.new(1, 0, 0)) + end) + + it("preserves the Transparency", function() + expect(result.Transparency).toEqual(0.5) + end) + + it("preserves the Anchored flag", function() + expect(result.Anchored).toEqual(true) + end) + + it("preserves the CanCollide flag", function() + expect(result.CanCollide).toEqual(false) + end) + end) + + describe("for ValueBase instances", function() + it("preserves the StringValue Value", function() + local original = Instance.new("StringValue") + original.Value = "hello, brine" + + local result = Brine.deserialize(Brine.serialize(original)) + + expect(result.ClassName).toEqual("StringValue") + expect(result.Value).toEqual("hello, brine") + end) + + it("preserves the IntValue Value", function() + local original = Instance.new("IntValue") + original.Value = 42 + + local result = Brine.deserialize(Brine.serialize(original)) + + expect(result.ClassName).toEqual("IntValue") + expect(result.Value).toEqual(42) + end) + + it("preserves the BoolValue Value when true", function() + local original = Instance.new("BoolValue") + original.Value = true + + local result = Brine.deserialize(Brine.serialize(original)) + + expect(result.Value).toEqual(true) + end) + end) + + describe("for a Folder containing children", function() + local folder = Instance.new("Folder") + folder.Name = "Container" + + local first = Instance.new("Part") + first.Name = "First" + first.Parent = folder + + local second = Instance.new("StringValue") + second.Name = "Second" + second.Value = "child value" + second.Parent = folder + + local result = Brine.deserialize(Brine.serialize(folder)) + + it("preserves the parent name", function() + expect(result.Name).toEqual("Container") + end) + + it("preserves both children", function() + expect(#result:GetChildren()).toEqual(2) + end) + + it("preserves the Part child", function() + local child = result:FindFirstChild("First") + expect(child).never.toEqual(nil) + expect(child.ClassName).toEqual("Part") + end) + + it("preserves the StringValue child and its Value", function() + local child = result:FindFirstChild("Second") + expect(child).never.toEqual(nil) + expect(child.ClassName).toEqual("StringValue") + expect(child.Value).toEqual("child value") + end) + end) + + describe("for nested children", function() + it("preserves the full hierarchy", function() + local outer = Instance.new("Folder") + outer.Name = "Outer" + + local middle = Instance.new("Folder") + middle.Name = "Middle" + middle.Parent = outer + + local inner = Instance.new("StringValue") + inner.Name = "Inner" + inner.Value = "deep" + inner.Parent = middle + + local result = Brine.deserialize(Brine.serialize(outer)) + local resolvedMiddle = result:FindFirstChild("Middle") + local resolvedInner = if resolvedMiddle then resolvedMiddle:FindFirstChild("Inner") else nil + + expect(resolvedMiddle).never.toEqual(nil) + expect(resolvedInner).never.toEqual(nil) + expect(resolvedInner.Value).toEqual("deep") + end) + end) + + describe("for an instance with attributes", function() + it("preserves attribute values across types", function() + local part = Instance.new("Part") + part:SetAttribute("Count", 7) + part:SetAttribute("Label", "tagged") + part:SetAttribute("Enabled", true) + + local result = Brine.deserialize(Brine.serialize(part)) + + expect(result:GetAttribute("Count")).toEqual(7) + expect(result:GetAttribute("Label")).toEqual("tagged") + expect(result:GetAttribute("Enabled")).toEqual(true) + end) + end) + + describe("for an instance with tags", function() + it("preserves all tags", function() + local part = Instance.new("Part") + part:AddTag("First") + part:AddTag("Second") + + local result = Brine.deserialize(Brine.serialize(part, { + includeTags = true, + includeAttributes = false, + includeDescendants = false, + })) + local tags = result:GetTags() + + expect(#tags).toEqual(2) + expect(table.find(tags, "First")).never.toEqual(nil) + expect(table.find(tags, "Second")).never.toEqual(nil) + end) + end) + + describe("with includeDescendants = false", function() + it("does not serialize children", function() + local folder = Instance.new("Folder") + folder.Name = "Empty" + + local child = Instance.new("Part") + child.Parent = folder + + local serialized = Brine.serialize(folder, { includeDescendants = false }) + local result = Brine.deserialize(serialized, { includeDescendants = false }) + + expect(result.Name).toEqual("Empty") + expect(#result:GetChildren()).toEqual(0) + end) + end) +end) + +describe("Brine.observeSerialize", function() + it("fires a full-frame packet immediately on subscribe", function() + local original = Instance.new("StringValue") + original.Value = "initial" + + local emissions = {} + local sub = Brine.observeSerialize(original):Subscribe(function(packet) + table.insert(emissions, packet) + end) + + expect(#emissions).toEqual(1) + expect(typeof(emissions[1])).toEqual("string") + expect(#emissions[1] > 0).toEqual(true) + sub:Destroy() + end) + + it("fires another packet when an encoded property changes", function() + local original = Instance.new("StringValue") + original.Value = "before" + + local emissions = {} + local sub = Brine.observeSerialize(original):Subscribe(function(packet) + table.insert(emissions, packet) + end) + + local before = #emissions + original.Value = "after" + + expect(#emissions > before).toEqual(true) + sub:Destroy() + end) + + it("stops firing after the subscription is destroyed", function() + local original = Instance.new("StringValue") + original.Value = "pre-destroy" + + local emissions = {} + local sub = Brine.observeSerialize(original):Subscribe(function(packet) + table.insert(emissions, packet) + end) + sub:Destroy() + + local before = #emissions + original.Value = "post-destroy" + + expect(#emissions).toEqual(before) + end) +end) + +describe("Brine.observeDeserialize", function() + it("does not emit anything for a source that never fires", function() + local source = Observable.new(function(_sub) + return function() end + end) + + local emitted = false + local sub = Brine.observeDeserialize(source):Subscribe(function() + emitted = true + end) + + expect(emitted).toEqual(false) + sub:Destroy() + end) + + it("emits an instance with the upstream class and properties on the full frame", function() + local original = Instance.new("StringValue") + original.Name = "Source" + original.Value = "hello observable" + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + expect(result).never.toEqual(nil) + expect(result.ClassName).toEqual("StringValue") + expect(result.Name).toEqual("Source") + expect(result.Value).toEqual("hello observable") + sub:Destroy() + end) + + it("applies upstream property changes to the deserialized instance", function() + local original = Instance.new("StringValue") + original.Value = "first" + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + original.Value = "second" + + expect(result).never.toEqual(nil) + expect(result.Value).toEqual("second") + sub:Destroy() + end) + + it("emits the same instance reference on each upstream packet", function() + local original = Instance.new("StringValue") + original.Value = "alpha" + + local seen = {} + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + table.insert(seen, instance) + end) + + original.Value = "beta" + original.Value = "gamma" + + expect(#seen >= 1).toEqual(true) + for index = 2, #seen do + expect(seen[index]).toEqual(seen[1]) + end + sub:Destroy() + end) +end) + +-- Edge-case probes for the observe pipeline. These are documentation tests: +-- some are expected to fail today and the failures are the signal — they +-- highlight scenarios the change-packet pipeline does not propagate yet. +describe("Brine.observeSerialize / Brine.observeDeserialize edge cases", function() + it("propagates a descendant added after subscribe", function() + local original = Instance.new("Folder") + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + local child = Instance.new("StringValue") + child.Name = "AddedAfterSubscribe" + child.Value = "fresh" + child.Parent = original + + expect(result).never.toEqual(nil) + local resolvedChild = result:FindFirstChild("AddedAfterSubscribe") + expect(resolvedChild).never.toEqual(nil) + expect(resolvedChild.Value).toEqual("fresh") + sub:Destroy() + end) + + it("propagates a descendant removed after subscribe", function() + local original = Instance.new("Folder") + local child = Instance.new("StringValue") + child.Name = "Removable" + child.Parent = original + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + expect(result:FindFirstChild("Removable")).never.toEqual(nil) + + child.Parent = nil + StepUtils.deferWait() + + expect(result:FindFirstChild("Removable")).toEqual(nil) + + sub:Destroy() + end) + + it("propagates reparenting between two folders inside the source tree", function() + local root = Instance.new("Folder") + + local first = Instance.new("Folder") + first.Name = "First" + first.Parent = root + + local second = Instance.new("Folder") + second.Name = "Second" + second.Parent = root + + local item = Instance.new("StringValue") + item.Name = "Item" + item.Parent = first + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(root)):Subscribe(function(instance) + result = instance + end) + + item.Parent = second + StepUtils.deferWait() + + local resolvedFirst = result:FindFirstChild("First") + local resolvedSecond = result:FindFirstChild("Second") + expect(resolvedFirst).never.toEqual(nil) + expect(resolvedSecond).never.toEqual(nil) + expect(resolvedFirst:FindFirstChild("Item")).toEqual(nil) + expect(resolvedSecond:FindFirstChild("Item")).never.toEqual(nil) + sub:Destroy() + end) + + it("propagates an ObjectValue.Value assignment that points inside the tree", function() + local root = Instance.new("Folder") + + local target = Instance.new("StringValue") + target.Name = "Target" + target.Value = "target value" + target.Parent = root + + local pointer = Instance.new("ObjectValue") + pointer.Name = "Pointer" + pointer.Parent = root + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(root)):Subscribe(function(instance) + result = instance + end) + + pointer.Value = target + + local resolvedPointer = result:FindFirstChild("Pointer") + local resolvedTarget = result:FindFirstChild("Target") + expect(resolvedPointer).never.toEqual(nil) + expect(resolvedTarget).never.toEqual(nil) + expect(resolvedPointer.Value).toEqual(resolvedTarget) + sub:Destroy() + end) + + it("propagates clearing an ObjectValue.Value back to nil", function() + local root = Instance.new("Folder") + + local target = Instance.new("StringValue") + target.Name = "Target" + target.Parent = root + + local pointer = Instance.new("ObjectValue") + pointer.Name = "Pointer" + pointer.Value = target + pointer.Parent = root + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(root)):Subscribe(function(instance) + result = instance + end) + + pointer.Value = nil + + local resolvedPointer = result:FindFirstChild("Pointer") + expect(resolvedPointer).never.toEqual(nil) + expect(resolvedPointer.Value).toEqual(nil) + sub:Destroy() + end) + + -- Tag change propagation is intentionally unsupported — there is no performant + -- way to observe per-instance tag changes, so these scenarios are skipped. + it.skip("propagates a tag added after subscribe", function() + local original = Instance.new("Part") + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + original:AddTag("LateTag") + + expect(result).never.toEqual(nil) + expect(result:HasTag("LateTag")).toEqual(true) + sub:Destroy() + end) + + it.skip("propagates a tag removed after subscribe", function() + local original = Instance.new("Part") + original:AddTag("InitialTag") + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + expect(result:HasTag("InitialTag")).toEqual(true) + + original:RemoveTag("InitialTag") + + expect(result:HasTag("InitialTag")).toEqual(false) + sub:Destroy() + end) + + it("propagates an attribute set after subscribe", function() + local original = Instance.new("Part") + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + original:SetAttribute("LateAttr", 99) + + expect(result).never.toEqual(nil) + expect(result:GetAttribute("LateAttr")).toEqual(99) + sub:Destroy() + end) + + it("propagates an attribute cleared back to nil after subscribe", function() + local original = Instance.new("Part") + original:SetAttribute("InitialAttr", 7) + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(original)):Subscribe(function(instance) + result = instance + end) + + expect(result:GetAttribute("InitialAttr")).toEqual(7) + + original:SetAttribute("InitialAttr", nil) + + expect(result:GetAttribute("InitialAttr")).toEqual(nil) + sub:Destroy() + end) + + it("propagates a descendant added two levels deep", function() + local root = Instance.new("Folder") + local middle = Instance.new("Folder") + middle.Name = "Middle" + middle.Parent = root + + local result + local sub = Brine.observeDeserialize(Brine.observeSerialize(root)):Subscribe(function(instance) + result = instance + end) + + local grandchild = Instance.new("StringValue") + grandchild.Name = "Grandchild" + grandchild.Value = "deep" + grandchild.Parent = middle + + local resolvedMiddle = result:FindFirstChild("Middle") + expect(resolvedMiddle).never.toEqual(nil) + local resolvedGrandchild = resolvedMiddle:FindFirstChild("Grandchild") + expect(resolvedGrandchild).never.toEqual(nil) + expect(resolvedGrandchild.Value).toEqual("deep") + sub:Destroy() + end) +end) + +-- Asserts directly on the wire packets emitted by observeSerialize. Each +-- emission is decoded with a fresh BrineContext so the test sees the raw +-- {type = ..., ...} packet table rather than the side effect on the receiver. +describe("Brine.observeSerialize packet shape", function() + local function decodePacket(stream, encodedReferences) + local context = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + return Brine._fromStream(context, stream, encodedReferences) + end + + local function collectPackets(observable) + local packets = {} + local sub = observable:Subscribe(function(stream, encodedReferences) + table.insert(packets, decodePacket(stream, encodedReferences)) + end) + return packets, sub + end + + it("emits exactly one 'full' packet on initial subscribe with no descendants", function() + local original = Instance.new("StringValue") + + local packets, sub = collectPackets(Brine.observeSerialize(original)) + + expect(#packets).toEqual(1) + expect(packets[1].type).toEqual("full") + sub:Destroy() + end) + + it("emits exactly one 'full' packet on initial subscribe with pre-existing descendants", function() + local root = Instance.new("Folder") + local first = Instance.new("StringValue") + first.Name = "First" + first.Parent = root + local second = Instance.new("StringValue") + second.Name = "Second" + second.Parent = root + + local packets, sub = collectPackets(Brine.observeSerialize(root)) + + expect(#packets).toEqual(1) + expect(packets[1].type).toEqual("full") + sub:Destroy() + end) + + it("does not emit any 'descendantAdded' packets on initial subscribe", function() + local root = Instance.new("Folder") + local child = Instance.new("StringValue") + child.Name = "Existing" + child.Parent = root + + local packets, sub = collectPackets(Brine.observeSerialize(root)) + + for _, packet in packets do + expect(packet.type).never.toEqual("descendantAdded") + end + sub:Destroy() + end) + + it("does not emit any 'change' packets on initial subscribe with attributes pre-set", function() + local original = Instance.new("Part") + original:SetAttribute("Existing", 1) + + local packets, sub = collectPackets(Brine.observeSerialize(original)) + + for _, packet in packets do + expect(packet.type).never.toEqual("change") + end + sub:Destroy() + end) + + it("emits a 'change' packet carrying the property that changed", function() + local original = Instance.new("StringValue") + + local packets, sub = collectPackets(Brine.observeSerialize(original)) + local before = #packets + + original.Value = "updated" + + expect(#packets > before).toEqual(true) + local latest = packets[#packets] + expect(latest.type).toEqual("change") + expect(latest.properties).never.toEqual(nil) + expect(latest.properties.Value).toEqual("updated") + sub:Destroy() + end) + + it("emits a 'change' packet carrying the attribute that was set", function() + local original = Instance.new("Part") + + local packets, sub = collectPackets(Brine.observeSerialize(original)) + local before = #packets + + original:SetAttribute("Late", 99) + + expect(#packets > before).toEqual(true) + local latest = packets[#packets] + expect(latest.type).toEqual("change") + expect(latest.attributes).never.toEqual(nil) + expect(latest.attributes.Late).toEqual(99) + sub:Destroy() + end) + + it("emits a 'descendantAdded' packet when a descendant is parented after subscribe", function() + local root = Instance.new("Folder") + + local packets, sub = collectPackets(Brine.observeSerialize(root)) + local before = #packets + + local child = Instance.new("StringValue") + child.Name = "Late" + child.Parent = root + StepUtils.deferWait() + + local sawDescendantAdded = false + for index = before + 1, #packets do + if packets[index].type == "descendantAdded" then + sawDescendantAdded = true + break + end + end + expect(sawDescendantAdded).toEqual(true) + sub:Destroy() + end) + + it("emits a 'descendantRemoving' packet when a descendant is removed after subscribe", function() + local root = Instance.new("Folder") + local child = Instance.new("StringValue") + child.Name = "Removable" + child.Parent = root + + local packets, sub = collectPackets(Brine.observeSerialize(root)) + local before = #packets + + child.Parent = nil + StepUtils.deferWait() + + local sawDescendantRemoving = false + for index = before + 1, #packets do + if packets[index].type == "descendantRemoving" then + sawDescendantRemoving = true + break + end + end + expect(sawDescendantRemoving).toEqual(true) + sub:Destroy() + end) + + it("does not emit any 'full' packets on a property change", function() + local original = Instance.new("StringValue") + + local packets, sub = collectPackets(Brine.observeSerialize(original)) + local before = #packets + + original.Value = "updated" + + for index = before + 1, #packets do + expect(packets[index].type).never.toEqual("full") + end + sub:Destroy() + end) +end) + +-- Direct (non-Rx) repro: build the same packet shape that observeSerialize +-- emits internally and roundtrip it via _toStream / _fromStream. Used to +-- isolate whether the hang is in the encode/decode layer or in the observer. +describe("Brine direct packet roundtrip (non-Rx)", function() + it("can roundtrip a manually-constructed 'full' packet", function() + local original = Instance.new("StringValue") + original.Value = "hello" + + local writeContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + local data = Brine._toIntermediate(writeContext, original) + local packet = { + type = "full", + data = data, + } + + local stream = Brine._toStream(writeContext, packet) + + local readContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + local intermediate = Brine._fromStream(readContext, stream, nil) + + expect(intermediate.type).toEqual("full") + end) + + local function buildDescendantAddedStream() + local root = Instance.new("Folder") + local child = Instance.new("StringValue") + child.Name = "Test" + child.Parent = root + + local writeContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + writeContext.InstanceRegistry:InstanceToId(root) + local data = Brine._toIntermediate(writeContext, child) + local packet = { + type = "descendantAdded", + parentInstanceId = writeContext.InstanceRegistry:InstanceToId(root), + instanceId = writeContext.InstanceRegistry:InstanceToId(child), + data = data, + } + return Brine._toStream(writeContext, packet) + end + + it("step 1: can encode a 'descendantAdded' packet", function() + local stream = buildDescendantAddedStream() + expect(#stream > 0).toEqual(true) + end) + + it("step 2: can buffer.fromstring the encoded stream", function() + local stream = buildDescendantAddedStream() + local buff = buffer.fromstring(stream) + expect(buffer.len(buff) > 0).toEqual(true) + end) + + it("step 3: can DecompressBuffer the encoded stream", function() + local stream = buildDescendantAddedStream() + local buff = buffer.fromstring(stream) + local EncodingService = game:GetService("EncodingService") + local decompressed = EncodingService:DecompressBuffer(buff, Enum.CompressionAlgorithm.Zstd) + expect(buffer.len(decompressed) > 0).toEqual(true) + end) + + it("step 4: can BufferEncoder.read the decompressed stream", function() + local stream = buildDescendantAddedStream() + local buff = buffer.fromstring(stream) + local EncodingService = game:GetService("EncodingService") + local decompressed = EncodingService:DecompressBuffer(buff, Enum.CompressionAlgorithm.Zstd) + local BufferEncoder = require("BufferEncoder") + local intermediate = BufferEncoder.read(decompressed, nil, { + allowdeduplication = true, + allowreferences = true, + references = nil, + rbxenum_behavior = "compact", + }) + expect(intermediate.type).toEqual("descendantAdded") + end) + + it("can decode N 'descendantAdded' streams back-to-back", function() + local N = 5 + local streams = {} + for _ = 1, N do + local stream = buildDescendantAddedStream() + table.insert(streams, stream) + end + + for _, stream in streams do + local readContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + local intermediate = Brine._fromStream(readContext, stream, nil) + expect(intermediate.type).toEqual("descendantAdded") + end + end) + + it("subscribe-only with 1 pre-existing descendant", function() + local root = Instance.new("Folder") + local first = Instance.new("StringValue") + first.Name = "First" + first.Parent = root + + local rawPackets = {} + local sub = Brine.observeSerialize(root):Subscribe(function(stream, _encodedReferences) + table.insert(rawPackets, stream) + end) + sub:Destroy() + + expect(#rawPackets > 0).toEqual(true) + end) + + it("subscribe-only with 2 pre-existing descendants", function() + local root = Instance.new("Folder") + local first = Instance.new("StringValue") + first.Name = "First" + first.Parent = root + local second = Instance.new("StringValue") + second.Name = "Second" + second.Parent = root + + local rawPackets = {} + local sub = Brine.observeSerialize(root):Subscribe(function(stream, _encodedReferences) + table.insert(rawPackets, stream) + end) + sub:Destroy() + + expect(#rawPackets > 0).toEqual(true) + end) + + it("can _toIntermediate a folder with 2 descendants", function() + local root = Instance.new("Folder") + local first = Instance.new("StringValue") + first.Name = "First" + first.Parent = root + local second = Instance.new("StringValue") + second.Name = "Second" + second.Parent = root + + local writeContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + local data = Brine._toIntermediate(writeContext, root) + expect(data).never.toEqual(nil) + end) + + it("can _toStream a folder with 2 descendants as a full frame", function() + local root = Instance.new("Folder") + local first = Instance.new("StringValue") + first.Name = "First" + first.Parent = root + local second = Instance.new("StringValue") + second.Name = "Second" + second.Parent = root + + local writeContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + local data = Brine._toIntermediate(writeContext, root) + local packet = { type = "full", data = data } + local stream = Brine._toStream(writeContext, packet) + expect(#stream > 0).toEqual(true) + end) + + it("shared-context: encode multiple packets reusing the same writeContext", function() + local root = Instance.new("Folder") + local first = Instance.new("StringValue") + first.Name = "First" + first.Parent = root + local second = Instance.new("StringValue") + second.Name = "Second" + second.Parent = root + + local writeContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + + -- Encode root as a full frame + local rootData = Brine._toIntermediate(writeContext, root) + local fullPacket = { type = "full", data = rootData } + local fullStream = Brine._toStream(writeContext, fullPacket) + expect(#fullStream > 0).toEqual(true) + + -- Encode first child as a descendantAdded + local firstData = Brine._toIntermediate(writeContext, first) + local firstPacket = { + type = "descendantAdded", + parentInstanceId = writeContext.InstanceRegistry:InstanceToId(root), + instanceId = writeContext.InstanceRegistry:InstanceToId(first), + data = firstData, + } + local firstStream = Brine._toStream(writeContext, firstPacket) + expect(#firstStream > 0).toEqual(true) + end) + + it("can roundtrip a 'full' packet whose data is a parented child", function() + local root = Instance.new("Folder") + local child = Instance.new("StringValue") + child.Name = "Test" + child.Parent = root + + local writeContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + writeContext.InstanceRegistry:InstanceToId(root) + local data = Brine._toIntermediate(writeContext, child) + local packet = { + type = "full", + data = data, + } + + local stream = Brine._toStream(writeContext, packet) + + local readContext = BrineContext.new(BrineOptionUtils.defaultOptions(nil)) + local intermediate = Brine._fromStream(readContext, stream, nil) + + expect(intermediate.type).toEqual("full") + end) +end) diff --git a/src/brine/src/Shared/BrineTypes.lua b/src/brine/src/Shared/BrineTypes.lua index 81b4d3d4af..6f2229cbf9 100644 --- a/src/brine/src/Shared/BrineTypes.lua +++ b/src/brine/src/Shared/BrineTypes.lua @@ -11,13 +11,15 @@ export type Intermediate = { [any]: any } export type BrineProperties = { [string]: any } export type BrineAttributes = { [string]: any } export type BrineTags = { string } +export type BrineChildren = { BrineInstance } export type BrineInstance = { + Id: string, ClassName: string, Properties: BrineProperties?, Attributes: BrineAttributes?, Tags: BrineTags?, - Children: { BrineInstance }?, + Children: BrineChildren?, } export type ExtraData = any @@ -54,6 +56,40 @@ export type SafeOptions = { export type BrineOptions = SharedOptons & SerializeBrineOptions & DeserializeBrineOptions export type SafeBrineOptions = SafeOptions & BrineOptions +export type FullFramePacket = { + type: "full", + data: Intermediate, +} + +export type ChangePacket = { + type: "change", + instanceId: string, + properties: BrineProperties?, + attributes: BrineAttributes?, + children: BrineChildren?, + tags: BrineTags?, + -- Lists of property/attribute names whose value was cleared to nil. Carried + -- separately because Lua tables cannot store nil values, so a `properties` + -- table cannot represent "this property is now nil" directly. + clearedProperties: { string }?, + clearedAttributes: { string }?, +} + +export type DescendantTreeAddedPacket = { + type: "descendantAdded", + parentInstanceId: string, + instanceId: string, + data: BrineInstance, +} + +export type DescendantTreeRemovingPacket = { + type: "descendantRemoving", + instanceId: string, +} + +export type BrinePacket = FullFramePacket | ChangePacket | DescendantTreeAddedPacket | DescendantTreeRemovingPacket +export type EncodedBrinePacket = string + return { PENDING_INSTANCE_MARKER = Symbol.named("PendingInstanceMarker"), } diff --git a/src/brine/src/Shared/Context/BrineContext.lua b/src/brine/src/Shared/Context/BrineContext.lua index e3a22b07f4..bdee9d215e 100644 --- a/src/brine/src/Shared/Context/BrineContext.lua +++ b/src/brine/src/Shared/Context/BrineContext.lua @@ -6,6 +6,7 @@ local require = require(script.Parent.loader).load(script) +local BrineInstanceRegistry = require("BrineInstanceRegistry") local BrineTypes = require("BrineTypes") local Symbol = require("Symbol") @@ -21,6 +22,7 @@ export type BrineContext = typeof(setmetatable( _instanceToBrineInstance: { [Instance]: BrineTypes.BrineInstance }, _brineInstanceToInstance: { [BrineTypes.BrineInstance]: Instance }, Options: BrineTypes.SafeBrineOptions, + InstanceRegistry: BrineInstanceRegistry.BrineInstanceRegistry, }, {} :: typeof({ __index = BrineContext }) )) @@ -31,6 +33,8 @@ function BrineContext.new(options: BrineTypes.SafeBrineOptions): BrineContext -- selene: allow(undefined_variable) self.SecurityCapabilities = SecurityCapabilities.fromCurrent() self.Options = options + self.InstanceRegistry = BrineInstanceRegistry.new() + self._instanceToBrineInstance = {} self._brineInstanceToInstance = {} @@ -50,6 +54,9 @@ function BrineContext.StoreSerialization(self: BrineContext, instance: Instance, self._brineInstanceToInstance[data] = instance end +--[=[ + Done on encode +]=] function BrineContext.ReplaceInstancesWithSerializedInstances( self: BrineContext, intermediate: BrineTypes.Intermediate? @@ -96,6 +103,7 @@ function BrineContext.ReplaceInstancesWithSerializedInstances( end local result = encode(intermediate) + if result == nil then return {} else @@ -103,18 +111,23 @@ function BrineContext.ReplaceInstancesWithSerializedInstances( end end +--[=[ + Done on decode +]=] function BrineContext.ReplaceSerializedInstancesWithInstances( self: BrineContext, intermediate: BrineTypes.Intermediate ): BrineTypes.Intermediate - local decodedLookup = {} + local seenTables = {} local function decode(value: any): any if value == BrineTypes.PENDING_INSTANCE_MARKER then return BrineTypes.PENDING_INSTANCE_MARKER + elseif value == UNSET_VALUE then + return nil elseif type(value) == "table" then - if decodedLookup[value] ~= nil then - return decodedLookup[value] + if seenTables[value] ~= nil then + return seenTables[value] end local finalValue = value @@ -124,6 +137,7 @@ function BrineContext.ReplaceSerializedInstancesWithInstances( if existing == nil then local constructed = self.Options.instanceHook.decode(value) value[BrineTypes.PENDING_INSTANCE_MARKER] = constructed + self.InstanceRegistry:SetInstanceId(constructed, value.Id) self:StoreSerialization(constructed, value) finalValue = constructed else @@ -131,7 +145,8 @@ function BrineContext.ReplaceSerializedInstancesWithInstances( end end - decodedLookup[value] = finalValue + assert(finalValue ~= nil, "Decoded instance cannot be nil") + seenTables[value] = finalValue for k, v in value do value[decode(k)] = decode(v) @@ -146,4 +161,9 @@ function BrineContext.ReplaceSerializedInstancesWithInstances( return decode(intermediate) end +function BrineContext.ClearState(self: BrineContext): () + self._instanceToBrineInstance = {} + self._brineInstanceToInstance = {} +end + return BrineContext diff --git a/src/brine/src/Shared/Context/BrineInstanceRegistry.lua b/src/brine/src/Shared/Context/BrineInstanceRegistry.lua new file mode 100644 index 0000000000..0b699e7811 --- /dev/null +++ b/src/brine/src/Shared/Context/BrineInstanceRegistry.lua @@ -0,0 +1,76 @@ +--!strict +--[=[ + @class BrineInstanceRegistry +]=] + +local require = require(script.Parent.loader).load(script) + +local BaseObject = require("BaseObject") + +local BrineInstanceRegistry = setmetatable({}, BaseObject) +BrineInstanceRegistry.ClassName = "BrineInstanceRegistry" +BrineInstanceRegistry.__index = BrineInstanceRegistry + +export type InstanceId = string + +export type BrineInstanceRegistry = typeof(setmetatable( + {} :: { + _idToInstance: { [InstanceId]: Instance }, + _instanceToId: { [Instance]: InstanceId }, + _counter: number, + }, + {} :: typeof({ __index = BrineInstanceRegistry }) +)) + +function BrineInstanceRegistry.new(): BrineInstanceRegistry + local self: BrineInstanceRegistry = setmetatable(BaseObject.new() :: any, BrineInstanceRegistry) + + self._idToInstance = setmetatable({} :: any, { __mode = "" }) + self._instanceToId = setmetatable({} :: any, { __mode = "" }) + self._counter = 0 + + return self +end + +function BrineInstanceRegistry.IdToInstance(self: BrineInstanceRegistry, id: InstanceId): Instance? + return self._idToInstance[id] +end + +function BrineInstanceRegistry.FindInstanceId(self: BrineInstanceRegistry, instance: Instance): InstanceId? + return self._instanceToId[instance] +end + +function BrineInstanceRegistry.InstanceToId(self: BrineInstanceRegistry, instance: Instance): InstanceId + local found = self._instanceToId[instance] + if found then + return found + end + + local id = self._counter + self._counter = self._counter + 1 + + local newId = tostring(id) + self._idToInstance[newId] = instance + self._instanceToId[instance] = newId + return newId +end + +function BrineInstanceRegistry.SetInstanceId(self: BrineInstanceRegistry, instance: Instance, id: InstanceId?) + local currentId = self._instanceToId[instance] + if currentId then + self._idToInstance[currentId] = nil + end + + if id ~= nil then + local currentInstance = self._idToInstance[id] + if currentInstance then + self._instanceToId[currentInstance] = nil + end + + self._idToInstance[id] = instance + end + + self._instanceToId[instance] = id :: any +end + +return BrineInstanceRegistry diff --git a/src/brine/src/Shared/Context/BrineOptionUtils.lua b/src/brine/src/Shared/Context/BrineOptionUtils.lua index 936a455c84..9a8198cb7e 100644 --- a/src/brine/src/Shared/Context/BrineOptionUtils.lua +++ b/src/brine/src/Shared/Context/BrineOptionUtils.lua @@ -26,8 +26,8 @@ function BrineOptionUtils.defaultOptions(options: BrineTypes.BrineOptions?): Bri return table.freeze({ includeDescendants = if current.includeDescendants == nil then true else current.includeDescendants, - includeAttributes = if current.includeAttributes == nil then true else current.includeDescendants, - includeTags = if current.includeTags == nil then true else current.includeDescendants, + includeAttributes = if current.includeAttributes == nil then true else current.includeAttributes, + includeTags = if current.includeTags == nil then true else current.includeTags, instanceHook = if current.instanceHook == nil then DEFAULT_HOOK else current.instanceHook, references = current.references, }) diff --git a/src/brine/src/Shared/DataTypes/BrineInstanceEncoder.lua b/src/brine/src/Shared/DataTypes/BrineInstanceEncoder.lua index bb05230d05..2788797614 100644 --- a/src/brine/src/Shared/DataTypes/BrineInstanceEncoder.lua +++ b/src/brine/src/Shared/DataTypes/BrineInstanceEncoder.lua @@ -93,6 +93,7 @@ function BrineInstanceEncoder.encodeInstance( local brineInstance: any = {} context:StoreSerialization(instance, brineInstance) + brineInstance.Id = context.InstanceRegistry:InstanceToId(instance) brineInstance.ClassName = instance.ClassName brineInstance.Properties = BrineInstanceEncoder.encodeProperties(context, instance) brineInstance.Attributes = BrineInstanceEncoder.encodeAttributes(context, instance) @@ -172,6 +173,7 @@ function BrineInstanceEncoder.decodeInstance( BrineInstanceEncoder.decodeProperties(context, instance, data.Properties) BrineInstanceEncoder.decodeAttributes(context, instance, data.Attributes) BrineInstanceEncoder.decodeChildren(context, instance, data.Children) + BrineInstanceEncoder.decodeTags(context, instance, data.Tags) end end @@ -182,10 +184,13 @@ function BrineInstanceEncoder.decodeInstance( end function BrineInstanceEncoder.decodeAttributes( - _context: BrineContext.BrineContext, + context: BrineContext.BrineContext, instance: Instance, attributes: BrineTypes.BrineAttributes? ): () + if not context.Options.includeAttributes then + return + end if not attributes then return end @@ -195,11 +200,31 @@ function BrineInstanceEncoder.decodeAttributes( end end +function BrineInstanceEncoder.decodeTags( + context: BrineContext.BrineContext, + instance: Instance, + tags: BrineTypes.BrineTags? +): () + if not context.Options.includeTags then + return + end + if not tags then + return + end + + for _, tag in tags do + instance:AddTag(tag) + end +end + function BrineInstanceEncoder.decodeChildren( context: BrineContext.BrineContext, instance: Instance, children: { BrineTypes.BrineInstance }? ): () + if not context.Options.includeDescendants then + return + end if not children then return end @@ -228,11 +253,57 @@ function BrineInstanceEncoder.decodeProperties( end for propertyName, value in properties do - -- If we constructed a different class type then the instance we need to guard against this - if propertyMetadata.lookup[propertyName] then + if propertyName == "Parent" then + -- Parent is filtered out of the encoded properties list (it's a containment + -- relationship, not a serialized property), but a live observer still needs + -- to forward reparenting within the source tree. + (instance :: any).Parent = value + elseif propertyMetadata.lookup[propertyName] then + -- If we constructed a different class type then the instance we need to guard against this (instance :: any)[propertyName] = value end end end +function BrineInstanceEncoder.clearProperties( + context: BrineContext.BrineContext, + instance: Instance, + clearedProperties: { string }? +): () + if not clearedProperties then + return + end + + local propertyMetadata = + BrineInstanceReflection.getEncodedPropertiesMemoized(instance.ClassName, context.SecurityCapabilities) + if propertyMetadata == nil then + return + end + + for _, propertyName in clearedProperties do + if propertyName == "Parent" then + (instance :: any).Parent = nil + elseif propertyMetadata.lookup[propertyName] then + (instance :: any)[propertyName] = nil + end + end +end + +function BrineInstanceEncoder.clearAttributes( + context: BrineContext.BrineContext, + instance: Instance, + clearedAttributes: { string }? +): () + if not context.Options.includeAttributes then + return + end + if not clearedAttributes then + return + end + + for _, attributeName in clearedAttributes do + instance:SetAttribute(attributeName, nil) + end +end + return BrineInstanceEncoder diff --git a/src/camera/README.md b/src/camera/README.md index 3c200b4ce1..f76286683b 100644 --- a/src/camera/README.md +++ b/src/camera/README.md @@ -68,7 +68,7 @@ This class takes two arguments and returns the summation of the two * Arguments can be either `CameraState` or a CameraEffect, assuming the effect has a `CameraState` member #### FadingCamera -This classes allows the effects of a camera to be faded / varied based upon a spring +This class allows the effects of a camera to be faded / varied based upon a spring * Starts at 0 percent effect diff --git a/src/servicebag/README.md b/src/servicebag/README.md index 58d3668d85..2265db3cab 100644 --- a/src/servicebag/README.md +++ b/src/servicebag/README.md @@ -24,7 +24,7 @@ npm install @quenty/servicebag --save - Remove requirement for many services to be loaded - Make installing new modules really easy - Make testing easier -- Reduce maintaince costs +- Reduce maintenance costs - Explicitly declare service pattern - Force declaration of service usage - Make it easy to trace service dependencies @@ -82,7 +82,7 @@ function TestClass.new(serviceBag) self._serviceProvider = assert(serviceBag, "No serviceBag") self._transparencyService = self._serviceProvider:GetRequiredService(TransparencyService) - + return self end diff --git a/tools/cli-output-helpers/src/reporting/spinner-reporter.ts b/tools/cli-output-helpers/src/reporting/spinner-reporter.ts index cea468da68..4b260e5e6f 100644 --- a/tools/cli-output-helpers/src/reporting/spinner-reporter.ts +++ b/tools/cli-output-helpers/src/reporting/spinner-reporter.ts @@ -44,6 +44,7 @@ export class SpinnerReporter extends BaseReporter { private _spinnerFrame: number = 0; private _extraLines = 0; private _originalStdoutWrite: typeof process.stdout.write | undefined; + private _originalStderrWrite: typeof process.stderr.write | undefined; private _isRendering = false; constructor(state: IStateTracker, options: SpinnerReporterOptions) { @@ -63,16 +64,25 @@ export class SpinnerReporter extends BaseReporter { process.stdout.write('\x1b[?25l'); this._renderedLineCount = 0; - // Intercept stdout to track external writes that shift the cursor + // Intercept stdout/stderr to track external writes that shift the cursor. + // Both streams visually consume rows in the same terminal, so both must + // count toward the cursor-rewind math. this._originalStdoutWrite = process.stdout.write.bind(process.stdout); + this._originalStderrWrite = process.stderr.write.bind(process.stderr); const self = this; + const countNewlines = (chunk: any): void => { + if (self._isRendering) return; + const str = typeof chunk === 'string' ? chunk : chunk.toString(); + self._extraLines += (str.match(/\n/g) || []).length; + }; process.stdout.write = function (chunk: any, ...args: any[]) { - if (!self._isRendering) { - const str = typeof chunk === 'string' ? chunk : chunk.toString(); - self._extraLines += (str.match(/\n/g) || []).length; - } + countNewlines(chunk); return self._originalStdoutWrite!.call(process.stdout, chunk, ...args); } as any; + process.stderr.write = function (chunk: any, ...args: any[]) { + countNewlines(chunk); + return self._originalStderrWrite!.call(process.stderr, chunk, ...args); + } as any; this._render(); this._renderInterval = setInterval(() => { @@ -88,11 +98,15 @@ export class SpinnerReporter extends BaseReporter { } this._render(); - // Restore original stdout.write + // Restore original stdout.write / stderr.write if (this._originalStdoutWrite) { process.stdout.write = this._originalStdoutWrite; this._originalStdoutWrite = undefined; } + if (this._originalStderrWrite) { + process.stderr.write = this._originalStderrWrite; + this._originalStderrWrite = undefined; + } process.stdout.write('\x1b[?25h'); console.log(''); diff --git a/tools/nevermore-cli/src/commands/batch-command/batch-test-command.ts b/tools/nevermore-cli/src/commands/batch-command/batch-test-command.ts index e574b0de61..276e9e7dfe 100644 --- a/tools/nevermore-cli/src/commands/batch-command/batch-test-command.ts +++ b/tools/nevermore-cli/src/commands/batch-command/batch-test-command.ts @@ -26,10 +26,12 @@ import { type BatchTestResult, type BatchTestSummary, CompositeReporter, + GithubCommentTableReporter, GroupedReporter, JsonFileReporter, SpinnerReporter, SummaryTableReporter, + createTestCommentConfig, } from '../../utils/testing/reporting/index.js'; import { runSingleTestAsync, @@ -174,6 +176,15 @@ async function _runAsync(args: BatchTestArgs): Promise { if (args.output) { reporters.push(new JsonFileReporter(state, args.output)); } + if (isCI()) { + reporters.push( + new GithubCommentTableReporter( + state, + createTestCommentConfig(), + concurrency + ) + ); + } return reporters; } ); @@ -227,6 +238,10 @@ async function _runAsync(args: BatchTestArgs): Promise { placeId: pkg.target.placeId, success: result.success, logs: result.logs, + // Aggregated mode reports per-package pcall time; the outer + // wall-clock would otherwise be identical for every package + // (they all await the same shared batch execution). + durationMs: result.durationMs, progressSummary: result.testCounts ? { kind: 'test-counts' as const, ...result.testCounts } : undefined, @@ -273,17 +288,28 @@ function _createBroadcastReporter( target: Reporter, packageNames: string[] ): Reporter { + // Phases from the inner context apply to the whole batch in aggregated mode. + // The cloud poll loop re-fires 'executing' on every poll tick, and we don't + // want that to keep flushing identical updates downstream — dedupe on the + // last broadcast phase so each transition is reported exactly once. + let lastBroadcastPhase: JobPhase | undefined; return { startAsync: async () => {}, stopAsync: async () => {}, onPackageStart: () => {}, onPackageResult: () => {}, onPackagePhaseChange: (_name: string, phase: JobPhase) => { + if (lastBroadcastPhase === phase) return; + lastBroadcastPhase = phase; for (const name of packageNames) { target.onPackagePhaseChange(name, phase); } }, onPackageProgressUpdate: (_name: string, progress: ProgressSummary) => { + // Upload progress (byte transfer) is genuinely informative when fanned + // out. Poll progress during 'executing' is just an incrementing counter + // duplicated across every row, so skip it once we're in lock-step. + if (lastBroadcastPhase === 'executing') return; for (const name of packageNames) { target.onPackageProgressUpdate(name, progress); } diff --git a/tools/nevermore-cli/src/utils/batch/batch-runner.ts b/tools/nevermore-cli/src/utils/batch/batch-runner.ts index 1b92f64e0d..0fd9636f8d 100644 --- a/tools/nevermore-cli/src/utils/batch/batch-runner.ts +++ b/tools/nevermore-cli/src/utils/batch/batch-runner.ts @@ -7,6 +7,19 @@ import { } from '@quenty/cli-output-helpers/reporting'; import { type TargetPackage } from './changed-packages-utils.js'; +/** Partial result returned by executeAsync — durationMs is optional. */ +export type PartialBatchResult = Omit< + TResult, + 'durationMs' +> & { + /** + * Inner-measured duration. When provided, overrides the outer wall-clock + * timing (useful in aggregated batch mode where every package's outer + * await resolves at the same instant). + */ + durationMs?: number; +}; + export interface BatchOptions { packages: TargetPackage[]; concurrency?: number; @@ -16,7 +29,7 @@ export interface BatchOptions { executeAsync: ( pkg: TargetPackage, reporter: Reporter - ) => Promise>; + ) => Promise>; } export async function runBatchAsync( @@ -85,7 +98,7 @@ async function _runOneAsync( executeAsync: ( pkg: TargetPackage, reporter: Reporter - ) => Promise>, + ) => Promise>, reporter: Reporter, bufferOutput: boolean, stateTracker?: IStateTracker @@ -96,7 +109,8 @@ async function _runOneAsync( const execute = async (): Promise => { try { const partial = await executeAsync(pkg, reporter); - return { ...partial, durationMs: Date.now() - startMs } as TResult; + const durationMs = partial.durationMs ?? Date.now() - startMs; + return { ...partial, durationMs } as TResult; } catch (err) { const errorMessage = err instanceof Error ? err.message : String(err); const currentPhase = stateTracker?.getCurrentPhase(pkg.name); diff --git a/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts b/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts index bf9702f226..44e259a51b 100644 --- a/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts +++ b/tools/nevermore-cli/src/utils/job-context/batch-script-job-context.ts @@ -132,7 +132,7 @@ export class BatchScriptJobContext implements JobContext { return { success: false }; } - return { success: result.success }; + return { success: result.success, durationMs: result.durationMs }; } async getLogsAsync(deployment: Deployment): Promise { diff --git a/tools/nevermore-cli/src/utils/job-context/job-context.ts b/tools/nevermore-cli/src/utils/job-context/job-context.ts index f59e551ba0..1be8dd2b19 100644 --- a/tools/nevermore-cli/src/utils/job-context/job-context.ts +++ b/tools/nevermore-cli/src/utils/job-context/job-context.ts @@ -17,6 +17,13 @@ export interface RunScriptOptions { export interface ScriptRunResult { /** Whether the execution infrastructure succeeded (not test assertions). */ success: boolean; + /** + * Optional inner script execution time, reported when the context can + * measure it directly (e.g. aggregated batch mode reports per-package + * pcall durations). When undefined, callers should fall back to their own + * wall-clock measurement. + */ + durationMs?: number; } /** diff --git a/tools/nevermore-cli/src/utils/sourcemap/sourcemap-resolver.ts b/tools/nevermore-cli/src/utils/sourcemap/sourcemap-resolver.ts index d64d8c2ab4..7deeb24bef 100644 --- a/tools/nevermore-cli/src/utils/sourcemap/sourcemap-resolver.ts +++ b/tools/nevermore-cli/src/utils/sourcemap/sourcemap-resolver.ts @@ -55,7 +55,11 @@ function _walkNode( ): void { const luaFile = _findLuaFilePath(node.filePaths); if (luaFile) { - const relative = path.relative(repoRoot, luaFile); + // Rojo's sourcemap.json uses POSIX separators on every platform, and + // downstream consumers (test output, CI annotations) compare paths + // assuming POSIX style. Normalize so Windows backslashes from + // `path.relative` don't leak through. + const relative = path.relative(repoRoot, luaFile).replace(/\\/g, '/'); index.set(dottedPath, relative); } diff --git a/tools/nevermore-cli/src/utils/testing/parsers/batch-log-parser.ts b/tools/nevermore-cli/src/utils/testing/parsers/batch-log-parser.ts index 643259ab64..24f7b0e61e 100644 --- a/tools/nevermore-cli/src/utils/testing/parsers/batch-log-parser.ts +++ b/tools/nevermore-cli/src/utils/testing/parsers/batch-log-parser.ts @@ -5,6 +5,8 @@ export interface BatchPackageResult { slug: string; success: boolean; logs: string; + /** Inner pcall execution time reported by the Luau batch runner. */ + durationMs?: number; testCounts?: ParsedTestCounts; } @@ -15,6 +17,7 @@ const SUMMARY_MARKER = '===BATCH_TEST_SUMMARY==='; interface SummaryEntry { slug: string; success: boolean; + durationMs?: number; error?: string; } @@ -24,9 +27,9 @@ interface SummaryEntry { * The batch Luau template prints structured markers around each package's output: * ===BATCH_TEST_BEGIN === * ... test output ... - * ===BATCH_TEST_END PASS|FAIL=== + * ===BATCH_TEST_END PASS|FAIL === * ===BATCH_TEST_SUMMARY=== - * [{"slug":"maid","success":true}, ...] + * [{"slug":"maid","success":true,"durationMs":1234}, ...] * * Success is determined from the JSON summary (based on pcall results), which is * immune to log reordering. The BEGIN/END markers are used only for splitting logs @@ -48,10 +51,14 @@ export function parseBatchTestLogs( // ── Pass 1: Extract per-package log sections from markers ── const logSections = new Map(); + const markerDurations = new Map(); let currentSlug: string | null = null; let currentLines: string[] = []; let summaryLineIndex = -1; + // Matches " PASS|FAIL []" — slugs have no whitespace. + const endInnerPattern = /^(\S+)\s+(?:PASS|FAIL)(?:\s+(\d+))?$/; + for (let i = 0; i < lines.length; i++) { const trimmed = lines[i].trimEnd(); @@ -63,11 +70,15 @@ export function parseBatchTestLogs( if (trimmed.startsWith(END_MARKER) && trimmed.endsWith('===')) { const inner = trimmed.slice(END_MARKER.length, -3); - const spaceIndex = inner.lastIndexOf(' '); - const endSlug = spaceIndex >= 0 ? inner.slice(0, spaceIndex) : inner; + const match = endInnerPattern.exec(inner); + const endSlug = match ? match[1] : inner; + const durationStr = match?.[2]; if (currentSlug && endSlug === currentSlug) { logSections.set(currentSlug, currentLines.join('\n')); + if (durationStr !== undefined) { + markerDurations.set(currentSlug, parseInt(durationStr, 10)); + } currentSlug = null; currentLines = []; } @@ -89,6 +100,7 @@ export function parseBatchTestLogs( // ── Pass 2: Parse the JSON summary for authoritative pcall results ── const summaryResults = new Map(); + const summaryDurations = new Map(); if (summaryLineIndex >= 0 && summaryLineIndex + 1 < lines.length) { const jsonLine = lines .slice(summaryLineIndex + 1) @@ -98,6 +110,9 @@ export function parseBatchTestLogs( const entries = JSON.parse(jsonLine) as SummaryEntry[]; for (const entry of entries) { summaryResults.set(entry.slug, entry.success); + if (typeof entry.durationMs === 'number') { + summaryDurations.set(entry.slug, entry.durationMs); + } } // Log any pcall failures from the Luau template const failures = entries.filter((e) => !e.success); @@ -144,7 +159,16 @@ export function parseBatchTestLogs( } const testCounts = sectionLogs ? parseTestCounts(sectionLogs) : undefined; - results.set(packageName, { slug, success, logs: sectionLogs, testCounts }); + // Prefer the JSON summary (authoritative, immune to log reordering); + // fall back to the END-marker value if the summary was truncated. + const durationMs = summaryDurations.get(slug) ?? markerDurations.get(slug); + results.set(packageName, { + slug, + success, + logs: sectionLogs, + durationMs, + testCounts, + }); } return results; diff --git a/tools/nevermore-cli/src/utils/testing/runner/test-runner.ts b/tools/nevermore-cli/src/utils/testing/runner/test-runner.ts index 09dd854ebc..f1ef31bcba 100644 --- a/tools/nevermore-cli/src/utils/testing/runner/test-runner.ts +++ b/tools/nevermore-cli/src/utils/testing/runner/test-runner.ts @@ -12,6 +12,12 @@ export interface SingleTestResult { success: boolean; logs: string; testCounts?: ParsedTestCounts; + /** + * Inner script execution time, forwarded from the JobContext when it can + * measure pcall duration directly (aggregated batch mode). Undefined for + * non-aggregated cloud/local runs — callers should fall back to wall-clock. + */ + durationMs?: number; } export interface SingleTestOptions { @@ -66,6 +72,7 @@ export async function runSingleTestAsync( success: result.success && parsed.success, logs: parsed.logs, testCounts: parseTestCounts(parsed.logs), + durationMs: result.durationMs, }; } finally { await context.releaseAsync(deployment); diff --git a/tools/nevermore-cli/templates/batch-test-runner.luau b/tools/nevermore-cli/templates/batch-test-runner.luau index 2e868d8d90..77452961e3 100644 --- a/tools/nevermore-cli/templates/batch-test-runner.luau +++ b/tools/nevermore-cli/templates/batch-test-runner.luau @@ -32,6 +32,7 @@ end type Result = { success: boolean, slug: string, + durationMs: number, error: string?, } @@ -42,8 +43,8 @@ for _, slug in packageSlugs do if not scriptSource then warn(`[BatchTest] No script for {slug}`) print("===BATCH_TEST_BEGIN " .. slug .. "===") - print("===BATCH_TEST_END " .. slug .. " FAIL===") - table.insert(results, { slug = slug, success = false, error = "No test script found" }) + print("===BATCH_TEST_END " .. slug .. " FAIL 0===") + table.insert(results, { slug = slug, success = false, durationMs = 0, error = "No test script found" }) continue end @@ -71,6 +72,10 @@ for _, slug in packageSlugs do print("===BATCH_TEST_BEGIN " .. slug .. "===") + -- Measure pcall execution time only. The surrounding cleanup/yield cost + -- is amortized across all packages and would otherwise dominate fast tests. + local startClock = os.clock() + local testOk, testErr = pcall(function() local fn, compileErr = loadstring(scriptSource, slug) if not fn then @@ -79,6 +84,8 @@ for _, slug in packageSlugs do fn() end) + local durationMs = math.floor((os.clock() - startClock) * 1000 + 0.5) + -- Yield before the END marker for the same reason: leaked deferred -- callbacks may fire during fn() and their output must land before -- the marker in the log stream. @@ -112,12 +119,12 @@ for _, slug in packageSlugs do end if testOk then - print("===BATCH_TEST_END " .. slug .. " PASS===") - table.insert(results, { slug = slug, success = true }) + print("===BATCH_TEST_END " .. slug .. " PASS " .. tostring(durationMs) .. "===") + table.insert(results, { slug = slug, success = true, durationMs = durationMs }) else warn("[BatchTest] " .. slug .. ": " .. tostring(testErr)) - print("===BATCH_TEST_END " .. slug .. " FAIL===") - table.insert(results, { slug = slug, success = false, error = tostring(testErr) }) + print("===BATCH_TEST_END " .. slug .. " FAIL " .. tostring(durationMs) .. "===") + table.insert(results, { slug = slug, success = false, durationMs = durationMs, error = tostring(testErr) }) end end diff --git a/tools/studio-bridge/src/docker/docker-delegator.ts b/tools/studio-bridge/src/docker/docker-delegator.ts index 512f89974a..03f371b8a8 100644 --- a/tools/studio-bridge/src/docker/docker-delegator.ts +++ b/tools/studio-bridge/src/docker/docker-delegator.ts @@ -6,7 +6,7 @@ import * as fs from 'fs/promises'; import * as os from 'os'; -import * as path from 'path'; +import { posix as path } from 'path'; import { execa } from 'execa'; import { OutputHelper } from '@quenty/cli-output-helpers'; import { validateCookieAsync } from '@quenty/nevermore-cli-helpers'; diff --git a/tools/studio-bridge/src/linux/linux-config.ts b/tools/studio-bridge/src/linux/linux-config.ts index edb6b9143d..3437cc1bbb 100644 --- a/tools/studio-bridge/src/linux/linux-config.ts +++ b/tools/studio-bridge/src/linux/linux-config.ts @@ -3,7 +3,7 @@ * under Wine on Linux. */ -import * as path from 'path'; +import { posix as path } from 'path'; import * as os from 'os'; export interface LinuxStudioConfig { diff --git a/tools/studio-bridge/src/plugin/persistent-plugin-installer.test.ts b/tools/studio-bridge/src/plugin/persistent-plugin-installer.test.ts index cb1169fccf..c996f35532 100644 --- a/tools/studio-bridge/src/plugin/persistent-plugin-installer.test.ts +++ b/tools/studio-bridge/src/plugin/persistent-plugin-installer.test.ts @@ -2,8 +2,19 @@ * Unit tests for the persistent plugin installer. */ +import * as path from 'path'; import { describe, it, expect, vi, beforeEach } from 'vitest'; +// The installer joins paths via node's `path.join`, which uses the host +// separator. Compute expectations the same way so these assertions work on +// both POSIX hosts and Windows. +const PLUGINS_FOLDER = '/mock/plugins/folder'; +const PLUGIN_FILENAME = 'StudioBridgePersistentPlugin.rbxm'; +const EXPECTED_PLUGIN_PATH = path.join(PLUGINS_FOLDER, PLUGIN_FILENAME); +const STAGING_PREFIX = path.join(PLUGINS_FOLDER, `.${PLUGIN_FILENAME}.tmp-`); +const escapeRegex = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +const STAGING_REGEX = new RegExp(`^${escapeRegex(STAGING_PREFIX)}`); + const { mockGetPersistentPluginPath, mockRojoBuildAsync, @@ -83,9 +94,7 @@ describe('persistent-plugin-installer', () => { it('builds plugin and atomically renames into the plugins folder', async () => { const result = await installPersistentPluginAsync(); - expect(result).toBe( - '/mock/plugins/folder/StudioBridgePersistentPlugin.rbxm' - ); + expect(result).toBe(EXPECTED_PLUGIN_PATH); expect(mockCreateDirectoryContentsAsync).toHaveBeenCalled(); // rojo builds into the temp dir, not the plugins folder. expect(mockRojoBuildAsync).toHaveBeenCalledWith( @@ -98,13 +107,9 @@ describe('persistent-plugin-installer', () => { expect(mockCopyFile).toHaveBeenCalledTimes(1); expect(mockRename).toHaveBeenCalledTimes(1); const [renameSrc, renameDest] = mockRename.mock.calls[0]; - expect(renameDest).toBe( - '/mock/plugins/folder/StudioBridgePersistentPlugin.rbxm' - ); + expect(renameDest).toBe(EXPECTED_PLUGIN_PATH); // Staging file lives in the destination filesystem. - expect(renameSrc).toMatch( - /^\/mock\/plugins\/folder\/\.StudioBridgePersistentPlugin\.rbxm\.tmp-/ - ); + expect(renameSrc).toMatch(STAGING_REGEX); }); it('cleans up build context on error', async () => { @@ -130,9 +135,7 @@ describe('persistent-plugin-installer', () => { 'rename failed' ); expect(mockUnlink).toHaveBeenCalledWith( - expect.stringMatching( - /^\/mock\/plugins\/folder\/\.StudioBridgePersistentPlugin\.rbxm\.tmp-/ - ) + expect.stringMatching(STAGING_REGEX) ); }); }); diff --git a/tools/studio-bridge/src/process/studio-process-manager.ts b/tools/studio-bridge/src/process/studio-process-manager.ts index 783d54ae2d..023af18414 100644 --- a/tools/studio-bridge/src/process/studio-process-manager.ts +++ b/tools/studio-bridge/src/process/studio-process-manager.ts @@ -98,11 +98,13 @@ export function findPluginsFolder(): string { } return path.join(home, 'Documents', 'Roblox', 'Plugins'); } else if (process.platform === 'linux') { - // Studio runs under Wine and resolves plugins via %LOCALAPPDATA% + // Studio runs under Wine and resolves plugins via %LOCALAPPDATA%. + // Use path.posix so the produced path is a real Linux/Wine path even + // when this function is exercised on a Windows host (tests). const winePrefix = - process.env.WINEPREFIX || path.join(os.homedir(), '.wine'); + process.env.WINEPREFIX || path.posix.join(os.homedir(), '.wine'); const wineUser = process.env.USER || os.userInfo().username; - return path.join( + return path.posix.join( winePrefix, 'drive_c', 'users',