How is my NPC spawning code?

Help and Feedback Code Review
#1

Hello! I have recently came back to game development with a fresh mind and have come up with a pretty cool idea. In the game, each player has their own plot where they can spawn an NPC and experiment (nothing against Roblox TOS of course). Right now, I am currently working on the system for spawning and deleting and I was wondering if I could improve anything about it.

-- Services
local ServerStorage = game:GetService("ServerStorage")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")

-- Variables
local npc = ServerStorage.NPC
local npcSpawnButton = workspace.NPCSpawnButton
local npcDeleteButton = workspace.NPCDeleteButton
local npcSpawnLocation = workspace.NPCSpawnLocation

local skinColors = require(ReplicatedStorage.SkinColors)

local newNPC = nil

function setRandomSkinColor(player)
	local chosenColor = skinColors[math.random(1, #skinColors)]
	
	for i, bodyPart in pairs(newNPC:GetChildren()) do
		if bodyPart:IsA("MeshPart") or bodyPart:IsA("Part") then
			bodyPart.BrickColor = BrickColor.new(chosenColor)
		end
	end
end

function spawnNPC (player)
	local currentNPC = workspace:FindFirstChild(player.Name.."'s NPCs"):FindFirstChildWhichIsA("Model")
	
	if currentNPC then
		return false
	end
	
	newNPC = npc:Clone()
	setRandomSkinColor()
	
	newNPC:SetPrimaryPartCFrame(npcSpawnLocation.CFrame)
	
	newNPC.Name = player.Name.."'s NPC"
	newNPC.Parent = workspace:FindFirstChild(player.Name.."'s NPCs")
	
	newNPC.PrimaryPart:SetNetworkOwner(player)
end

function deleteNPC (player)
	local currentNPC = workspace:FindFirstChild(player.Name.."'s NPCs"):FindFirstChildWhichIsA("Model")
	
	if currentNPC then
		currentNPC:Destroy()
	else
		return false
	end
end

function createNPCFolder (player)
	local playerNPCsFolder = Instance.new("Folder")
	playerNPCsFolder.Name = player.Name.."'s NPCs"
	playerNPCsFolder.Parent = workspace
end

Players.PlayerAdded:Connect(createNPCFolder)
npcDeleteButton.ProximityPrompt.Triggered:Connect(deleteNPC)
npcSpawnButton.ProximityPrompt.Triggered:Connect(spawnNPC)

This is actually my first time using returning, but it seems to be working flawlessly in game.

2 Likes
#2

You create the folder, but you forget to clean it up when the player leaves. The folder will still remain in the workspace.

Not sure why you want to return false. In these cases, you might as well just always return nothing.

This variable does not have to exist in this scope as it is only used in spawnNPC.

3 Likes