Optimizing code to destroy tycoon model

Help and Feedback Code Review
#1 
local function DeleteTycoon(player)
	
	print(player.Name.." has left the game!")
	
	for _, Tycoon in pairs(TycoonsHolder:GetChildren()) do

		local TycoonOwner = Tycoon:FindFirstChild("TycoonOwner").Value
		local CashToSave = Tycoon:FindFirstChild("CashToCollect")
		if TycoonOwner then
			if TycoonOwner == player then

				local StatsFolder = player:FindFirstChild("StatsFolder")
				local TycoonSavedCash = StatsFolder:FindFirstChild("TycoonSavedCash")

				TycoonSavedCash.Value = CashToSave.Value

				local StatsFolder = player:FindFirstChild("StatsFolder")
				local TimePlayed = StatsFolder:FindFirstChild("TimePlayed")
				
				local Start = tick()
				Tycoon:Destroy()
				print("Performance check: [Destroying tycoon]: "..tick()-Start)
				
				local Start2 = tick()
				TycoonCloneHolder:FindFirstChild(Tycoon.Name).Parent = TycoonsHolder
				print("Performance check: [Parenting tycoon back]: "..tick()-Start2)

				print(player.Name.." has left the game. Successfully destroyed their tycoon!")
			end
		end
	end
end

Hello! I’ve currently released my first tycoon and everything is going well except for the major lag that occurs when a player leaves and rejoins the game. (The above code is connected to a PlayerRemoving event in the DataStore script.)

How the tycoons function:

  • Each tycoon has its own script that calls 4 functions, InitializePurchasingPads(), SetDisplayCollectorAmount(), CreatePurchasePads() and InitializeMoneyGiver()
  • When the first player joins, all the tycoons run these functions and sets up the tycoons ready to be used for all players.
  • When a player joins, LoadSavedBuildings() is called and basically loads every building through a for-loop (this is inside the tycoon script).
  • When a player leaves, the tycoon is destroyed and reparented to the workspace.

Above, I have marked down the speed for how long the Destroy and Reparenting takes for 1 tycoon in the game and the results were:

Performance check: [Destroying tycoon]: 0.14637398719787598
Performance check: [Parenting tycoon back]: 0.6646103858947754

The performance check is in studio with 1 tycoon but it doesn’t seem like it’s adding up as in a real game it lags the entire server for 1-2 seconds. I’m assuming that the actual problem is due to the fact that destroying the tycoon which has hundreds of models that contain 10-20 parts in each model and replicating it to all clients and therefore causing issues. What would be the best way to remove the tycoon without causing major lag? In addition if there is a better way to manage the tycoons in general?

All help is greatly appreciated.

Thank you!

1 Like
#2

Hi there, one thing that I immediately thought that should be replaced is using ipairs over pairs. ipairs in general, is faster than pairs when using arrays, and GetChildren return an array.

I am not that sure about how your game works, but from what I got, I feel like preparing all the tycoons once a new player join is a bit off, maybe saying what those functions do would help in finding a solution.

Also, with the replication part, I don’t think Roblox manages destroy’ing replication like this. With a recent update on Destroy() replication behavior, the server sends a message to the client telling it to “Delete this instance” which can be the tycoon, and internally, Destroy will also be called on the children of the instance, but the only thing that can make sense is recreating it since it will replicate those to the client but I doubt it will be the case.

#3

Hello thank you for the reply. I apologize for not being clear about some things.

InitializePurchasingPad() - Connects each pad to a touched event to handle building creation.
SetDisplayCollectorAmount() - Set the collector’s display amount for how much cash it contains.
CreatePurchasePads() - Sets the price and description of each pad
InitializeMoneyGiver() - Connects the collector to a touched event to handle money collection.

Overall the functions don’t really do much in the destruction process. It’s just the DeleteTycoon() function which destroys the tycoon that is causing me issues by creating a lot of lag destroying hundreds of models. What’s a better way of presetting all the tycoons when the first player joins?

1 Like
#4

Interesting. The thing is, when destroying objects on the server, the server will just send a message to all the clients to destroy that object, and by the nature of Destroy, it will also be called on the object’s descendants both on the server and the clients separately.

Because I can’t explain well, here is an example:
You called Destroy() on an object, perhaps the tycoon → the server sends a signal to the client saying to it to destroy the object → both will destroy the object and it’s descendants separately without the need of replication.

These functions sound like not a big deal, you can benchmark them though and see if there is anything off, also, talking about benchmarking, I suggest you to use os.clock() as it is intended use is benchmarking.

Other than that, I feel like destroying a tycoon and then creating a brand new one once a player leaves is a bit costy as this where replication comes in. You see, in Destroy()ing, the server sends a signal, that’s it, on the other hand, when instancing, the server sends the information about the object and it’s descendants. It works differently though when using StreamingEnabled. So, the thing that seems like most expensive in terms of replication, is constructing a new tycoon.

After I read about what these functions do, it is okay

Anyways, I am going to test destroying tons of objects, maybe like 100 baseparts in 100 models each to be sure about what I said.

EDIT for the test: I ran a test which I firstly instance’d 10 models in a folder and then in each model I constructed 100 parts, thus having 1000 parts (which isn’t generally a great amount of parts for a single tycoon for instance). The time that the server took to destroy them was 0.14912970000023051 which seems low for destroying that amount of parts. Anyways, you seriously need another way of this if you think the 0.15 seconds are too high for destroying and 0.7 for reparenting but again, I have no idea about how you create another tycoon and so on.

Edit: also, my English isn’t the best thing because it isn’t my native language, so sorry if I used some tense in the wrong context.

1 Like
#5

[quote=“UnderEvaluation, post:1, topic:1820108”]

local function DestroyTycoon(player)

    local statsFolder = player:FindFirstChild("StatsFolder")
    local tycoonSavedCash = statsFolder:FindFirstChild("TycoonSavedCash")

    local tycoon = TycoonsHolder:FindFirstChild(player.Name)
    local cashToSave = tycoon:FindFirstChild("CashToCollect")
    local tycoonClone = TycoonCloneHolder:FindFirstChild(player.Name)

    tycoonSavedCash.Value = cashToSave.Value

    tycoon:Destroy()
    tycoonClone.Parent = TycoonsHolder

    print(player.Name .. " has left the game. Successfully destroyed their tycoon!")
end
#6

Hello sorry for the very late reply.

I never understood the background behind how Destroy() really works, so this definitely helped me. I think that I will experiment and see if I can slowly delete and slowly instance models to prevent the game from lagging.

1 Like
#7

Glad that I helped, but I found out this behavior isn’t enabled by default, hence why there is lag, probably because of memory usage as currently, parts on the client only get parented to nil. You can enable this behavior by checking ReplicateInstanceDestroySetting to Enabled though. More information:

UPDATE: I didn’t see the latest message in the post stating that it is enabled by default :sweat_smile:

2 Likes
#8

Thank for another helpful tip! I just checked into studio and its setting is set to Default does that means its enabled by default or set to disabled? Sorry just wanted to clarify!

#9

Enabled by default.

(the character limit)

#11

Having this same issue. The issue is tends to get much worse when using StreamingEnabled