Basic instance replicator

Help and Feedback Code Review
#1

Hi,

I’m looking for some feedback on a basic instance replicator I made in a few minutes. It’s very bare-bones but does what is expected. The issue I was experiencing was that many instances (parts, folders, models, textures…) were parented to workspace when the player first joined. Obviously this caused performance issues for users with devices that had 1GB of ram, particularly crashing.

(Also thanks to Colbert for the example which this was based on)

Server script:

local players = game:GetService('Players')
local serverStorage = game:GetService('ServerStorage')
local replicatedStorage = game:GetService('ReplicatedStorage')

local playerData = replicatedStorage:WaitForChild('PlayerData')

local decorations = workspace:WaitForChild('Decorations')
decorations.Parent = serverStorage

local replicatedInstances = {} -- table that handles all instances that were replicated

local function handlePlayerAdded(player: Player)
	local data = playerData:WaitForChild(player.UserId)
	local settings = data:WaitForChild('Settings')
	local stageDecorations = settings:WaitForChild('StageDecorations') -- this disables stage decorations, the instances that are being replicated
	local playerGui = player:WaitForChild('PlayerGui') :: typeof(game:GetService('StarterGui'))
	local instanceReplicationFolder = playerGui:WaitForChild('InstanceReplicationContainer') -- a ScreenGui that the items are parented to since this is the only way to simulate targeted replication
	replicatedInstances[player.UserId] = {} -- table that contains all replicated instances for this player
	local doReplication = true -- used to halt instances that are being replicated if need be
	local function replicate()
		doReplication = true
		for i,v in ipairs(decorations:GetChildren()) do
			local thisInstance = v:Clone()
			thisInstance.Parent = instanceReplicationFolder
			if i % 50 == 0 then
				task.wait(0.1)
			end
			table.insert(replicatedInstances[player.UserId], thisInstance)
			if not doReplication then -- if the setting is set to false mid-load
				return
			end
		end
	end
	local function unReplicate() -- remove the replicated instances
		doReplication = false
		for i,v in ipairs(replicatedInstances[player.UserId]) do
			v:Destroy()
		end
	end
	if stageDecorations.Value then -- only if they choose to replicate stage decorations
		replicate()
	end
	stageDecorations:GetPropertyChangedSignal('Value'):Connect(function()
		if stageDecorations.Value then -- is this if-statement necessary?
			replicate()
		else
			unReplicate()
		end
	end)
end
for i,v in ipairs(players:GetPlayers()) do -- just in case
	handlePlayerAdded(v)
end
players.PlayerAdded:Connect(handlePlayerAdded)
players.PlayerRemoving:Connect(function(player)
	if replicatedInstances[player.UserId] then
		for i,v in ipairs(replicatedInstances[player.UserId]) do
			v:Destroy() -- destroy the instances
		end
		replicatedInstances[player.UserId] = nil -- discard the table
	end
end)

LocalScript, child of instance replication folder:

script.Parent.ChildAdded:Connect(function(child)
	if child ~= script then -- shouldn't ever fire but just in case
		task.defer(function() -- need task.defer so it doesn't spam "something unexpectedly set the parent of ___"
			child.Parent = workspace
		end)
	end
end)

That’s it.

I’m looking for ways to further improve performance if any. I also want to remove the task.defer if possible but I don’t know if it’s possible.

Thanks!

1 Like
#2

This is really good. The more I look at it, the better it gets.
Only thing of note is you don’t need replicatedInstances table.
When you move the items over to the screengui, the client can’t remove them from the server.
So if you remove the table you don’t need anything in PlayerRemoving and you can change unReplicate to:

	local function unReplicate() -- remove the replicated instances
		doReplication = false
		for i,v in ipairs(instanceReplicationFolder:GetDescendants()) do
			v:Destroy()
		end
	end
1 Like
#3

Thanks!

Would this cause a memory leak on the server tho? When players leave, are the player instances actually destroyed or are they just parented to nil?

#4

Did some testing in studio and yes, roblox does destroy it
I ran this a few times in the server cmd bar until the memory usage rose a bit then I had player 2 leave

for i=1,30000 do Instance.new("Part",game.Players.Player2.PlayerGui.ScreenGui.Frame) end print("fin")
1 Like
#5

Alright, that’s perfect then. Thanks!