local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")
local voting = script.Parent
local padsFolder = voting.Pads
local votingPadValues = ServerStorage.VotingPadValues
local function updateText(votingPad, voteText)
voteText.Text = votingPadValues[votingPad.Name].Value
end
for _, votingPad in ipairs(padsFolder:GetChildren()) do
local voteText = votingPad.VoteScreen.Votes.SurfaceGui.Text
if votingPad ~= nil then
votingPad.Touched:Connect(function(hit)
local player = Players:GetPlayerFromCharacter(hit.Parent)
if player then
local voted = ServerStorage[player.Name].Other:FindFirstChild("Voted")
if voted.Value ~= votingPad then
votingPadValues[votingPad.Name].Value +=1
if voted.Value ~= "N/A" then
local previousPad = votingPadValues:FindFirstChild(voted.Value)
votingPadValues[previousPad.Name].Value -= 1
end
voted.Value = votingPad.Name
end
end
end)
votingPadValues[votingPad.Name].Changed:Connect(function()
updateText(votingPad, voteText)
end)
end
end
Edits: The voting values of each pad are now stored inside a folder in ServerStorage and Player Voted Value as well, benefits in a bit better security and low memory.
I think my main concern really is the fact you’ve used the Player itself to store values in place of doing it through the script itself.
You could easily do other methods such as storing it in a dictionary, with the pads either acting as ways to set the players choice (this would also enable you to detect accurately when a player leaves the proper tally number) or as vote platforms which tally.
Second MAJOR concern.
Logically speaking, every time Pad.Votes changes you create a new thread - that new thread then creates another three new connections as seen in the updateText, eventually this will slow down your game as the connections aren’t ever disconnected and they are all firing the same thing. By simply running updateText once you’d get the same implied logic as before.
I personally think player would be better since there would be no use of dictionaries as it’s only 1 value. I’ve made changes to the script, thanks for letting me know.
Just use arrays, the array will keep the data a bit more secure and help keep memory usages down. Creating the dictionary/array really isn’t an added level of complexity. Just go for it.
I use client to server communication with RemoteEvent/RemoteFunctions to manage the vote counts in table, i put sanity check into my voting system to make sure.
Not sure how it would be a security concern, if the player changes his value locally it would not affect the server at all. The only benefit is less memory, however one thing you have to do is manually remove the key of the player when he leaves the game, while storing an value inside the player would not require this since the player object is destoryed once he leaves
It depends on the system of input, technically any exploiting is a “security concern” - in this case its not much of a “concern” but more so a chance of data visualization being incorrect or things not being presented correctly if done client side (depends how he manipulates and uses it).