From e5d3461e5f835c9442e49eb0f304dc5fb360cb28 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:30:08 -0700 Subject: [PATCH 01/15] fix: Add ObservableMapSet:GetKeyList() --- src/observablecollection/src/Shared/ObservableMapSet.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/observablecollection/src/Shared/ObservableMapSet.lua b/src/observablecollection/src/Shared/ObservableMapSet.lua index 90ca0fbd7d..d43199d4bc 100644 --- a/src/observablecollection/src/Shared/ObservableMapSet.lua +++ b/src/observablecollection/src/Shared/ObservableMapSet.lua @@ -122,7 +122,7 @@ end Gets a list of all keys. @return { TKey } ]=] -function ObservableMapSet:ObserveKeyList() +function ObservableMapSet:GetKeyList() return self._observableMapOfSets:GetKeyList() end From bdfc7596d8e25350e65a3cc841debcfa0ad444de Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:30:16 -0700 Subject: [PATCH 02/15] feat: Add ObservableMapList:GetFirstItemForKey(key) and ObservableMapList:GetListOfValuesAtListIndex(index) --- .../src/Shared/ObservableMapList.lua | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/observablecollection/src/Shared/ObservableMapList.lua b/src/observablecollection/src/Shared/ObservableMapList.lua index 76abebc9dc..c058fd522d 100644 --- a/src/observablecollection/src/Shared/ObservableMapList.lua +++ b/src/observablecollection/src/Shared/ObservableMapList.lua @@ -92,6 +92,22 @@ function ObservableMapList:Push(observeKey, entry) return maid end +--[=[ + Gets the first item for the given key + @param key TKey + @return TValue | nil +]=] +function ObservableMapList:GetFirstItemForKey(key) + assert(key ~= nil, "Bad key") + + local observableList = self:GetListForKey(key) + if not observableList then + return nil + end + + return observableList:Get(1) +end + --[=[ Gets how many lists exist @return number @@ -244,6 +260,27 @@ function ObservableMapList:GetListForKey(key) return self._observableMapOfLists:Get(key) end +--[=[ + Gets a list of all of the entries at the given index, if it exists + + @param index number + @return ObservableList +]=] +function ObservableMapList:GetListOfValuesAtListIndex(index) + assert(type(index) == "number", "Bad index") + + local list = table.create(self._observableMapOfLists:GetCount()) + + for _, observableList in pairs(self._observableMapOfLists:GetValueList()) do + local value = observableList:Get(index) + if value ~= nil then + table.insert(list, value) + end + end + + return list +end + --[=[ Observes the observable list for the given key From 81fa0d74f3ee67b5e7145e30539e382f6194ad0b Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:30:26 -0700 Subject: [PATCH 03/15] docs: Add documentation to ObservableList:ObserveAtIndex(indexToObserve) --- src/observablecollection/src/Shared/ObservableList.lua | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/observablecollection/src/Shared/ObservableList.lua b/src/observablecollection/src/Shared/ObservableList.lua index 133f52cb05..3c033c457e 100644 --- a/src/observablecollection/src/Shared/ObservableList.lua +++ b/src/observablecollection/src/Shared/ObservableList.lua @@ -144,6 +144,11 @@ end Observes the current value at a given index. This can be useful for observing the first entry, or matching stuff up to a given slot. + ``` + list:ObserveAtIndex(1):Subscribe(print) --> prints first item + list:ObserveAtIndex(-1):Subscribe(print) --> prints last item + ``` + @param indexToObserve number @return Observable ]=] From b8043cfad4e4cfdb2c8ec5d73988bb8bae723c42 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:30:37 -0700 Subject: [PATCH 04/15] perf: Improve :GetKeyList() allocation performance --- src/observablecollection/src/Shared/ObservableCountingMap.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/observablecollection/src/Shared/ObservableCountingMap.lua b/src/observablecollection/src/Shared/ObservableCountingMap.lua index 3b104cd012..94d4305834 100644 --- a/src/observablecollection/src/Shared/ObservableCountingMap.lua +++ b/src/observablecollection/src/Shared/ObservableCountingMap.lua @@ -363,7 +363,7 @@ end @return { T } ]=] function ObservableCountingMap:GetKeyList() - local list = {} + local list = table.create(self._totalKeyCountValue.Value) for key, _ in pairs(self._map) do table.insert(list, key) end From 3878c3e8cb90df650d569abfbc5b5d96b09bdb62 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:31:18 -0700 Subject: [PATCH 05/15] fix: Handle arg that is nil --- src/tuple/src/Shared/TupleLookup.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tuple/src/Shared/TupleLookup.lua b/src/tuple/src/Shared/TupleLookup.lua index f37253e4a7..9ab1071fcf 100644 --- a/src/tuple/src/Shared/TupleLookup.lua +++ b/src/tuple/src/Shared/TupleLookup.lua @@ -31,7 +31,7 @@ function TupleLookup:ToTuple(...) local n = select("#", ...) if n == 1 then local arg = ... - if self._singleArgTuples[arg] then + if arg ~= nil and self._singleArgTuples[arg] then return self._singleArgTuples[arg] end end From 42432c2e789b5c97d2d38e40cec16c8ec2130136 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:31:31 -0700 Subject: [PATCH 06/15] fix: PromiseMaidUtils ensures maid invocation --- src/promisemaid/src/Shared/PromiseMaidUtils.lua | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/promisemaid/src/Shared/PromiseMaidUtils.lua b/src/promisemaid/src/Shared/PromiseMaidUtils.lua index 2c1f29702b..4698748731 100644 --- a/src/promisemaid/src/Shared/PromiseMaidUtils.lua +++ b/src/promisemaid/src/Shared/PromiseMaidUtils.lua @@ -33,6 +33,11 @@ function PromiseMaidUtils.whilePromise(promise, callback) callback(maid) + -- Cleanup immediately if the callback resolves the promise immeidately + if not promise:IsPending() then + maid:DoCleaning() + end + return maid end From 224dd250f2794b7f584d756828dffd9e49b0d009 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:40:04 -0700 Subject: [PATCH 07/15] feat: Add ObservableMapList:GetItemForKeyAtIndex(key, index) --- .../src/Shared/ObservableMapList.lua | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/observablecollection/src/Shared/ObservableMapList.lua b/src/observablecollection/src/Shared/ObservableMapList.lua index c058fd522d..a6ef417758 100644 --- a/src/observablecollection/src/Shared/ObservableMapList.lua +++ b/src/observablecollection/src/Shared/ObservableMapList.lua @@ -94,6 +94,7 @@ end --[=[ Gets the first item for the given key + @param key TKey @return TValue | nil ]=] @@ -108,8 +109,37 @@ function ObservableMapList:GetFirstItemForKey(key) return observableList:Get(1) end +--[=[ + Gets the item for the given key at the index + + ``` + mapList:Push("fruits", "apple") + mapList:Push("fruits", "orange") + mapList:Push("fruits", "banana") + + -- Print the last item + print(mapList:GetItemForKeyAtIndex("fruits", -1)) ==> banana + ``` + + @param key TKey + @param index number + @return TValue | nil +]=] +function ObservableMapList:GetItemForKeyAtIndex(key, index) + assert(key ~= nil, "Bad key") + assert(type(index) == "number", "Bad index") + + local observableList = self:GetListForKey(key) + if not observableList then + return nil + end + + return observableList:Get(index) +end + --[=[ Gets how many lists exist + @return number ]=] function ObservableMapList:GetListCount() @@ -118,6 +148,7 @@ end --[=[ Observes how many lists exist + @return Observable ]=] function ObservableMapList:ObserveListCount() From 4fc24b7892fcc443614f68b86e39d4ad1fd9aa56 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:42:55 -0700 Subject: [PATCH 08/15] refactor: Use table.pack on Promise invocation --- src/promise/src/Shared/Promise.lua | 65 +++++++++++++++--------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/src/promise/src/Shared/Promise.lua b/src/promise/src/Shared/Promise.lua index a1f053a6fb..0a91511ad1 100644 --- a/src/promise/src/Shared/Promise.lua +++ b/src/promise/src/Shared/Promise.lua @@ -12,6 +12,7 @@ local HttpService = game:GetService("HttpService") local ENABLE_TRACEBACK = false local _emptyRejectedPromise = nil local _emptyFulfilledPromise = nil +local EMPTY_PACKED_TUPLE = table.freeze({ n = 0; }) local Promise = {} Promise.ClassName = "Promise" @@ -138,7 +139,7 @@ function Promise.rejected(...) end local promise = Promise.new() - promise:_reject({...}, n) + promise:_reject(table.pack(...)) return promise end @@ -178,7 +179,7 @@ end ]=] function Promise:Wait() if self._fulfilled then - return unpack(self._fulfilled, 1, self._valuesLength) + return table.unpack(self._fulfilled, 1, self._fulfilled.n) elseif self._rejected then return error(tostring(self._rejected[1]), 2) else @@ -195,7 +196,7 @@ function Promise:Wait() if self._rejected then return error(tostring(self._rejected[1]), 2) else - return unpack(self._fulfilled, 1, self._valuesLength) + return table.unpack(self._fulfilled, 1, self._fulfilled.n) end end end @@ -210,9 +211,9 @@ end ]=] function Promise:Yield() if self._fulfilled then - return true, unpack(self._fulfilled, 1, self._valuesLength) + return true, table.unpack(self._fulfilled, 1, self._fulfilled.n) elseif self._rejected then - return false, unpack(self._rejected, 1, self._valuesLength) + return false, table.unpack(self._rejected, 1, self._rejected.n) else local waitingCoroutine = coroutine.running() @@ -225,9 +226,9 @@ function Promise:Yield() coroutine.yield() if self._fulfilled then - return true, unpack(self._fulfilled, 1, self._valuesLength) + return true, table.unpack(self._fulfilled, 1, self._fulfilled.n) elseif self._rejected then - return false, unpack(self._rejected, 1, self._valuesLength) + return false, table.unpack(self._rejected, 1, self._rejected.n) else error("Bad state") end @@ -247,7 +248,7 @@ function Promise:Resolve(...) local len = select("#", ...) if len == 0 then - self:_fulfill({}, 0) + self:_fulfill(EMPTY_PACKED_TUPLE) elseif self == (...) then self:Reject("TypeError: Resolved to self") elseif Promise.isPromise(...) then @@ -266,16 +267,16 @@ function Promise:Resolve(...) function(...) -- Still need to verify at this point that we're pending! if self._pendingExecuteList then - self:_reject({...}, select("#", ...)) + self:_reject(table.pack(...)) end end, nil } elseif promise2._rejected then -- rejected promise2._unconsumedException = false - self:_reject(promise2._rejected, promise2._valuesLength) + self:_reject(promise2._rejected) elseif promise2._fulfilled then -- fulfilled - self:_fulfill(promise2._fulfilled, promise2._valuesLength) + self:_fulfill(promise2._fulfilled) else error("[Promise.Resolve] - Bad promise2 state") end @@ -291,23 +292,21 @@ function Promise:Resolve(...) -- TODO: Handle thenable promises! -- Problem: Lua has :andThen() and also :Then() as two methods in promise -- implementations. - self:_fulfill({...}, len) + self:_fulfill(table.pack(...)) end end ---[=[ +--[[ Fulfills the promise with the value @param values { T } -- Params to fulfil with - @param valuesLength number @private -]=] -function Promise:_fulfill(values, valuesLength) +]] +function Promise:_fulfill(values) if not self._pendingExecuteList then return end self._fulfilled = values - self._valuesLength = valuesLength local list = self._pendingExecuteList self._pendingExecuteList = nil @@ -321,16 +320,15 @@ end @param ... T -- Params to reject with ]=] function Promise:Reject(...) - self:_reject({...}, select("#", ...)) + self:_reject(table.pack(...)) end -function Promise:_reject(values, valuesLength) +function Promise:_reject(values) if not self._pendingExecuteList then return end self._rejected = values - self._valuesLength = valuesLength local list = self._pendingExecuteList self._pendingExecuteList = nil @@ -339,7 +337,7 @@ function Promise:_reject(values, valuesLength) end -- Check for uncaught exceptions - if self._unconsumedException and self._valuesLength > 0 then + if self._unconsumedException and self._rejected.n > 0 then task.defer(function() -- Yield to end of frame, giving control back to Roblox. -- This is the equivalent of giving something back to a task manager. @@ -461,11 +459,12 @@ function Promise:Catch(onRejected) return self:Then(nil, onRejected) end + --[=[ Rejects the current promise. Utility left for Maid task ]=] function Promise:Destroy() - self:_reject({}, 0) + self:_reject(EMPTY_PACKED_TUPLE) end --[=[ @@ -480,9 +479,9 @@ end ]=] function Promise:GetResults() if self._rejected then - return false, unpack(self._rejected, 1, self._valuesLength) + return false, table.unpack(self._rejected, 1, self._rejected.n) elseif self._fulfilled then - return true, unpack(self._fulfilled, 1, self._valuesLength) + return true, table.unpack(self._fulfilled, 1, self._fulfilled.n) else error("Still pending") end @@ -492,7 +491,7 @@ function Promise:_getResolveReject() return function(...) self:Resolve(...) end, function(...) - self:_reject({...}, select("#", ...)) + self:_reject(table.pack(...)) end end @@ -510,10 +509,10 @@ function Promise:_executeThen(onFulfilled, onRejected, promise2) -- If either onFulfilled or onRejected returns a value x, run -- the Promise Resolution Procedure [[Resolve]](promise2, x). if promise2 then - promise2:Resolve(onFulfilled(unpack(self._fulfilled, 1, self._valuesLength))) + promise2:Resolve(onFulfilled(table.unpack(self._fulfilled, 1, self._fulfilled.n))) return promise2 else - local results = table.pack(onFulfilled(unpack(self._fulfilled, 1, self._valuesLength))) + local results = table.pack(onFulfilled(table.unpack(self._fulfilled, 1, self._fulfilled.n))) if results.n == 0 then return _emptyFulfilledPromise elseif results.n == 1 and Promise.isPromise(results[1]) then @@ -530,7 +529,7 @@ function Promise:_executeThen(onFulfilled, onRejected, promise2) -- If onFulfilled is not a function and promise1 is fulfilled, -- promise2 must be fulfilled with the same value as promise1. if promise2 then - promise2:_fulfill(self._fulfilled, self._valuesLength) + promise2:_fulfill(self._fulfilled) return promise2 else return self @@ -541,10 +540,10 @@ function Promise:_executeThen(onFulfilled, onRejected, promise2) -- If either onFulfilled or onRejected returns a value x, run -- the Promise Resolution Procedure [[Resolve]](promise2, x). if promise2 then - promise2:Resolve(onRejected(unpack(self._rejected, 1, self._valuesLength))) + promise2:Resolve(onRejected(table.unpack(self._rejected, 1, self._rejected.n))) return promise2 else - local results = table.pack(onRejected(unpack(self._rejected, 1, self._valuesLength))) + local results = table.pack(onRejected(table.unpack(self._rejected, 1, self._rejected.n))) if results.n == 0 then return _emptyFulfilledPromise elseif results.n == 1 and Promise.isPromise(results[1]) then @@ -561,7 +560,7 @@ function Promise:_executeThen(onFulfilled, onRejected, promise2) -- If onRejected is not a function and promise1 is rejected, promise2 must be -- rejected with the same reason as promise1. if promise2 then - promise2:_reject(self._rejected, self._valuesLength) + promise2:_reject(self._rejected) return promise2 else @@ -575,9 +574,9 @@ end -- Initialize promise values _emptyFulfilledPromise = Promise.new() -_emptyFulfilledPromise:_fulfill({}, 0) +_emptyFulfilledPromise:_fulfill(EMPTY_PACKED_TUPLE) _emptyRejectedPromise = Promise.new() -_emptyRejectedPromise:_reject({}, 0) +_emptyRejectedPromise:_reject(EMPTY_PACKED_TUPLE) return Promise \ No newline at end of file From ecf893949ba127d97b45c946cf320cf7b987e9eb Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:46:29 -0700 Subject: [PATCH 09/15] feat: Remove TemplateContainerUtils.reparentFromWorkspaceIfNeeded() --- .../src/Shared/TemplateContainerUtils.lua | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 src/templateprovider/src/Shared/TemplateContainerUtils.lua diff --git a/src/templateprovider/src/Shared/TemplateContainerUtils.lua b/src/templateprovider/src/Shared/TemplateContainerUtils.lua deleted file mode 100644 index 2b500992cc..0000000000 --- a/src/templateprovider/src/Shared/TemplateContainerUtils.lua +++ /dev/null @@ -1,39 +0,0 @@ ---[=[ - Utility functions for the TemplateProvider - @class TemplateContainerUtils -]=] - -local Workspace = game:GetService("Workspace") -local RunService = game:GetService("RunService") - -local TemplateContainerUtils = {} - -function TemplateContainerUtils.reparentFromWorkspaceIfNeeded(parent, name) - assert(typeof(parent) == "Instance", "Bad parent") - assert(type(name) == "string", "Bad name") - - local workspaceContainer = Workspace:FindFirstChild(name) - local parentedContainer = parent:FindFirstChild(name) - if workspaceContainer then - if parentedContainer then - error(string.format("Duplicate container in %q and %q", - workspaceContainer:GetFullName(), - parentedContainer:GetFullName())) - end - - -- Reparent - if RunService:IsRunning() then - workspaceContainer.Parent = parent - end - - return workspaceContainer - end - - if not parentedContainer then - error(string.format("No template container with name %q in %q", parent:GetFullName(), name)) - end - - return parentedContainer -end - -return TemplateContainerUtils \ No newline at end of file From 00cf87a8b73f893f7a736e938389923df096f203 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:46:23 -0700 Subject: [PATCH 10/15] refactor: Move ModuleProvider to its own folder --- src/templateprovider/src/Shared/{ => Modules}/ModuleProvider.lua | 0 .../src/Shared/{ => Modules}/ModuleProviderFakeLoader.lua | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/templateprovider/src/Shared/{ => Modules}/ModuleProvider.lua (100%) rename src/templateprovider/src/Shared/{ => Modules}/ModuleProviderFakeLoader.lua (100%) diff --git a/src/templateprovider/src/Shared/ModuleProvider.lua b/src/templateprovider/src/Shared/Modules/ModuleProvider.lua similarity index 100% rename from src/templateprovider/src/Shared/ModuleProvider.lua rename to src/templateprovider/src/Shared/Modules/ModuleProvider.lua diff --git a/src/templateprovider/src/Shared/ModuleProviderFakeLoader.lua b/src/templateprovider/src/Shared/Modules/ModuleProviderFakeLoader.lua similarity index 100% rename from src/templateprovider/src/Shared/ModuleProviderFakeLoader.lua rename to src/templateprovider/src/Shared/Modules/ModuleProviderFakeLoader.lua From 7b5df24cd0759668d2a5a7d59f1798bca9ddf0f9 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 14:42:10 -0700 Subject: [PATCH 11/15] feat: Modernize template provider to new specifications --- .../src/Shared/AnimationProvider.lua | 2 +- .../CmdrTemplateProviderServer/init.lua | 2 +- src/templateprovider/README.md | 28 +- src/templateprovider/package.json | 7 + .../Util/TemplateReplicationModes.lua | 13 + .../Util/TemplateReplicationModesUtils.lua | 26 + .../src/Shared/TaggedTemplateProvider.lua | 52 +- .../src/Shared/TemplateProvider.lua | 709 ++++++++++++------ 8 files changed, 530 insertions(+), 309 deletions(-) create mode 100644 src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModes.lua create mode 100644 src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModesUtils.lua diff --git a/src/animationprovider/src/Shared/AnimationProvider.lua b/src/animationprovider/src/Shared/AnimationProvider.lua index eef340974c..3ff36f6993 100644 --- a/src/animationprovider/src/Shared/AnimationProvider.lua +++ b/src/animationprovider/src/Shared/AnimationProvider.lua @@ -9,4 +9,4 @@ local require = require(script.Parent.loader).load(script) local TaggedTemplateProvider = require("TaggedTemplateProvider") -return TaggedTemplateProvider.new("AnimationContainer") \ No newline at end of file +return TaggedTemplateProvider.new(script.Name, "AnimationContainer") \ No newline at end of file diff --git a/src/cmdrservice/src/Server/CmdrTemplateProviderServer/init.lua b/src/cmdrservice/src/Server/CmdrTemplateProviderServer/init.lua index c0d45a52d0..41ffccf7c0 100644 --- a/src/cmdrservice/src/Server/CmdrTemplateProviderServer/init.lua +++ b/src/cmdrservice/src/Server/CmdrTemplateProviderServer/init.lua @@ -7,4 +7,4 @@ local require = require(script.Parent.loader).load(script) local TemplateProvider = require("TemplateProvider") -return TemplateProvider.new(script) \ No newline at end of file +return TemplateProvider.new(script.Name, script) \ No newline at end of file diff --git a/src/templateprovider/README.md b/src/templateprovider/README.md index 1844bf5389..f1fc861a4d 100644 --- a/src/templateprovider/README.md +++ b/src/templateprovider/README.md @@ -20,28 +20,12 @@ Base of a template retrieval system npm install @quenty/templateprovider --save ``` -## Usage -Usage is designed to be simple. +## Deferred replication template behavior -### `TemplateProvider.new(container, replicationParent)` +1. We want to defer replication of templates until client requests the template +2. Then we want to send the template to the client via PlayerGui methods +3. We want to decide whether or not to bother doing this -If `replicationParent` is given then contents loaded from the cloud will be replicated to the replicationParent when on the server. - -### `TemplateProvider:Init()` - -Initializes the template provider, downloading components and other things needed - -### `TemplateProvider:Clone(templateName)` - -### `TemplateProvider:Get(templateName)` - -### `TemplateProvider:AddContainer(container)` - -### `TemplateProvider:RemoveContainer(container)` - -### `TemplateProvider:IsAvailable(templateName)` - -### `TemplateProvider:GetAll()` - -### `TemplateProvider:GetContainers()` +This will prevent memory usage of unused templates on the client, which happens with the cars and a variety of other game-systems. +We can't store the stuff initially in cameras or in team-create Roblox won't replicate the stuff. But we can move on run-time and hope... \ No newline at end of file diff --git a/src/templateprovider/package.json b/src/templateprovider/package.json index 4d12f01fe4..8ced09f23f 100644 --- a/src/templateprovider/package.json +++ b/src/templateprovider/package.json @@ -26,10 +26,17 @@ ], "dependencies": { "@quenty/baseobject": "file:../baseobject", + "@quenty/brio": "file:../brio", + "@quenty/collectionserviceutils": "file:../collectionserviceutils", + "@quenty/ducktype": "file:../ducktype", "@quenty/insertserviceutils": "file:../insertserviceutils", + "@quenty/instanceutils": "file:../instanceutils", "@quenty/loader": "file:../loader", "@quenty/maid": "file:../maid", + "@quenty/observablecollection": "file:../observablecollection", "@quenty/promise": "file:../promise", + "@quenty/promisemaid": "file:../promisemaid", + "@quenty/rx": "file:../rx", "@quenty/string": "file:../string" }, "publishConfig": { diff --git a/src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModes.lua b/src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModes.lua new file mode 100644 index 0000000000..d8abe04d45 --- /dev/null +++ b/src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModes.lua @@ -0,0 +1,13 @@ +--[=[ + @class TemplateReplicationModes +]=] + +local require = require(script.Parent.loader).load(script) + +local Table = require("Table") + +return Table.readonly({ + CLIENT = "client"; + SERVER = "server"; + SHARED = "shared"; +}) \ No newline at end of file diff --git a/src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModesUtils.lua b/src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModesUtils.lua new file mode 100644 index 0000000000..ae670d5d07 --- /dev/null +++ b/src/templateprovider/src/Shared/Replication/Util/TemplateReplicationModesUtils.lua @@ -0,0 +1,26 @@ +--[=[ + @class TemplateReplicationModesUtils +]=] + +local require = require(script.Parent.loader).load(script) + +local RunService = game:GetService("RunService") +local TemplateReplicationModes = require("TemplateReplicationModes") + +local TemplateReplicationModesUtils = {} + +function TemplateReplicationModesUtils.inferReplicationMode() + if not RunService:IsRunning() then + return TemplateReplicationModes.SHARED + end + + if RunService:IsServer() then + return TemplateReplicationModes.SERVER + elseif RunService:IsClient() then + return TemplateReplicationModes.CLIENT + else + return TemplateReplicationModes.SHARED + end +end + +return TemplateReplicationModesUtils \ No newline at end of file diff --git a/src/templateprovider/src/Shared/TaggedTemplateProvider.lua b/src/templateprovider/src/Shared/TaggedTemplateProvider.lua index 2ec547b1ee..70377a8295 100644 --- a/src/templateprovider/src/Shared/TaggedTemplateProvider.lua +++ b/src/templateprovider/src/Shared/TaggedTemplateProvider.lua @@ -1,59 +1,21 @@ --[=[ Like a template provider, but it also reparents and retrieves tagged objects + @class TaggedTemplateProvider ]=] local require = require(script.Parent.loader).load(script) -local CollectionService = game:GetService("CollectionService") -local RunService = game:GetService("RunService") - local TemplateProvider = require("TemplateProvider") +local RxCollectionServiceUtils = require("RxCollectionServiceUtils") -local TaggedTemplateProvider = setmetatable({}, TemplateProvider) -TaggedTemplateProvider.ClassName = "TaggedTemplateProvider" -TaggedTemplateProvider.__index = TaggedTemplateProvider - -function TaggedTemplateProvider.new(containerTagName) - local self = setmetatable(TemplateProvider.new(), TaggedTemplateProvider) - - assert(type(containerTagName) == "string", "Bad containerTagName") - - -- We prefer a default tag name so test scripts can still read assets for testing - self._tagsToInitializeSet = { [containerTagName] = true } - - return self -end - -function TaggedTemplateProvider:Init() - assert(not self._maid, "Should not have a maid") - - getmetatable(TaggedTemplateProvider).Init(self) - - assert(self._maid, "Should have a maid") - - for tag, _ in pairs(self._tagsToInitializeSet) do - self:AddContainersFromTag(tag) - end -end - -function TaggedTemplateProvider:AddContainersFromTag(containerTagName) - assert(self._maid, "Not initialized") - assert(type(containerTagName) == "string", "Bad containerTagName") - - if RunService:IsRunning() then - self._maid:GiveTask(CollectionService:GetInstanceAddedSignal(containerTagName):Connect(function(inst) - self:AddContainer(inst) - end)) +local TaggedTemplateProvider = {} - self._maid:GiveTask(CollectionService:GetInstanceRemovedSignal(containerTagName):Connect(function(inst) - self:RemoveContainer(inst) - end)) - end +function TaggedTemplateProvider.new(providerName, tagName) + assert(type(providerName) == "string", "bad providerName") + assert(type(tagName) == "string", "Bad tagName") - for _, inst in pairs(CollectionService:GetTagged(containerTagName)) do - self:AddContainer(inst) - end + return TemplateProvider.new(providerName, RxCollectionServiceUtils.observeTaggedBrio(tagName)) end return TaggedTemplateProvider \ No newline at end of file diff --git a/src/templateprovider/src/Shared/TemplateProvider.lua b/src/templateprovider/src/Shared/TemplateProvider.lua index 81352b751b..9c5b4fd6ed 100644 --- a/src/templateprovider/src/Shared/TemplateProvider.lua +++ b/src/templateprovider/src/Shared/TemplateProvider.lua @@ -1,19 +1,27 @@ --[=[ - Base of a template retrieval system. Templates can be retrieved from Roblox, or from the cloud, - and then retrieved by name. Folders are ignored, so assets may be organized however you want. + Base of a template retrieval system. Templates can be retrieved from Roblox and then retrieved by name. If a folder is used + all of their children are also included as templates, which allows for flexible organization by artists. - Templates can repliate to client if desired. + Additionally, you can provide template overrides as the last added template will always be used. ```lua - -- shared/Templates.lua + -- shared/CarTemplates.lua - return TemplateProvider.new(182451181, script) -- Load from Roblox cloud + return TemplateProvider.new(script.Name, script) -- Load locally ``` + :::tip + If the TemplateProvider is initialized on the server, the the templates will be hidden from the client until the + client requests them. + + This prevents large amounts of templates from being rendered to the client, taking up memory on the client. This especially + affects meshes, but can also affect sounds and other similar templates. + ::: + ```lua -- Server local serviceBag = ServiceBag.new() - local templates = serviceBag:GetService(require("Templates")) + local templates = serviceBag:GetService(require("CarTemplates")) serviceBag:Init() serviceBag:Start() ``` @@ -21,12 +29,12 @@ ```lua -- Client local serviceBag = ServiceBag.new() - local templates = serviceBag:GetService(require("Templates")) + local templates = serviceBag:GetService(require("CarTemplates")) serviceBag:Init() serviceBag:Start() - templates:PromiseClone("Crate"):Then(function(crate) - print("Got crate from the cloud!") + templates:PromiseCloneTemplate("CopCar"):Then(function(crate) + print("Got crate!") end) ``` @@ -35,13 +43,27 @@ local require = require(script.Parent.loader).load(script) -local RunService = game:GetService("RunService") -local StarterGui = game:GetService("StarterGui") +local ReplicatedStorage = game:GetService("ReplicatedStorage") +local HttpService = game:GetService("HttpService") +local Brio = require("Brio") +local DuckTypeUtils = require("DuckTypeUtils") local Maid = require("Maid") -local String = require("String") -local InsertServiceUtils = require("InsertServiceUtils") +local Observable = require("Observable") +local ObservableCountingMap = require("ObservableCountingMap") +local ObservableMapList = require("ObservableMapList") local Promise = require("Promise") +local PromiseMaidUtils = require("PromiseMaidUtils") +local Remoting = require("Remoting") +local Rx = require("Rx") +local RxInstanceUtils = require("RxInstanceUtils") +local String = require("String") +local TemplateReplicationModes = require("TemplateReplicationModes") +local TemplateReplicationModesUtils = require("TemplateReplicationModesUtils") + +local TOMBSTONE_ID_ATTRIBUTE = "UnreplicatedTemplateId" +local TOMBSTONE_NAME_POSTFIX_UNLOADED = "_Unloaded" +local TOMBSTONE_NAME_POSTFIX_LOADED = "_Loaded" local TemplateProvider = {} TemplateProvider.ClassName = "TemplateProvider" @@ -49,144 +71,218 @@ TemplateProvider.ServiceName = "TemplateProvider" TemplateProvider.__index = TemplateProvider --[=[ - Constructs a new [TemplateProvider]. - @param container Instance | table | number -- Value - @param replicationParent Instance? -- Place to replicate instances to. + @type Template Instance | Observable> | table + @within TemplateProvider ]=] -function TemplateProvider.new(container, replicationParent) - local self = setmetatable({}, TemplateProvider) - - self._replicationParent = replicationParent - self._containersToInitializeSet = {} - self._containersToInitializeList = {} - - if typeof(container) == "Instance" or type(container) == "number" then - self:_registerContainer(container) - elseif typeof(container) == "table" then - for _, item in pairs(container) do - assert(typeof(item) == "Instance" or type(item) == "number", "Bad item in initialization set") - self:_registerContainer(item) +--[=[ + Constructs a new [TemplateProvider]. - -- For easy debugging/iteration loop - if typeof(item) == "Instance" - and item:IsDescendantOf(StarterGui) - and item:IsA("ScreenGui") - and RunService:IsRunning() then + @param providerName string + @param initialTemplates Template +]=] +function TemplateProvider.new(providerName, initialTemplates) + assert(type(providerName) == "string", "Bad providerName") + local self = setmetatable({}, TemplateProvider) - item.Enabled = false - end - end - end + self.ServiceName = assert(providerName, "No providerName") + self._initialTemplates = initialTemplates - -- Make sure to replicate our parent - if self._replicationParent then - self:_registerContainer(self._replicationParent) + if not (self:_isValidTemplate(self._initialTemplates) or self._initialTemplates == nil) then + error(string.format("[TemplateProvider.%s] - Bad initialTemplates of type %s", self.ServiceName, typeof(initialTemplates))) end return self end -function TemplateProvider:_registerContainer(container) - assert(typeof(container) == "Instance" or type(container) == "number", "Bad container") +--[=[ + Returns if the value is a template provider - if not self._containersToInitializeSet[container] then - self._containersToInitializeSet[container] = true - table.insert(self._containersToInitializeList, container) - end + @param value any + @return boolean +]=] + +function TemplateProvider.isTemplateProvider(value) + return DuckTypeUtils.isImplementation(TemplateProvider, value) end --[=[ Initializes the container provider. Should be done via [ServiceBag]. ]=] -function TemplateProvider:Init() - assert(not self._initialized, "Already initialized") - +function TemplateProvider:Init(serviceBag) + assert(not self._serviceBag, "Already initialized") + self._serviceBag = assert(serviceBag, "No serviceBag") self._maid = Maid.new() - self._initialized = true - self._registry = {} -- [name] = rawTemplate - self._containersSet = {} -- [parentOrAssetId] = true - self._promises = {} -- [name] = Promise + self._replicationMode = TemplateReplicationModesUtils.inferReplicationMode() - for _, container in pairs(self._containersToInitializeList) do - self:AddContainer(container) - end + -- There can be multiple templates for a given name + self._templateMapList = self._maid:Add(ObservableMapList.new()) + self._unreplicatedTemplateMapList = self._maid:Add(ObservableMapList.new()) + + self._containerRootCountingMap = self._maid:Add(ObservableCountingMap.new()) + self._pendingTemplatePromises = {} -- [templateName] = Promise + + self:_setupTemplateCache() end ---[=[ - Promises to clone the template as soon as it exists. - @param templateName string - @return Promise -]=] -function TemplateProvider:PromiseClone(templateName) - assert(type(templateName) == "string", "templateName must be a string") +function TemplateProvider:_setupTemplateCache() + if self._replicationMode == TemplateReplicationModes.SERVER then + self._tombstoneLookup = {} + self._remoting = self._maid:Add(Remoting.Server.new(ReplicatedStorage, self.ServiceName .. "TemplateProvider")) + + -- TODO: Maybe de-duplicate and use a centralized service + self._maid:GiveTask(self._remoting.ReplicateTemplate:Bind(function(player, tombstoneId) + assert(type(tombstoneId) == "string", "Bad tombstoneId") + assert(self._tombstoneLookup[tombstoneId], "Not a valid tombstone") + + -- Stuff doesn't replicate in the PlayerGui + local playerGui = player:FindFirstChildWhichIsA("PlayerGui") + if not playerGui then + return Promise.rejected("No playerGui") + end + + -- Just group stuff to simplify things + local replicationParent = playerGui:FindFirstChild("TemplateProviderReplication") + if not replicationParent then + replicationParent = Instance.new("Folder") + replicationParent.Name = "TemplateProviderReplication" + replicationParent.Archivable = false + replicationParent.Parent = playerGui + end + + local copy = self._tombstoneLookup[tombstoneId]:Clone() + copy.Parent = playerGui + + task.delay(0.1, function() + copy:Remove() + end) + + return copy + end)) + elseif self._replicationMode == TemplateReplicationModes.CLIENT then + self._pendingTombstoneRequests = {} - self:_verifyInit() + self._remoting = self._maid:Add(Remoting.Client.new(ReplicatedStorage, self.ServiceName .. "TemplateProvider")) + end - local template = self._registry[templateName] - if template then - return Promise.resolved(self:Clone(templateName)) + if self._initialTemplates then + self._maid:GiveTask(self:AddTemplates(self._initialTemplates)) end - if not self._promises[templateName] then - local promise = Promise.new() - self._promises[templateName] = promise + -- Recursively adds roots, but also de-duplicates them as necessary + self._maid:GiveTask(self._containerRootCountingMap:ObserveKeysBrio():Subscribe(function(containerBrio) + if containerBrio:IsDead() then + return + end - -- Make sure to clean up the promise afterwards - self._maid[promise] = promise - promise:Then(function() - self._maid[promise] = nil - end) + local containerMaid, container = containerBrio:ToMaidAndValue() + self:_handleContainer(containerMaid, container) + end)) +end + +function TemplateProvider:_handleContainer(containerMaid, container) + if self._replicationMode == TemplateReplicationModes.SERVER + and not container:IsA("Camera") + and not container:FindFirstAncestorWhichIsA("Camera") then + -- Prevent replication to client immediately + + local camera = containerMaid:Add(Instance.new("Camera")) + camera.Name = "PreventReplication" + camera.Parent = container - task.delay(5, function() - if promise:IsPending() then - warn(string.format("[TemplateProvider.PromiseClone] - May fail to replicate template %q from cloud. %s", templateName, self:_getReplicationHint())) + local function handleChild(child) + if child == camera then + return end - end) + if child:GetAttribute(TOMBSTONE_ID_ATTRIBUTE) then + return + end + + child.Parent = camera + end + + containerMaid:GiveTask(container.ChildAdded:Connect(handleChild)) + + for _, child in pairs(container:GetChildren()) do + handleChild(child) + end + + self:_replicateTombstones(containerMaid, camera, container) + + return end - return self._promises[templateName] - :Then(function() - -- Get a new copy - return self:Clone(templateName) - end) + containerMaid:GiveTask(RxInstanceUtils.observeChildrenBrio(container):Subscribe(function(brio) + if brio:IsDead() then + return + end + + local maid, child = brio:ToMaidAndValue() + self:_addInstanceTemplate(maid, child) + end)) end -function TemplateProvider:_getReplicationHint() - local hint = "" +function TemplateProvider:_replicateTombstones(topMaid, unreplicatedParent, replicatedParent) + assert(self._replicationMode == TemplateReplicationModes.SERVER, "Only should be invoked on server") - if RunService:IsClient() then - hint = "Make sure the template provider is initialized on the server." - end + -- Tombstone each child so the client knows what is replicated + topMaid:GiveTask(RxInstanceUtils.observeChildrenBrio(unreplicatedParent):Subscribe(function(brio) + if brio:IsDead() then + return + end + + local maid, child = brio:ToMaidAndValue() + self:_addInstanceTemplate(maid, child) + + local tombstoneId = HttpService:GenerateGUID(false) + + -- Tell the client something exists here + local tombstone = maid:Add(Instance.new("Folder")) + tombstone.Name = child.Name .. TOMBSTONE_NAME_POSTFIX_UNLOADED + tombstone:SetAttribute(TOMBSTONE_ID_ATTRIBUTE, tombstoneId) + + -- Recursively replicate other tombstones + if self:_shouldAddChildrenAsTemplates(child) then + self:_replicateTombstones(maid, child, tombstone) + end - return hint + self._tombstoneLookup[tombstoneId] = child + + maid:GiveTask(function() + self._tombstoneLookup[tombstoneId] = nil + end) + + tombstone.Parent = replicatedParent + end)) end --[=[ - Clones the template. - - :::info - If the template name has a prefix of "Template" then it will remove it on the cloned instance. - ::: + Observes the given template by name @param templateName string - @return Instance? + @return Observable ]=] -function TemplateProvider:Clone(templateName) +function TemplateProvider:ObserveTemplate(templateName) assert(type(templateName) == "string", "templateName must be a string") - self:_verifyInit() + return self._templateMapList:ObserveList(templateName):Pipe({ + Rx.switchMap(function(list) + if not list then + return Rx.of(nil); + end - local template = self._registry[templateName] - if not template then - error(string.format("[TemplateProvider.Clone] - Cannot provide %q", tostring(templateName))) - return nil - end + return list:ObserveAtIndex(-1) + end); + }) +end - local newItem = template:Clone() - newItem.Name = String.removePostfix(templateName, "Template") - return newItem +function TemplateProvider:ObserveTemplateNamesBrio() + return self._templateMapList:ObserveKeysBrio() +end + +function TemplateProvider:ObserveUnreplicatedTemplateNamesBrio() + return self._unreplicatedTemplateMapList:ObserveKeysBrio() end --[=[ @@ -195,197 +291,330 @@ end @param templateName string @return Instance? ]=] -function TemplateProvider:Get(templateName) +function TemplateProvider:GetTemplate(templateName) assert(type(templateName) == "string", "templateName must be a string") - self:_verifyInit() - - return self._registry[templateName] -end ---[=[ - Adds a new container to the provider for provision of assets. - - @param container Instance | number -]=] -function TemplateProvider:AddContainer(container) - assert(typeof(container) == "Instance" or type(container) == "number", "Bad container") - self:_verifyInit() - - if not self._containersSet[container] then - self._containersSet[container] = true - if type(container) == "number" then - self._maid[container] = self:_loadCloudAsset(container) - elseif typeof(container) == "Instance" then - self._maid[container] = self:_loadFolder(container) - else - error("Unknown container type to load") - end - end + return self._templateMapList:GetItemForKeyAtIndex(templateName, -1) end --[=[ - Removes a container from the provisioning set. + Promises to clone the template as soon as it exists - @param container Instance | number + @param templateName string + @return Promise ]=] -function TemplateProvider:RemoveContainer(container) - assert(typeof(container) == "Instance", "Bad container") - self:_verifyInit() +function TemplateProvider:PromiseCloneTemplate(templateName) + assert(type(templateName) == "string", "templateName must be a string") - self._containersSet[container] = nil - self._maid[container] = nil + return self:PromiseTemplate(templateName) + :Then(function(template) + return self:_cloneTemplate(template) + end) end --[=[ - Returns whether or not a template is registered at the time + Promise to resolve the raw template as soon as it exists + @param templateName string - @return boolean + @return Promise ]=] -function TemplateProvider:IsAvailable(templateName) +function TemplateProvider:PromiseTemplate(templateName) assert(type(templateName) == "string", "templateName must be a string") - self:_verifyInit() - return self._registry[templateName] ~= nil + local foundTemplate = self._templateMapList:GetItemForKeyAtIndex(templateName, -1) + if foundTemplate then + return Promise.resolved(foundTemplate) + end + + if self._pendingTemplatePromises[templateName] then + return self._pendingTemplatePromises[templateName] + end + + local promiseTemplate = Promise.new() + + -- Observe thet template + PromiseMaidUtils.whilePromise(promiseTemplate, function(topMaid) + topMaid:GiveTask(self:ObserveTemplate(templateName):Subscribe(function(template) + if template then + promiseTemplate:Resolve(template) + end + end)) + + if self._replicationMode == TemplateReplicationModes.SERVER then + -- There's a chance an external process will stream in our template + + topMaid:GiveTask(task.delay(5, function() + warn(string.format("[TemplateProvider.%s.PromiseTemplate] - Missing template %q", self.ServiceName, templateName)) + end)) + elseif self._replicationMode == TemplateReplicationModes.CLIENT then + -- Replicate from the unfound area + topMaid:GiveTask(self._unreplicatedTemplateMapList:ObserveAtListIndexBrio(templateName, -1):Subscribe(function(brio) + if brio:IsDead() then + return + end + + local maid, templateTombstone = brio:ToMaidAndValue() + + local originalName = templateTombstone.Name + + maid:GivePromise(self:_promiseReplicateTemplateFromTombstone(templateTombstone)) + :Then(function(template) + -- Cache the template here which then loads it into the known templates naturally + templateTombstone.Name = String.removePostfix(originalName, TOMBSTONE_NAME_POSTFIX_UNLOADED) .. TOMBSTONE_NAME_POSTFIX_LOADED + template.Parent = templateTombstone + + promiseTemplate:Resolve(template) + end) + end)) + + topMaid:GiveTask(task.delay(5, function() + if self._unreplicatedTemplateMapList:GetListForKey(templateName) then + warn(string.format("[TemplateProvider.%s.PromiseTemplate] - Failed to replicate template %q from server to client", self.ServiceName, templateName)) + else + warn(string.format("[TemplateProvider.%s.PromiseTemplate] - Template %q is not a known template", self.ServiceName, templateName)) + end + end)) + elseif self._replicationMode == TemplateReplicationModes.SHARED then + -- There's a chance an external process will stream in our template + + topMaid:GiveTask(task.delay(5, function() + warn(string.format("[TemplateProvider.%s.PromiseTemplate] - Missing template %q", self.ServiceName, templateName)) + end)) + else + error("Bad replicationMode") + end + end) + + self._maid[promiseTemplate] = promiseTemplate + self._pendingTemplatePromises[templateName] = promiseTemplate + + promiseTemplate:Finally(function() + self._maid[promiseTemplate] = nil + self._pendingTemplatePromises[templateName] = nil + end) + + return promiseTemplate end ---[=[ - Returns all current registered items. +function TemplateProvider:_promiseReplicateTemplateFromTombstone(templateTombstone) + assert(self._replicationMode == TemplateReplicationModes.CLIENT, "Bad replicationMode") + assert(typeof(templateTombstone) == "Instance", "Bad templateTombstone") - @return { Instance } -]=] -function TemplateProvider:GetAll() - self:_verifyInit() + local tombstoneId = templateTombstone:GetAttribute(TOMBSTONE_ID_ATTRIBUTE) + if type(tombstoneId) ~= "string" then + return Promise.rejected("tombstoneId must be a string") + end - local list = {} - for _, item in pairs(self._registry) do - table.insert(list, item) + if self._pendingTombstoneRequests[tombstoneId] then + return self._pendingTombstoneRequests[tombstoneId] end - return list + local promiseTemplate = Promise.new() + + PromiseMaidUtils.whilePromise(promiseTemplate, function(topMaid) + topMaid:GivePromise(self._remoting.ReplicateTemplate:PromiseInvokeServer(tombstoneId)) + :Then(function(tempTemplate) + if not tempTemplate then + return Promise.rejected("Failed to get any template") + end + + -- This tempTemplate will get destroyed by the server soon to free up server memory + -- TODO: cache on client + local copy = tempTemplate:Clone() + promiseTemplate:Resolve(copy) + end, function(...) + promiseTemplate:Reject(...) + end) + end) + + self._maid[promiseTemplate] = promiseTemplate + self._pendingTombstoneRequests[tombstoneId] = promiseTemplate + + promiseTemplate:Finally(function() + self._maid[promiseTemplate] = nil + self._pendingTombstoneRequests[tombstoneId] = nil + end) + + return promiseTemplate end --[=[ - Gets all current the containers. + Clones the template. + + :::info + If the template name has a prefix of "Template" then it will remove it on the cloned instance. + ::: - @return { Instance | number } + @param templateName string + @return Instance? ]=] -function TemplateProvider:GetContainers() - self:_verifyInit() +function TemplateProvider:CloneTemplate(templateName) + assert(type(templateName) == "string", "templateName must be a string") - local list = {} - for parent, _ in pairs(self._containersSet) do - table.insert(list, parent) - end - return list -end + local template = self._templateMapList:GetItemForKeyAtIndex(templateName, -1) + if not template then + local unreplicated = self._unreplicatedTemplateMapList:GetListForKey(templateName) -function TemplateProvider:_verifyInit() - if not RunService:IsRunning() then - if not self._initialized then - -- Initialize for hoarcecat! - self:Init() + if unreplicated then + error(string.format("[TemplateProvider.%s.CloneTemplate] - Template %q is not replicated. Use PromiseCloneTemplate instead", self.ServiceName, tostring(templateName))) + else + error(string.format("[TemplateProvider.%s.CloneTemplate] - Cannot provide template %q", self.ServiceName, tostring(templateName))) end + + return nil end - assert(self._initialized, "TemplateProvider is not initialized") + return self:_cloneTemplate(template) end -function TemplateProvider:_loadCloudAsset(assetId) - assert(type(assetId) == "number", "Bad assetId") - local maid = Maid.new() - - -- Load on server - if RunService:IsServer() or not RunService:IsRunning() then - maid:GivePromise(InsertServiceUtils.promiseAsset(assetId)):Then(function(result) - if RunService:IsRunning() then - for _, item in pairs(result:GetChildren()) do - -- Replicate in children - item.Parent = self._replicationParent - end +--[=[ + Adds a new container to the provider for provision of assets. The initial container + is considered a template. Additionally, we will include any children that are in a folder + as a potential root + + :::tip + The last template with a given name added will be considered the canonical template. + ::: + + @param container Template + @return MaidTask +]=] +function TemplateProvider:AddTemplates(container) + assert(self:_isValidTemplate(container), "Bad container") + + if typeof(container) == "Instance" then + -- Always add this instance as we explicitly asked for it to be added as a root. This could be a + -- module script, or other component. + return self._containerRootCountingMap:Add(container) + elseif Observable.isObservable(container) then + local topMaid = Maid.new() + + self:_addObservableTemplates(topMaid, container) + + self._maid[topMaid] = topMaid + topMaid:GiveTask(function() + self._maid[topMaid] = nil + end) + + return topMaid + elseif type(container) == "table" then + local topMaid = Maid.new() + + for _, value in pairs(container) do + if typeof(value) == "Instance" then + -- Always add these as we explicitly ask for this to be a root too. + topMaid:GiveTask(self._containerRootCountingMap:Add(value)) + elseif Observable.isObservable(value) then + self:_addObservableTemplates(topMaid, value) else - -- Load without parenting - maid:GiveTask(self:_loadFolder(result)) + error(string.format("[TemplateProvider.%s] - Bad value of type %q in container table", self.ServiceName, typeof(value))) end - end) - end + end - return maid -end + self._maid[topMaid] = topMaid + topMaid:GiveTask(function() + self._maid[topMaid] = nil + end) -function TemplateProvider:_transformParent(getParent) - if typeof(getParent) == "Instance" then - return getParent - elseif type(getParent) == "function" then - local container = getParent() - assert(typeof(container) == "Instance", "Bad container") - return container + return topMaid else - error("Bad getParent type") + error(string.format("[TemplateProvider.%s] - Bad container of type %s", self.ServiceName, typeof(container))) end end -function TemplateProvider:_loadFolder(parent) - local maid = Maid.new() +function TemplateProvider:_addObservableTemplates(topMaid, observable) + topMaid:GiveTask(observable:Subscribe(function(result) + if Brio.isBrio(result) then + if result:IsDead() then + return + end - -- Only connect events if we're running - if RunService:IsRunning() then - maid:GiveTask(parent.ChildAdded:Connect(function(child) - self:_handleChildAdded(maid, child) - end)) - maid:GiveTask(parent.ChildRemoved:Connect(function(child) - self:_handleChildRemoved(maid, child) - end)) + local maid, template = result:ToMaidAndValue() + if typeof(template) == "Instance" then + self:_addInstanceTemplate(maid, template) + else + error("Cannot add non-instance from observable template") + end + else + error("Cannot add non Brio from observable") + end + end)) +end + +function TemplateProvider:_addInstanceTemplate(topMaid, template) + if self:_shouldAddChildrenAsTemplates(template) then + topMaid:GiveTask(self._containerRootCountingMap:Add(template)) end - for _, child in pairs(parent:GetChildren()) do - self:_handleChildAdded(maid, child) + if template:GetAttribute(TOMBSTONE_ID_ATTRIBUTE) then + topMaid:GiveTask(self._unreplicatedTemplateMapList:Push(RxInstanceUtils.observeProperty(template, "Name"):Pipe({ + Rx.map(function(name) + if String.endsWith(name, TOMBSTONE_NAME_POSTFIX_UNLOADED) then + return String.removePostfix(name, TOMBSTONE_NAME_POSTFIX_UNLOADED) + elseif String.endsWith(name, TOMBSTONE_NAME_POSTFIX_LOADED) then + return String.removePostfix(name, TOMBSTONE_NAME_POSTFIX_LOADED) + else + return name + end + end); + Rx.distinct(); + }), template)) + else + topMaid:GiveTask(self._templateMapList:Push(RxInstanceUtils.observeProperty(template, "Name"), template)) end +end - maid:GiveTask(function() - maid:DoCleaning() +--[=[ + Returns whether or not a template is registered at the time - -- Deregister children - for _, child in pairs(parent:GetChildren()) do - self:_handleChildRemoved(maid, child) - end - end) + @param templateName string + @return boolean +]=] +function TemplateProvider:IsTemplateAvailable(templateName) + assert(type(templateName) == "string", "templateName must be a string") - return maid + return self._templateMapList:GetItemForKeyAtIndex(templateName, -1) ~= nil end -function TemplateProvider:_handleChildRemoved(maid, child) - maid[child] = nil - self:_removeFromRegistry(child) -end +--[=[ + Returns all current registered items. -function TemplateProvider:_handleChildAdded(maid, child) - if child:IsA("Folder") then - maid[child] = self:_loadFolder(child) - else - self:_addToRegistery(child) - end + @return { Instance } +]=] +function TemplateProvider:GetTemplateList() + return self._templateMapList:GetListOfValuesAtListIndex(-1) end -function TemplateProvider:_addToRegistery(child) - local childName = child.Name - -- if self._registry[childName] then - -- warn(string.format("[TemplateProvider._addToRegistery] - Duplicate %q in registery. Overridding", childName)) - -- end +--[=[ + Gets all current the containers. + + @return { Instance } +]=] +function TemplateProvider:GetContainerList() + return self._containerRootCountingMap:GetList() +end - self._registry[childName] = child +-- Backwards compatibility +TemplateProvider.IsAvailable = assert(TemplateProvider.IsTemplateAvailable, "Missing method") +TemplateProvider.Get = assert(TemplateProvider.GetTemplate, "Missing method") +TemplateProvider.Clone = assert(TemplateProvider.CloneTemplate, "Missing method") +TemplateProvider.PromiseClone = assert(TemplateProvider.PromiseCloneTemplate, "Missing method") +TemplateProvider.GetAllTemplates = assert(TemplateProvider.GetTemplateList, "Missing method") - if self._promises[childName] then - self._promises[childName]:Resolve(child) - self._promises[childName] = nil - end +function TemplateProvider:_cloneTemplate(template) + local newItem = template:Clone() + newItem.Name = String.removePostfix(template.Name, "Template") + return newItem end -function TemplateProvider:_removeFromRegistry(child) - local childName = child.Name +function TemplateProvider:_shouldAddChildrenAsTemplates(container) + return container:IsA("Folder") +end - if self._registry[childName] == child then - self._registry[childName] = nil - end +function TemplateProvider:_isValidTemplate(container) + return typeof(container) == "Instance" + or Observable.isObservable(container) + or type(container) == "table" end --[=[ From ab240099ad24b5f0709b6ea5bfe766d84f7acf3a Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 18:57:15 -0700 Subject: [PATCH 12/15] feat: Freeze Table.readonly and avoid extra work in Table.take --- src/table/src/Shared/Table.lua | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/table/src/Shared/Table.lua b/src/table/src/Shared/Table.lua index 62aa194276..b12a9a0b79 100644 --- a/src/table/src/Shared/Table.lua +++ b/src/table/src/Shared/Table.lua @@ -309,10 +309,13 @@ end @return table -- List with the entries retrieved ]=] function Table.take(source, count) - local newTable = {} - for i=1, math.min(#source, count) do + local n = math.min(#source, count) + local newTable = table.create(n) + + for i=1, n do newTable[i] = source[i] end + return newTable end @@ -333,7 +336,7 @@ local READ_ONLY_METATABLE = { @return table -- The same table, with the metatable set to readonly ]=] function Table.readonly(target) - return setmetatable(target, READ_ONLY_METATABLE) + return table.freeze(setmetatable(target, READ_ONLY_METATABLE)) end --[=[ From e546588d97b2e8f8386d9795f04929c9b45a27d1 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 18:57:24 -0700 Subject: [PATCH 13/15] fix: Brios sometime cleanup wrongly --- .../src/Shared/ObservableCountingMap.lua | 26 ++++++++++++------- .../src/Shared/ObservableSet.lua | 12 ++++++--- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/observablecollection/src/Shared/ObservableCountingMap.lua b/src/observablecollection/src/Shared/ObservableCountingMap.lua index 94d4305834..9717acd1ba 100644 --- a/src/observablecollection/src/Shared/ObservableCountingMap.lua +++ b/src/observablecollection/src/Shared/ObservableCountingMap.lua @@ -79,7 +79,7 @@ end ]=] function ObservableCountingMap:ObserveKeysList() return self:_observeDerivedDataStructureFromKeys(function() - local list = {} + local list = table.create(self._totalKeyCountValue.Value) for key, _ in pairs(self._map) do table.insert(list, key) @@ -184,22 +184,28 @@ end function ObservableCountingMap:ObserveKeysBrio() return Observable.new(function(sub) local maid = Maid.new() + local keyMaid = maid:Add(Maid.new()) local function handleItem(key) + -- Happens upon key added re-entrance + if keyMaid[key] then + return + end + local brio = Brio.new(key) - maid[key] = brio + keyMaid[key] = brio sub:Fire(brio) end - for key, _ in pairs(self._map) do - handleItem(key) - end - maid:GiveTask(self.KeyAdded:Connect(handleItem)) maid:GiveTask(self.KeyRemoved:Connect(function(key) - maid[key] = nil + keyMaid[key] = nil end)) + for key, _ in pairs(self._map) do + handleItem(key) + end + self._maid[sub] = maid maid:GiveTask(function() self._maid[sub] = nil @@ -286,8 +292,10 @@ function ObservableCountingMap:Add(key, amount) return end - if self._map[key] then - local newValue = self._map[key] + amount + local oldValue = self._map[key] + + if oldValue then + local newValue = oldValue + amount if newValue == 0 then -- Remove item self._map[key] = nil diff --git a/src/observablecollection/src/Shared/ObservableSet.lua b/src/observablecollection/src/Shared/ObservableSet.lua index db545497df..227bc25236 100644 --- a/src/observablecollection/src/Shared/ObservableSet.lua +++ b/src/observablecollection/src/Shared/ObservableSet.lua @@ -78,20 +78,26 @@ function ObservableSet:ObserveItemsBrio() local maid = Maid.new() local function handleItem(item) + if maid[item] then + -- Happens when we're re-entrance + return + end + local brio = Brio.new(item) maid[item] = brio sub:Fire(brio) end - for item, _ in pairs(self._set) do - handleItem(item) - end maid:GiveTask(self.ItemAdded:Connect(handleItem)) maid:GiveTask(self.ItemRemoved:Connect(function(item) maid[item] = nil end)) + for item, _ in pairs(self._set) do + handleItem(item) + end + self._maid[sub] = maid maid:GiveTask(function() self._maid[sub] = nil From 3c7f020c61a0c378ef84f8a49dcd7f070ec2c6c5 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 18:57:38 -0700 Subject: [PATCH 14/15] fix: Replicator replicates attributes and tags properly --- src/loader/src/Replication/Replicator.lua | 41 ++++++++++++++++++++++- src/loader/src/init.lua | 6 +++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/loader/src/Replication/Replicator.lua b/src/loader/src/Replication/Replicator.lua index 3c827c5a17..43a160525c 100644 --- a/src/loader/src/Replication/Replicator.lua +++ b/src/loader/src/Replication/Replicator.lua @@ -307,6 +307,7 @@ function Replicator:_doStandardReplication(maid, replicator, child, copy) assert(typeof(child) == "Instance", "Bad child") self:_setupAttributeReplication(maid, child, copy) + self:_setupTagReplication(maid, child, copy) self:_setupNameReplication(maid, child, copy) self:_setupParentReplication(maid, copy) self:_setupReference(maid, child, copy) @@ -467,6 +468,44 @@ function Replicator:_setupParentReplication(maid, copy) copy.Parent = self._target.Value end +--[[ + Sets up tag replication explicitly. + + @param maid Maid + @param child Instance + @param copy Instance +]] +function Replicator:_setupTagReplication(maid, child, copy) + assert(Maid.isMaid(maid), "Bad maid") + assert(typeof(child) == "Instance", "Bad child") + assert(typeof(copy) == "Instance", "Bad copy") + + for _, tag in pairs(child:GetTags()) do + copy:AddTag(tag) + end + + maid:GiveTask(child.Changed:Connect(function(property) + if property == "Tags" then + local ourTagSet = {} + for _, tag in pairs(copy:GetTags()) do + ourTagSet[tag] = true + end + + for _, tag in pairs(child:GetTags()) do + if not ourTagSet[tag] then + copy:AddTag(tag) + end + + ourTagSet[tag] = nil + end + + for tag, _ in pairs(ourTagSet) do + copy:RemoveTag(tag) + end + end + end)) +end + --[[ Sets up the object value replication to point towards new values. @@ -528,7 +567,7 @@ end function Replicator:_setupAttributeReplication(maid, child, copy) for key, value in pairs(child:GetAttributes()) do - child:SetAttribute(key, value) + copy:SetAttribute(key, value) end maid:GiveTask(child.AttributeChanged:Connect(function(attribute) diff --git a/src/loader/src/init.lua b/src/loader/src/init.lua index 465c3adf90..b1ede487b3 100644 --- a/src/loader/src/init.lua +++ b/src/loader/src/init.lua @@ -148,7 +148,11 @@ function Loader:_findDependency(request) -- Just standard dependency search local foundBackup = DependencyUtils.findDependency(self._packages, request, self._replicationType) if foundBackup then - warn(string.format("[Loader] - Failed to find package %q in package tracker of root %s\n%s", request, self._packages:GetFullName(), debug.traceback())) + if packageTracker then + warn(string.format("[Loader] - No package tracker for root %s (while loading %s)\n%s", self._packages:GetFullName(), request, debug.traceback())) + else + warn(string.format("[Loader] - Failed to find package %q in package tracker of root %s\n%s", request, self._packages:GetFullName(), debug.traceback())) + end -- Ensure hoarcekat story has a link to use -- TODO: Maybe add to global package cache instead... From fac3b1a72adf9f483e8b989a863aa82c24e10480 Mon Sep 17 00:00:00 2001 From: James Onnen Date: Sat, 5 Oct 2024 19:27:12 -0700 Subject: [PATCH 15/15] docs: Fix documentation --- src/templateprovider/src/Shared/TemplateProvider.lua | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/templateprovider/src/Shared/TemplateProvider.lua b/src/templateprovider/src/Shared/TemplateProvider.lua index 9c5b4fd6ed..096d23652c 100644 --- a/src/templateprovider/src/Shared/TemplateProvider.lua +++ b/src/templateprovider/src/Shared/TemplateProvider.lua @@ -108,6 +108,8 @@ end --[=[ Initializes the container provider. Should be done via [ServiceBag]. + + @param serviceBag ServiceBag ]=] function TemplateProvider:Init(serviceBag) assert(not self._serviceBag, "Already initialized")