What does the code do and what are you not satisfied with?
Hello developers! I have a system where the player collects rings and when they touch the rings, the counter gui goes up by 1. Is there anything that I should be doing differently?
What potential improvements have you considered?
Storing the IntValue somewhere else other than ReplicatedStorage, so that exploiters cannot abuse it.
How (specifically) do you want to improve the code?
The IntValue for the rings is in ReplicatedStorage, so it might be vulnerable to exploiters. Should I be changing values on the server or on the client? The ring value will not appear the same for everyone, so is changing it on the server a good idea?
Server Script
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RemoteEvents = ReplicatedStorage:FindFirstChild("RemoteEvents")
local RingEvent = RemoteEvents:WaitForChild("RingEvent")
local CollectionService = game:GetService("CollectionService")
local Rings = CollectionService:GetTagged("Ring")
for i, theRing in pairs(Rings) do
if theRing:IsA("BasePart") then
local debounceTable = {}
theRing.Touched:Connect(function(Hit)
local player = Players:GetPlayerFromCharacter(Hit.Parent)
if player then
if not debounceTable[player] then
debounceTable[player] = true
RingEvent:FireClient(player, theRing)
end
end
end)
end
end
Local Script
local Player = game.Players.LocalPlayer
local PlayerGui = Player.PlayerGui
local RingCounter = PlayerGui.RingCounter
local Rings = RingCounter.Rings
local Character = Player.Character or Player.CharacterAdded:Wait()
local Humanoid = Character:WaitForChild("Humanoid")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local RemoteEvents = ReplicatedStorage:FindFirstChild("RemoteEvents")
local RingEvent = RemoteEvents:WaitForChild("RingEvent")
local Values = ReplicatedStorage:FindFirstChild("Values")
local Integers = Values:FindFirstChild("Integers")
local RingValue = Integers:WaitForChild("RingValue")
local Sounds = script:WaitForChild("Sounds")
local RingCollected = Sounds:WaitForChild("RingCollected")
local Debris = game:GetService("Debris")
local TweenService = game:GetService("TweenService")
Humanoid.Died:Connect(function()
if RingValue.Value > 0 then
RingValue.Value = 0 -- reset the ring counter.
end
end)
RingEvent.OnClientEvent:Connect(function(theRing)
local ringThread = coroutine.wrap(function()
RingCollected:Play()
local transparencyTween = TweenService:Create(
theRing,
TweenInfo.new(0.5),
{Transparency = 1}
)
transparencyTween:Play()
transparencyTween.Completed:Wait()
theRing.CanTouch = false
end)()
RingValue.Value += 1
Rings.Text = "Rings: "..tostring(RingValue.Value)
theRing:WaitForChild("Effect"):WaitForChild("Collected"):Emit(20)
Debris:AddItem(theRing, 0.5)
end)
I’d highly recommend making any changes to values (Ex: RingValue.Value) be handled strictly by the server. The client should just handle visual effects, IE: Updating UIs, etc.
Yeah, I reworked the script. What I did was I made a folder called DataFolder inside the player when they joined, put the RingValue their, and when they touched a ring, increased their value by 1. Then I use :GetPropertyChangedSignal() to update the counter on the client.
So now, should exploiters be able to tinker with the RingValue as much? Now it is in a safer place, rather than ReplicatedStorage.
It’s not really in any safer of a place, the client is able to see the service. Server Storage would be the way to completely prevent the client from see it. However, this isn’t necessary. What you’re doing is fine, even if an exploiter was to mess with the values, it wouldn’t matter, it’d be purely visuals. As long as, any value validation is being handled on the server.
If you’re using the “RingValue” as a currency of sorts, or a method to track player leaderboards, it needs to be on the server. Never trust the client. If you were to have the client handle values, you’d have to let the server know of any changes, which can be done by either a RemoteEvent or a RemoteFunction. However, these can be fired by exploiters, and they can input their own values for any given argument. Whereas, if we let the server be the authority, we have one less vector to worry about from a security perspective. Plus, if you’re using the Property Changed signal, it will still fire on the client from any server changes. Allowing the client to handle their own visuals and removing the need to fire any remote classes.
Yes, when I had the value in ReplicatedStorage. Let’s say I’m an exploiter. I give myself 100 rings. Then the value would be now 105. But when the value was in the player’s folder, and I gave myself 100 rings, and then touched a group of rings, it just became 5.
This is because the server doesn’t see these changes, and it just updates the counter with whatever value is on the server.
Any changes made on the client no matter where it is, is never replicated to the Server, unless otherwise specified. You can store the Value in ReplicatedStorage and it wouldn’t make a difference as long as the Server is the one in control.