-
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
Conversation
plugin/src/tryGetObjects.lua
Outdated
|
||
-- GetObjects won't help with anything that failed to remove | ||
unappliedPatch.removed = patch.removed | ||
-- TODO: Implement this? Do we have to? |
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
, not added
.
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.
It's not just Connect that's not changing state...it's Settings too? Honestly clueless, will look further 🤷 |
Oh...it's because Flipper is using PreRender now, which isn't actually enabled yet. CC @Reselim, might not want to do that yet! |
Yeah, no worries. I realized that and I figured it would be okay to keep it as is as long as there isn't a new release... it seems Rojo is using the master branch though. I'll revert the change for now. |
Should be all good! |
@LPGhatguy It appears that invalid rbxm files crash Roblox Studio, while invalid rbxmx files will simply error. I think it is better for Rojo to produce the latter for GetObjects, any thoughts? |
Code is all ready (except for the GC entry I added to the changelog). I'm going to start battle testing this for my own games. |
Found the bug, when you create an instance in Studio then save to file, it duplicates. I don't think GetObjects has anything to do with this though, as nothing in this was GetObjects-worthy (maybe SourceAssetId?). 2021-02-27T20-26-39.mp4 |
I've been looking for a while and honestly still have no clue what's up with MeshParts being funky, and am honestly worried that its just a GetObjects bug. The visual size is what the mesh was actually modeled at (if I import that deagle normally it's that huge). The blue is the actual |
Can you upload the model to the website and try to insert it with |
It's already on the website, and I can import it just fine, it just imports really big (but the part size is accurate to the model). |
Looks like |
Looks like in this implementation, refs are not being reserved. |
Refs are going to be tricky! We'll need to use a hybrid approach to write ref properties that fall outside the bounds of the object we use |
So if I can understand the problem here, the Ref structure in the file format (which points to some other part of the file?) doesn't get preserved and so we lose this information. What approach can we take to solve this? This is the primary blocker for my team using Rojo 😔 so I'd love to help out if you'd like! |
Closes #205.
TODO:
* - This is because the serve is trying to request an update with the service its under. It tries to insert the service as a child, and then crashes. GetObjects should not be trying to create new instances when its just metadta that is different.