:clone() is cloning Archivable-false objects and their children

Bug Reports Engine Bugs
#1

It appears the :clone()/:Clone() function is cloning objects whose Archivable properties are set to false. The function should ignore non-Archivable objects. I noticed this while testing some old code that was broken, initially thinking the behavior of BindableFunctions were changed. I tracked the bug down to the clone() function acting strange.

Steps to reproduce:
[ol]
[li]Open a new place.[/li]
[li]Add a Part. Put a BoolValue (or any dummy object) inside of the part. Set both Part and the child object Archivable properties to false.[/li]
[li]Type this into the command bar:[/li]

workspace.Part:clone().Parent = workspace

[/ol]
Expected behavior:
The :clone() function should return nil and therefore throw an error when trying to set the .Parent property of the returned nil value.
Actual buggy behavior:
The :clone() function returns a copy of the part and all its descendants with all the Archivable properties set to true.

Edit:
Here’s some simpler code. The print() statement should print nil.

obj = Instance.new("Part")
obj.Archivable = false
obj2 = obj:clone()
print(obj2)
#2

As far as I know Archivable applies to online mode.
Can you attempt this in online mode via Server Stats frame?

#3

The problem seems to also occur in Online mode.

#4

This is an unintentional change, but in retrospect a good one I think.
Unintentional = we did not know that Clone does not clone non-archivable instances so we could not tell developers about it beforehand
A good one = why doesn’t it clone non-archivable instances? :slight_smile:

For the record, the technical answer to the latter is “clone worked by serializing instances to a in-memory XML and deserializing”. We changed it to native cloning to make it faster.

#5

It’s good in that it was made faster by not having to do the serialization, but I don’t think it’s good because it can be quite useful in situations. Eh. It’s a strange change I’ll have to get used to.

For example, I’ve created a system to work with Humanoids that allows me to apply more immersive combat mechanics such as damage mitigation, healing, numerical shields, stacking slow/haste effects, death prevention, etc. It adds objects (non-Archivable BindableFunctions, BindableEvents, NumberValues, containers) which custom scripts can interact with to make the combat mechanics mentioned earlier. It’s quite old. I should probably re-make the system.

Specifically I would have a dummy character in the workspace which the script would set up the objects of interaction. To make new AI-controlled characters it would clone the dummy one (ignoring the script-made objects) and as I put the new character in the workspace the script would set up the objects again to work with the new humanoid. In retrospect this probably wasn’t the best system, but it worked for the longest time and in the past humanoids tended to die on me if I cloned them from anywhere other than the workspace.

The script is set up to automatically create the objects on humanoids not specifically tagged to be ignored. This allows me to simply clone the dummy AI on-the-fly to create units and trust that the script would set it up for use with the new objects (workspace.DescendantAdded is a powerful thing!).

Still, it’s an interesting change. I don’t know why it wouldn’t clone the non-Archivable objects, but in the same vein of reasoning I can’t think of why it should. I got used to knowing that a non-Archivable object would never stick around in my game, and it also was a good way to mark that something had been generated by a script and was a “temporary object”. If something was cloned, you could be sure that the “temporary objects” would not be cloned along with it.

#6

[quote] This is an unintentional change, but in retrospect a good one I think.
Unintentional = we did not know that Clone does not clone non-archivable instances so we could not tell developers about it beforehand
A good one = why doesn’t it clone non-archivable instances? :slight_smile:

For the record, the technical answer to the latter is “clone worked by serializing instances to a in-memory XML and deserializing”. We changed it to native cloning to make it faster. [/quote]

I thought this was the main purpose of the Archivable property? Will it be removed now, as it doesn’t really have a use?

I don’t imagine people actually use it intentionally to omit changes from being published.

#7

[quote]
A good one = why doesn’t it clone non-archivable instances? :slight_smile: [/quote]

It is nothing but logical to have the archivable property both make stuff not save and have it not be cloned. The idea is that it should be used for things that should never be reused elsewhere, whether that means being cloned, saved, etc.

I used to mark all the services in the game except those with content as non-archivable before saving a game to have it start with only the minimum services, and also to reduce the size of place files when I saved to a file.

I also used to make all my scripts creating GUIs set the archivable property of all the descendants of the GUI objects to false, because these GUIs can only work when the event handlers attached by the script are connected, and they can only ever be connected for the instance of the GUI created by the script, so saving or cloning the GUI makes no sense as that will make it not work anymore (I did this in Boteri, as you can see here).

I can easily imagine that someone would want to dynamically add NPCs, prizes, rewards, items, bonuses or other temporary things in random locations in a map in a game where maps are chosen for different rounds. In such a case, it only makes sense to set the archivable property of these objects to false so that they do not ever get cloned or saved (which might happen while testing, saving a custom map with data persistence, or cloning the map around for various reasons). I have never done this, but I would certainly have done so if I had created such a game.

This bug is undesirable, as I have explained with these three examples, ergo it should be fixed. It should certainly not be left as it is! I cannot imagine any reasonable case where someone would not want something to save to a file, a model, or data persistence, but would want it to be cloned. If we consider in addition that previous scripts expect this behavior (and the archivable property isn’t as uncommonly used as one would think; Ozzypig and I have used it, for example), I think it is only too clear that this bug should be fixed.

#8

Unfortunately, I can’t ask the person who designed this whether the intention for Archivable was to prevent cloning or not. The comments in the serialization code (and the name of the property!) suggest otherwise:

	// TODO: archivable==false messes up functions like clone()
	//       find a better way to do this

So I’m reasonably sure it was a bug. I’m still not sure whether the behavior is useful or not.
If anybody else who did not express an opinion find the old behavior better (or worse), please chime in.

#9

I find making Archivable false as to prevent models from being copied quite useful. It especially was a big security option back when the .dll exploit was about. Still is too. I use it for security for parts of all kind, please don’t change it.

#10

I’m with Mark Otaris on this one. I also think that this change is for the worse. I actually thought that Archivable = false objects not being cloned was not a bug because it made perfect sense: In general in APIs, if something can’t be Serialized then it can’t be copied either.

And as such I actually used this behavior heavily in my projects. I have quite a few pieces of code and behaviors that have been broken by this change: Including the fact that all of my plugins use parts for their 3D UI, and I marked those parts as Archivable = false. Previously cloning an object with some temporary 3D UI elements inside of it, created by one of my plugins, would not clone the UI elements, correctly acting as those though those elements were ephemeral objects of sorts. Now when you clone a part that I’ve put some 3D UI elements into, those elements will be incorrectly copied as well, and there’s no good way for me to fix it.

I think that for more people than you expected were actually using this behavior assuming, in my opinion correctly, that not copying was an intentional behavior.

#11

Yeah I thought this was the practical use for Archivable :confused:

#12

Okay. It should have been called “Cloneable”, but oh well.

A fix for this will ship next week.

#13

[quote] Unfortunately, I can’t ask the person who designed this whether the intention for Archivable was to prevent cloning or not. The comments in the serialization code (and the name of the property!) suggest otherwise:

	// TODO: archivable==false messes up functions like clone()
	//       find a better way to do this

So I’m reasonably sure it was a bug. I’m still not sure whether the behavior is useful or not.
If anybody else who did not express an opinion find the old behavior better (or worse), please chime in. [/quote]

It seems clear that it was not intended, then. Regardless, the old behavior was more useful than the new one, and both can be considered as “bugs”, so I guess it doesn’t matter.

#14

Thank you zeuxcg. I’m with Mark and Stravant too. I make my scripts instantiate GUI objects with the Archivable property as false, as well as many other items.

One should note that “Archivable = false” is not an appropriate security measure against people cloning your game, into things like settings and such. Using a serialized ROBLOX API (which I released), or just common sense, it is quite possible to clone a game’s geometry with “Archivable = false”.