RP Nametag Review

Help and Feedback Code Review
#1

Hiya! After some late-night coding, I more or less finished my RP Nametag feature. It allows you to set a (filtered) name, and change the font and colours from some pre-selected ones.
I still need to code for when the player resets/dies, but I want to get my code reviewed first before I do that.

Keep in mind, I’m VERY new to Roblox/Lua, and this is actually my first real project! Like, ever! So with that said, here goooeees!


Here is a video of the feature in action!

And here is the code of the Server script that handles it all;

-- StringValue for the Name
game.Players.PlayerAdded:Connect(function(player)
	local rpName = Instance.new("StringValue", player)
end)

-- When enter is pressed, create a Nametag and change it to the Name
game.ReplicatedStorage.SetRpName.OnServerEvent:Connect(function(player, name) 
	print(name)
	-- Filtering for Public Use
	if game:GetService('TextService'):FilterStringAsync(name,player.userId,Enum.TextFilterContext.PublicChat):GetNonChatStringForBroadcastAsync() == name then
		local rpName = name
		print(rpName)
		
		local head = player.Character.Head
		local usertag = game:GetService("ServerStorage"):WaitForChild("UserTag")
		local clonedtag = head:FindFirstChild("UserTag") or usertag:Clone()
		clonedtag.TextLabel.Text = rpName
		clonedtag.Parent = head
	 else 
		-- If Text is filtered, the Nametag will not update (tested)
		error('Text filtered! Original string: '..name..' filtered string: '..game:GetService('TextService'):FilterStringAsync(name,player.userId,Enum.TextFilterContext.PublicChat):GetNonChatStringForBroadcastAsync())
	end
end)

-- Changing the Font of the Nametag
game.ReplicatedStorage.SetFont.OnServerEvent:Connect(function(player, font)
	local nameFont = font
	local head = player.Character.Head
	local clonedtag = head:FindFirstChild("UserTag")
	clonedtag.TextLabel.Font = font
end)

-- Changing the colour of the nametag
game.ReplicatedStorage.SetColour.OnServerEvent:Connect(function(player, colour)
	local nameColour = colour
	local head = player.Character.Head
	local clonedtag = head:FindFirstChild("UserTag")
	clonedtag.TextLabel.TextColor3 = nameColour
	-- To compensate for Colour4 vvv
	clonedtag.TextLabel.TextStrokeColor3 = Color3.fromRGB(26,26,26)
end)

-- Changing the colour of the Nametag to Black w/ a White Stroke
-- Better way to do this?
game.ReplicatedStorage.SetColour4.OnServerEvent:Connect(function(player, colour)
	local nameColour = colour
	local head = player.Character.Head
	local clonedtag = head:FindFirstChild("UserTag")
	clonedtag.TextLabel.TextColor3 = nameColour
	-- Manual stroke colour below vv
	clonedtag.TextLabel.TextStrokeColor3 = Color3.fromRGB(243,243,243)
end)

Apart from anywhere I could clean the code up/make it less redundant - is there any better way to do the Fourth Colour option? Basically, unlike the other options, it sets a different TextStrokeColor3 value as well (white, to offset the black), however, I couldn’t really figure out how to send a third value through a RemoteEvent? Or if that’s even possible?
I don’t mind leaving it if it’s not really a concern, I probably won’t have anyone collaborating on the code, if it’s relevant.

Thanks for any pointers!

9 Likes
Need help making a Name Tag Customizer
#2

This is how you can do more than three parameters via remote events

Local
image

Server

image
image760×154 1.98 KB

#3

At some point, it’s going to get annoying sending each variable as it’s own parameter. You might forget what the order of each parameter is. And if you want to send some default values, it becomes even more annoying, as you may have to put nil for any properties you are not changing.

Try creating a table of all of the information from your name tag, and naming the keys based on what their values represent. For example:


local remoteFunction = game.ReplicatedStorage:WaitForChild("playerRequest_changeRoleplayName")

local roleplayName = {}
roleplayName.text = desiredText
roleplayName.textColor3 = desiredTextColor3
roleplayName.textStrokeColor3 = desiredTextStrokeColor3
roleplayName.font = desiredFont

local success, errorMsg = remoteFunction:InvokeServer(roleplayName)

You’ll want to use a RemoteFunction instead of a RemoteEvent so that the server can return information about your name (i.e. was it filtered, was there an error, etc.)

Be warned: this example will allow an exploiter to set whatever color they want for their name, even if it is not an approved color. To counter-act this, you may want to store a list of approved colors on the server, and ensure that the client is requesting a valid color. You also want to make sure you are always properly filtering text before sending it to other clients.

Then on the server:

local remoteFunction = game.ReplicatedStorage:FindFirstChild("playerRequest_changeRoleplayName")

function remoteFunction.OnServerInvoke(player, roleplayName)
     local text = roleplayName.text
     local textColor3 = roleplayName.textColor3
     local textStrokeColor3 = roleplayName.textStrokeColor3
     local font = roleplayName.font

    -- do your thing here

     -- let the client know there was an issue
     if textWasFiltered then
          return false, "whoops your text was illegal"
     end

     -- let the client know things went well
     return true
end
7 Likes
#4

Looks good good so far!

One thing I would modify: instead of creating a String Value on PlayerAdded, clone the usertag billboard directly to the head, then set ‘ResetOnSpawn’ to false. That way you’ll be using less Instances and can completely avoid repeating the same process for when a player respawns.

1 Like
#5

Thank you guys so much for the tips! I’ll go ahead and put these suggestions into place when I next work on it!

One small question; @berezaa I haven’t dabbled in sanity checks, what is the best method to do these? Should it be a table of permitted values in the main RP Name script, or somewhere else on the server? Thank you for any advice!

1 Like
#6

Always store stuff like that on the server - as long as you’re checking before it’s set, then it’s fine anywhere as long as the server has the final check on what is added.

#7

Something like a list of permitted values would be useful to store in a ModuleScript in ReplicatedStorage, so that it can be accessed by both the client and the server. The client uses it to know what options to display to the player and then the server can use it to validate requests.

#8

You have a number of Denial of Service vulnerabilities in this code. Remember that although event listeners run in a new thread, due to how expensive stack traces are to generate, exploiters can rapidly trigger errors that will cause all players in the server to crash. It’s important to make sure none of your remote handlers ever error. Even if it didn’t cause a security vulnerability, code in general should never be able to error without error handling + protected call mode.

Remember that exploiters can pass any (ok, some stuff can’t be serialized through remotes) values to any arguments, and the only argument you can trust is player.

Don’t assume that player.Character exists, it might be nil if there isn’t a character at the moment. Also, clonedtag may not exist, depending on the code that creates it.

An exploiter could pass nil to the name for setting the roleplay name, triggering an error when attempting to filter chat. You should also call TextService:FilterStringAsync() in all code in protected mode anyway, as remote web calls like these can error in general for any number of reasons.

1 Like