Skip to content
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

Instance.Clone method definition does not account for Instance.Archivable behavior #543

Closed
jheinem1 opened this issue Mar 18, 2022 · 3 comments · Fixed by #585
Closed

Instance.Clone method definition does not account for Instance.Archivable behavior #543

jheinem1 opened this issue Mar 18, 2022 · 3 comments · Fixed by #585

Comments

@jheinem1
Copy link

jheinem1 commented Mar 18, 2022

According to the documentation of Instance.Archivable, when this property is false, it introduces the possibility of Instance.Clone() failing by returning nil rather than throwing an error.

Calling Clone directly on an object will return nil if the cloned object is not archivable.

https://roblox-ts.com/playground/#code/DYUwLgBAlgdgzmAhjAxiCBeCMQHcICS8SqIAFAEQCyA9gCYjAUCUAdAMLA05nMQD0-CGAAWUOBBQ0ArsDoQARugAG0mAwBmsEHWUAoPQAcATrDBlYCZGlYA5RAFsQzIA

@Dionysusnu
Copy link
Contributor

Dionysusnu commented Mar 18, 2022

This is unfortunately a conscious decision that is slightly unsafe. The Archivable property is very rarely used, and the inconvenience of the possibly-undefined type outweighs the benefit of the better correctness.
We should add a warning to the documentation comment though.

@jheinem1
Copy link
Author

Fair enough, though it may be necessary to add some sort of warning specifically when cloning models, as character models are not archivable by default.

@osyrisrblx
Copy link
Member

I agree with what Dionysusnu said above. Going to close this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants