NPC Backend Optimization

Help and Feedback Code Review
1

Hey everyone,

I’m making a tycoon game and already (to my suprise) people have pushed the limits to my NPC system. 360 NPCs, we’ve got a lot of server memory usage.

Some things to note:

  • There is almost no client lag at all. Streaming Enabled is Enabled as well as other optimization methods.
  • The NPCs are tweened on the client, not the server. The server sees it as the NPC “teleporting” to each point to limit server calculations.
  • The NPCs have humanoids (for purposes of clothing) but have all humanoid states disabled.

Now, some of you are probably going to cringe at this but, each of the 360 NPCs run on a seperate thread using the spawn() function. It’s served well for 360 NPCs but I’d like to push it further and further. The server memory usage at 360 NPCs is about 1gb. We have 4 players per server that each can have at the momment, no cap to the NPCs.

The NPCs use pathfinding. But the path is only computed once per path. The NPCs only using CFrame and are anchored.

Here is a gif of what the client sees: (Note FPS is lower because of GIF encode)
https://gyazo.com/ec4f2afe123bd932b35cdd3ce2cca99e
https://i.gyazo.com/ec4f2afe123bd932b35cdd3ce2cca99e.mp4

So here is my main question. I’m looking for different ways/suggestions to handle the NPC controllers. I’m welcome to learn any new methods of doing so.

Here is some of the code now:

  ['Generic Personnel'] = function(plr,npc)
		local targetSeat = npcUtils.FindStation(plr,"FoundationPersonnel")
		if targetSeat then
			targetSeat.Value = npc
			npc:SetPrimaryPartCFrame(npc:WaitForChild("Moving").Owner.Value.Tycoon.Value.DoubleWorkingDoor.BasicDoubleDoorAnchor.CFrame * CFrame.new(0,1,-5))
			local targetObject = targetSeat:FindFirstChildWhichIsA("Seat")
			if targetObject then
				pfs.computeAndMove(targetObject.Position,npc)
				while npc:WaitForChild("Moving").StopNPCLoop.Value == false do
					npc:SetPrimaryPartCFrame(targetObject.CFrame)
					npc:WaitForChild("Humanoid").Sit = false
					wait(math.random(30,90))
					if math.random(1,8) == 5 then
						local targetPatientSeat = npcUtils.FindCaptive(plr,"MedicalBay")
						if not targetPatientSeat then
							game.ReplicatedStorage.Remotes.FireHeaderBar:FireClient(npc:WaitForChild("Moving").Owner.Value,"Your staff members are sick. Consider making a medical bay with medical personnel.",Color3.new(255/255,0/255,0/255))
		                    wait(0.1)
		                    game.ReplicatedStorage.Remotes.FireHeaderBar:FireClient(npc:WaitForChild("Moving").Owner.Value,"You will be charged $250 if one of your staff members die.",Color3.new(255/255,0/255,0/255))
		                    npc:WaitForChild("Humanoid"):TakeDamage(20)
						else
							targetPatientSeat.Value = npc
							
							local targetObjectPatientSeat = targetPatientSeat:FindFirstChildWhichIsA("Seat")
							
							npc:WaitForChild("Humanoid").Jump = true
							npcUtils.ClearSeat(targetObject)
							
							pfs.computeAndMove(targetObjectPatientSeat.Position,npc)
							npc:SetPrimaryPartCFrame(targetObjectPatientSeat.CFrame)
							npc:WaitForChild("Humanoid").Sit = false
							wait(30)
							targetPatientSeat.Value = nil
							npc:WaitForChild("Humanoid").Jump = true
							
							npc:SetPrimaryPartCFrame(targetObjectPatientSeat.CFrame)
							pfs.computeAndMove(targetObject.Position,npc)
						end
				
						
					
					end
				end
			end
		end
	end,

Thank you!

EDIT: I would also like to note that after a while, the server will eventually crash.

2 Likes
2

It sounds like you have a memory leak somewhere. Overall your code is structured nicely and you’re using some good methods to reduce performance cost. Finally, I’d say don’t use spawn. Spawn is extremely bulky and slow, especially when used in a loop. It even yields the thread exactly like calling wait(). Instead you should use coroutine.create (or alternatively, coroutine.wrap). This will most likely slightly improve memory usage and it’ll make your code much faster where you create threads.

5 Likes
3

Coroutines made a massive improvement, thank you for the suggestion!

1 Like
4

I do have one error with using coroutines. When the server or player leaves/shutdown it runs into a detrimental lag spike due to the coroutines erroring out for script execution past shutdown. I’ve tried yielding the coroutines before stopping. Any ideas on how to fix this?

5

Hmm. You can create a signal of some kind to shutdown the coroutine. Anwhere before and inside of loops should check, including for loops and stuff, and anywhere after yields. To kill the thread you can simply return. Finally, you’d want to store each thread. When shutting down your threads you can use coroutine.status to check if they’re yielding, and if they are, continuously try to resume them until they come up with a dead status.

Example:

local killThreads = false
local threads = setmetatable({}, {
	__mode = "v" -- This sets the table to use weak values, which basically means the table does not keep references to its values (so they can GC even when in the table)
})
local function killRunningThreads()
	killThreads = true
	for _, thread in ipairs(threads) do
		-- Note: You don't want to yield in an infinite loop without a killThreads check since that will cause this to loop forever
		while coroutine.status(thread) == "yield" do
			pcall(coroutine.resume, thread)
		end
	end
end

local thread = coroutine.create(function()
	if killThreads then
		return -- Returning will kill the thread
	end
	while true do
		if killThreads then
			return
		end
		-- Code
		-- Some yield
		if killThreads then
			return
		end
		-- More code
		local someTable = {1, 2, 3} -- Example table/for loop
		for index, value in ipairs(someTable) do
			if killThreads then
				return
			end
			-- Some for loop code
			wait() -- Example yield
			if killThreads then
				return
			end
		end
		-- Etc, etc
	end
end)
table.insert(threads, thread)
pcall(coroutine.resume, thread) -- This is something I overlooked previously.
-- If the coroutine were to error on resume (including in a wrapped function) aka before it has yielded, the error will be thrown by resume.
2 Likes