-
Notifications
You must be signed in to change notification settings - Fork 182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use GetObjects as fallback for instances that can't be created normally #401
Closed
Closed
Changes from 2 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
5224644
Tests for mocked tryGetObjects
Kampfkarren 002a264
Preliminary GetObjects support
Kampfkarren b80422a
Use latest Flipper
Kampfkarren bd62509
Tabs -> spaces
Kampfkarren ced507a
I don't think this is necessary, given the current ocode
Kampfkarren 1208c79
Condense UUID
Kampfkarren e42675f
Use real fs
Kampfkarren 1be54f4
Fix clippy lint
Kampfkarren a5917fd
itFOCUS -> it
Kampfkarren 69a240b
Don't GetObjects when properties don't update (fixes a crash)
Kampfkarren 2cc7478
Remove inaccurate TODO
Kampfkarren 8089cbd
Use XML instead of binary
Kampfkarren df9139e
I don't know why this is a TODO
Kampfkarren e05bdbb
Use multi-rooted instance
Kampfkarren 2ed4750
Update test (even though the other tests don't work)
Kampfkarren ac55378
pcall Parent setting
Kampfkarren 5f207d2
Remove unused Fmt
Kampfkarren abcf5f4
Trying to get created to occur by updating project file
Kampfkarren acf39c9
Support added semantically
Kampfkarren dbca0a1
Remove isDevBuild
Kampfkarren 6222064
it -> itFOCUS
Kampfkarren 87fe427
Remove TODO that isn't possible
Kampfkarren File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Submodule flipper
updated
18 files
+7 −8 | .github/workflows/build.yml | |
+23 −0 | .github/workflows/ci.yml | |
+3 −3 | .gitignore | |
+2 −2 | foreman.toml | |
+1 −4 | selene.toml | |
+8 −3 | src/BaseMotor.lua | |
+2 −2 | src/BaseMotor.spec.lua | |
+2 −1 | src/GroupMotor.lua | |
+27 −4 | src/GroupMotor.spec.lua | |
+2 −2 | src/Instant.lua | |
+2 −2 | src/Linear.lua | |
+2 −2 | src/Signal.lua | |
+4 −2 | src/SingleMotor.lua | |
+19 −2 | src/SingleMotor.spec.lua | |
+2 −2 | src/Spring.lua | |
+1 −1 | src/init.lua | |
+3 −3 | test/demo.client.lua | |
+9 −3 | typings/BaseMotor.d.ts |
Submodule promise
updated
4 files
+7 −2 | README.md | |
+174 −58 | lib/init.lua | |
+211 −50 | lib/init.spec.lua | |
+7 −0 | rotriever.toml |
Submodule roact
updated
17 files
+8 −8 | .github/workflows/ci.yml | |
+21 −0 | .github/workflows/clabot.yml | |
+2 −2 | .github/workflows/deploy-docs.yml | |
+0 −32 | .travis.yml | |
+6 −0 | CHANGELOG.md | |
+3 −9 | README.md | |
+1 −4 | docs/advanced/context.md | |
+1 −4 | docs/api-reference.md | |
+1 −1 | docs/extra.css | |
+2 −2 | docs/guide/hello-roact.md | |
+3 −3 | docs/guide/installation.md | |
+2 −1 | mkdocs.yml | |
+1 −1 | rotriever.toml | |
+2 −1 | src/Component.lua | |
+29 −0 | src/Component.spec/validateProps.spec.lua | |
+83 −33 | src/createContext.lua | |
+185 −3 | src/createContext.spec.lua |
Submodule t
updated
18 files
+14 −0 | .github/workflows/dependabot.yml | |
+47 −0 | .github/workflows/main.yml | |
+1 −0 | .gitignore | |
+1 −1 | .luacheckrc | |
+0 −29 | .travis.yml | |
+48 −14 | README.md | |
+6 −0 | default.project.json | |
+271 −28 | lib/init.lua | |
+380 −161 | lib/init.spec.lua | |
+191 −0 | lib/t.d.ts | |
+604 −0 | lib/ts.lua | |
+1 −1 | modules/lemur | |
+1 −1 | modules/testez | |
+13 −0 | package-lock.json | |
+25 −0 | package.json | |
+3 −2 | rotriever.toml | |
+10 −7 | spec.lua | |
+16 −0 | tsconfig.json |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
local Fmt = require(script.Parent.Parent.Fmt) | ||
local Log = require(script.Parent.Parent.Log) | ||
local PatchSet = require(script.Parent.PatchSet) | ||
local Promise = require(script.Parent.Parent.Promise) | ||
|
||
local function tryGetObjects(instanceMap, apiContext, patch) | ||
assert(PatchSet.validate(patch)) | ||
|
||
local unappliedPatch = PatchSet.newEmpty() | ||
|
||
-- GetObjects won't help with anything that failed to remove | ||
unappliedPatch.removed = patch.removed | ||
-- TODO: Implement this? Do we have to? | ||
unappliedPatch.added = patch.added | ||
|
||
local assetsToRequest = {} | ||
local receiveCallbacks = {} | ||
|
||
Log.trace("tryGetObjects({:#?})", patch) | ||
|
||
-- TODO: added? | ||
|
||
-- GetObjects only create instances, we can't update the properties of existing ones. | ||
-- Instead, just create them again, move their children, and replace the instance. | ||
for _, update in ipairs(patch.updated) do | ||
table.insert(assetsToRequest, update.id) | ||
table.insert(unappliedPatch.updated, update) | ||
|
||
receiveCallbacks[update.id] = function(newInstance) | ||
table.remove(unappliedPatch.updated, table.find(unappliedPatch.updated, update)) | ||
|
||
local oldInstance = instanceMap.fromIds[update.id] | ||
|
||
-- TODO: What if oldInstance is nil? | ||
Kampfkarren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
-- TODO: This will dupe children if the rbxm is not empty. | ||
Kampfkarren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for _, oldChild in ipairs(oldInstance:GetChildren()) do | ||
oldChild.Parent = newInstance | ||
end | ||
|
||
local oldParent = oldInstance.Parent | ||
instanceMap:destroyInstance(oldInstance) | ||
newInstance.Parent = oldParent | ||
Kampfkarren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
end | ||
|
||
if #assetsToRequest == 0 then | ||
return Promise.resolve(unappliedPatch) | ||
end | ||
|
||
return apiContext:createAssets(assetsToRequest):andThen(function(assetId) | ||
--[[ | ||
The assets Rojo creates that we will be loading is in the following structure: | ||
|
||
Assuming we requested the following IDs: | ||
- DOGE: A doge mesh named Doge | ||
- ROCK: A rock mesh named MyPet | ||
|
||
Root: Folder | ||
* DOGE: Folder | ||
* Doge: MeshPart | ||
* ROCK: Folder | ||
* MyPet: MeshPart | ||
]] | ||
Kampfkarren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
-- GetObjects doesn't yield, it blocks. | ||
-- There is no way to promisify this unless GetObjectsAsync is opened up. | ||
local createdAssets = game:GetObjects(assetId)[1] | ||
|
||
if createdAssets == nil then | ||
Log.warn("Request to create assets returned an asset that was empty.") | ||
return unappliedPatch | ||
end | ||
|
||
for _, assetFolder in ipairs(createdAssets:GetChildren()) do | ||
local requestedId = assetFolder.Name | ||
|
||
-- TODO: Does this need to support multi-rooted instances? Probably not? | ||
local assetInstance = assetFolder:GetChildren()[1] | ||
|
||
receiveCallbacks[requestedId](assetInstance) | ||
instanceMap:insert(requestedId, assetInstance) | ||
end | ||
|
||
return unappliedPatch | ||
end) | ||
end | ||
|
||
return tryGetObjects |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
-- TODO: Have some documentation/script in order to force the rbxassets to be there | ||
Kampfkarren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return function() | ||
local InstanceMap = require(script.Parent.InstanceMap) | ||
local PatchSet = require(script.Parent.PatchSet) | ||
local Promise = require(script.Parent.Parent.Promise) | ||
local tryGetObjects = require(script.Parent.tryGetObjects) | ||
|
||
local MESH_DOGE = "rbxassetid://4574885352" | ||
|
||
itFOCUS("should apply updates to existing instances", function() | ||
Kampfkarren marked this conversation as resolved.
Show resolved
Hide resolved
|
||
local mockApiContext = {} | ||
function mockApiContext:createAssets() | ||
return Promise.resolve("rbxasset://rojo-tests/DogeUpdate.rbxm") | ||
end | ||
|
||
local instanceMap = InstanceMap.new() | ||
|
||
local oldDoge = Instance.new("MeshPart") | ||
instanceMap:insert("DOGE", oldDoge) | ||
|
||
local patch = PatchSet.newEmpty() | ||
table.insert(patch.updated, { | ||
id = "DOGE", | ||
changedProperties = { | ||
MeshId = { | ||
Type = "Content", | ||
Value = MESH_DOGE, | ||
}, | ||
}, | ||
}) | ||
|
||
local _, unappliedPatch = assert(tryGetObjects(instanceMap, mockApiContext, patch):await()) | ||
assert(PatchSet.isEmpty(unappliedPatch), "expected remaining patch to be empty") | ||
|
||
local newDoge = instanceMap.fromIds["DOGE"] | ||
assert(newDoge ~= nil, "no instance with id DOGE") | ||
expect(newDoge.MeshId).to.equal(MESH_DOGE) | ||
end) | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ | |
}, | ||
|
||
"TestEZ": { | ||
"$path": "modules/testez/lib" | ||
"$path": "modules/testez/src" | ||
} | ||
}, | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
rojo build test-place.project.json -o TestPlace.rbxlx | ||
run-in-roblox --script run-tests.server.lua --place TestPlace.rbxlx |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
std = "roblox" | ||
std = "roblox+testez" | ||
|
||
[config] | ||
unused_variable = { allow_unused_self = true } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! This is important for if a file is newly added on the filesystem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Figured, just couldn't figure it out because I was using existing projects, and wasn't creating new files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied and pasted a file and...it worked perfectly? It was still under
updated
, notadded
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind, added seems to be used when modifying .project.json.