How should I approach this NPC system better?

Currently, I’m working on an npc system that gets npcs to walk into a restaurant, sit down, and then are served by a player.

This is the server script, the SpawnCustomer function is the function that creates the npc, moves them outside the restaurant and into a seat inside. This also creates a prompt in the HumanoidRootPart of the npc, which when triggered, will fire a remote to the client triggering it.

The client will then wait until the player has inputed the customers order into a GUI, then will fire the remote back to the server. I am using RemoteEvents to communicate with the client instead of RemoteFunctions because of this problem I had in my previous post.

Here are a few modified bits cut out of my script (read the comments in the script for info):

Server

local function SpawnCustomer()
	-- tone of code here creating the npc, and moving it into the restaurant.
	
	Prompt.Triggered:Connect(function(Player) -- this fires when the proximity prompt in the npc is triggered
		Remote:FireClient(Player) -- this fires the remote to the client so it can process the npcs order
	end)
	
	local Connection -- predfine the connection variable
	local function OnRecieve()
		Connection:Disconnect()
		-- I basically only want this function to run once, then disconnect it immediately once it's triggered.
		-- The problem is, other clients that finish the order the same time as the local player, can fire the event...
		-- and then this function is triggered, when only the player who triggered the prompt originally should be triggering this
		-- This "disconnect" thing doesn't always work too, sometimes Connection is nil somehow. I don't think this is the best way to approach it.
		
		-- runs a bunch more code after the server has recieved the npc's restaurant order from the client blah blah blah
	end
	Connection = Remote.OnServerEvent:Connect(OnRecieve) -- connects the function to the connection
end

coroutine.resume(coroutine.create(SpawnCustomer)) -- this is definitely not the best way of doing it, but this creates a new coroutine so this function can run on a new thread, creating the npc without having to yield the script

Client

local MainConnection -- predefine the connection variable(s) - this is the connection for the whole .OnClientEvent function
local DistanceConnection -- and this connection is for checking the distance between the player and the npc
local function OnEventRecieved() -- client recieves the event from the server
	
	local function Return(Data) -- this function is to return (fire the event back to the server) once triggered
		if MainConnection then MainConnection:Disconnect(); MainConnection = nil end
		if DistanceConnection then DistanceConnection:Disconnect(); DistanceConnection = nil end
		-- disconnecting the above two doesnt work sometimes, and overall isnt very reliable...
		-- I am mainly looking for a better way of handling all this code instead of using all these connections.
		
		Remote:FireServer(Data) -- sends the given data BACK to the server with the same remote.
		-- run a bunch of other not really important gui stuff blah blah...
		
		local f = coroutine.create(function() -- create a new thread
			wait(1)
			MainConnection = Remote.OnClientEvent:Connect(OnEventRecieved) -- connect the connection again after 1 second
		end)
		-- I am not really sure why I have this, but it's to connect the event again once we've sent the remote to the server, so the server can fire the remote again if needed - its not the best though
		coroutine.resume(f)
		return
	end
	
	local Distance = 10
	local function PlayerHasMoved()
		local Magnitude = DistanceBetweenPlayerAndNpc -- (obviously this is not how I calculate the magnitude)
		if Magnitude > Distance then
			if DistanceConnection then DistanceConnection:Disconnect() end
			Return("Timeout")
		end
	end
	DistanceConnection = Player.Character.Humanoid:GetPropertyChangedSignal("MoveDirection"):Connect(PlayerHasMoved)
	-- This above function will now run everytime the player moves.
	
	local SecondsBeforeTimeout = 10
	local Timeout = coroutine.create(function()
		local Count = SecondsBeforeTimeout

		while true do
			task.wait(1)
			Count -= 1

			if MainConnection == nil then break end -- if the main connection is nil (ie: the data has been returned back to the server already with the Return() function) then break the loop
			if Count <= 0 then Return("Timedout") break end -- Return "Timeout" as the player took too long to serve the customer
		end
	end)
	coroutine.resume(Timeout) -- run all this on a new thread
	
	-- IMPORTANT: The 'Return("Timeout")' is returning a timeout message back to the server, letting the server know that either the player got too far away from the customer so it cancelled the process, or the player took too long to server it so it cancelled the process.
	
	-- bunch more irrelavant code things
	
	if WhatTheCustomerOrdered == WhatThePlayerWroteOnNotePad then -- If the waiter wrote what the customer wanted correctly then:
		Return(true) -- Return true back (ie: the player successfully served this customer! yay!)
	else
		Return(false) -- Return false back (ie: the player ordered the wrong thing for the customer, not yay...)
	end
end
MainConnection = Remote.OnClientEvent:Connect(OnEventRecieved) -- connect the connection

I don’t know how much sense this made; please ask any questions if you have any, since sometimes I struggle to get my head around this lol.

TLDR: Using all these connections I don’t think is very efficient and safe, and they often don’t work. I am looking for a good way of spawning in npc’s, running basic checks, and then disconnecting the functions.

Thanks, please let me know your thoughts, and if there is a better way of doing all this…

1 Like

dont use coroutine.resume(coroutine.create() or whatever use wrap

Yea I’m about to change that, but I’m looking for a better way to do the whole thing basically.

well its not really well easy to change try learning abt oop

I know about OOP, I’m just looking for an overall way of improving this instead of using so many connections, but I seem to have improved it myself.