How can I improve and shorten this NPC chat code?

Help and Feedback Code Review
#1

Hello, developers. So I made a code where when you click on an NPC, they will talk. This is for a shop system.

I think the code looks and is okay, but I want to hear from some developers on what you think on the code.

  • How can I shorten the code?
  • How can I improve the code?
  • How can I make the code run better?
-- 
script.Parent.MouseClick:Connect(function()
	wait(1)
	local chatService = game:GetService("Chat")
	local head = game.Workspace[" "].Head 
	
	while wait(0.1) do
		chatService:Chat(head,"Hi there! Are you sure you want to buy this item?")
		wait(5)
	end
end)

Thanks!

#2

In its current state, the NPC will never stop chatting. Is that the intended functionality?

#3

I just realized that, and no, it’s not suppose to chat endlessly.

#4

Ah ok. If it’s only supposed to chat once, then you can remove the while loop and keep the chatService:Chat(...).

I also recommend you define chatService and head outside of the function. If you’re going to use them a bunch of times, you can define them once at the top and then never again. Currently, you’re redefining them every time the button is clicked. The performance gain is negligible, but it’s a good habit to not repeatedly define the same variables like that.

1 Like
#5

I would recommend dropping the while wait(x) idiom too. Not only are you already waiting at the end of the loop (wait(5)) but it’s also generally looked down upon as code smell. while true whould fit your needs better.

5 Likes
#6

Here’s how I would write your code;

local chatService = game:GetService("Chat")

local function onMouseClick(player, text)
    if player:IsDescendantOf(Players) then
       assert(typeof(text) == "string", "Text argument must be a string")

       local head = workspace[" "].Head 

	   chatService:Chat(head, text)
  	   wait(5)
    end
end

script.Parent.MouseClick:Connect(onMouseClick(player, text))
2 Likes