Review my padlock code checker


local codeVal = game.Workspace.Values.Code.Value
local camera = game.Workspace.Camera
local accessed = { ["1"] = false, ["2"] = false, ["3"] = false, ["4"] = false}
local codePressed = game:GetService("ReplicatedStorage"):WaitForChild("Events"):WaitForChild("CodePressed")
local openShutter = game:GetService("ReplicatedStorage"):WaitForChild("Events"):WaitForChild("OpenShutter")
local codeCam = game.Workspace:WaitForChild("SafeCodeCam")
local counter = 1
local plrGui = game:GetService("Players").LocalPlayer:WaitForChild("PlayerGui")
local codeGui = plrGui:WaitForChild("LeaveCodeGui")
local TS = game:GetService("TweenService")
local beep = game:GetService("ReplicatedStorage"):WaitForChild("AnimationObjects"):WaitForChild("beep")
local tweenToCam = TS:Create(camera, TweenInfo.new(2, Enum.EasingStyle.Quad, Enum.EasingDirection.InOut, 0, false), {CFrame = codeCam.CFrame})
local code = {}
local garageSound = game:GetService("ReplicatedStorage"):WaitForChild("AnimationObjects"):WaitForChild("garage door sound")
local camOn = false
local success = game:GetService("ReplicatedStorage"):WaitForChild("AnimationObjects"):WaitForChild("SuccessSound")
local errorSound = game:GetService("ReplicatedStorage"):WaitForChild("AnimationObjects"):WaitForChild("ErrorSound")
print(codeVal)

codeGui.Frame.Button.Activated:Connect(function()
	codeGui.Enabled = false
	camera.CameraType = Enum.CameraType.Custom
	camOn = false
end)

codePressed.OnClientEvent:Connect(function(buttonNo)
	if not camOn then
		camera.CameraType = Enum.CameraType.Scriptable
		tweenToCam:Play()
		tweenToCam.Completed:Connect(function()
			codeGui.Enabled = true
			camOn = true
		end)
	end
	for x, y in pairs(accessed) do
		if y == false and counter <= 4 and camOn then
			y = true
			beep:Play()
			local clone = game:GetService("ReplicatedStorage"):WaitForChild("CodeWorkspace"):WaitForChild("Numbers"):WaitForChild(buttonNo):Clone()
			clone.Parent = game.Workspace
			clone.CFrame = game.Workspace.CodePlacement:FindFirstChild(tostring(counter)).CFrame
			clone.Name = "numberclones"
			counter += 1
			table.insert(code, tonumber(buttonNo))
			break
		end
	end
end)

game.Workspace.VerifyCode.ClickDetector.MouseClick:Connect(function(plr)
	if counter == 5 then
		beep:Play()
		print(code)
		local str = ""
		if code ~= nil then
			for i, v in pairs(code) do
				str = str .. tostring(v)
			end
		end
		local newCode = tonumber(str)
		if newCode == codeVal then
			success:Play()
			openShutter:FireServer()
			codeGui.Enabled = false
			camera.CameraType = Enum.CameraType.Custom
			camOn = false
			garageSound:Play()
		else
			errorSound:Play()
			for i = 1, 4, 1 do
				game.Workspace:FindFirstChild("numberclones"):Destroy()
			end
			counter = 1
			table.clear(code)
		end
	end
	
end)

1 Like

We dont understand anything about how this is supposed to work, what it does and etc.

Please briefly explain how this works and what it does and please provide a video so we can see it in action.

This is pretty cool, but you should probably organize the variables. In addition, you should index by numbers on your dictionary:

You should probably find a place for the code value other than workspace so players can’t see it.

I don’t think the script is very secure.

1 Like

Thank you for your review, what do you mean by not putting it in workspace? Why would there be an issue with that?

Exploiters can find values in workspace no problem. You need to hard-code the code into the script so that it’s harder to find.

However, to make the code door secure, you need to put this on the server where the server checks the code, and the client handles the input.

1 Like

Can hackers change a value in the workspace?

Hackers can change anything on their end. This is why in client-server architecture, you let the server do as much of the verification work as possible and leave the client to do only non-critical things where possible, such as only accepting user input and not fully processing anything important with it (send that stuff to the server to calculate instead).

Changes made on the client will only reflect on that specific client, legal (i.e. through a LocalScript you wrote to actually do something) or not (i.e. through a hacker hacking). Assuming they’re not exploiting a game design flaw (e.g. you have a RemoteEvent with poorly designed code listening on it which they can leverage) nor something fundamentally broken with the Roblox game engine itself (nothing we can really do anything about directly), then any changes they hack in don’t affect other players.

Sorry for double-posting.

Had a look over your code now after writing my first reply, and I just wanted to say a few things.

First, what exactly is the purpose of this snippet?

    for x, y in pairs(accessed) do
		if y == false and counter <= 4 and camOn then
			y = true
			beep:Play()
			local clone = game:GetService("ReplicatedStorage"):WaitForChild("CodeWorkspace"):WaitForChild("Numbers"):WaitForChild(buttonNo):Clone()
			clone.Parent = game.Workspace
			clone.CFrame = game.Workspace.CodePlacement:FindFirstChild(tostring(counter)).CFrame
			clone.Name = "numberclones"
			counter += 1
			table.insert(code, tonumber(buttonNo))
			break
		end
	end

If you’re trying to display the numbers the user is punching in, you should just use a SurfaceGui with a TextLabel inside of it and manipulate that. Doing whatever this is with cloning Instances is a waste of memory and processing power for no useful gain.


Secondly:

local accessed = { ["1"] = false, ["2"] = false, ["3"] = false, ["4"] = false}
if code ~= nil then
    for i, v in pairs(code) do
	    str = str .. tostring(v)
    end
end
local newCode = tonumber(str)

Why are you working with strings for your code if it’s all numerals? Again, wasting memory and processing time for no gain converting numbers to strings then back to numbers.

Tables in Lua can have numeric indices. Just do:

local accessed = { [1] = false, [2] = false, [3] = false, [4] = false }

Or, since it technically counts as an array (i.e. a table with numeric indices in ascending order starting from 1), you could literally just do:

local accessed = {false, false, false, false}

In either case, it can be indexed (read from) like accessed[1], accessed[2], etc. No need to use strings. For putting the numbers of the code together, you could just do some simple arithmetic instead:

local newCode = accessed[1] * 1000 + accessed[2] * 100 + accessed[3] * 10 + accessed[4]

which looks complex to us as humans, but for a computer, this should be less resource-intensive than trying to put a variable-length string together and then reading it to spit out a number.


Lastly, I saw that codeVal is defined by some NumberValue or IntValue sitting somewhere in Workspace. I would personally just not have that and put the code itself directly into the script so it’s not just sitting somewhere where the client has access to it, then put the script in ServerScriptService if it isn’t in there already.

If you need it to be in a Number-/IntValue, I recommend you put it in ServerStorage. Nothing in ServerStorage replicates to the client, so any malicious user would have no way of being able to read the code directly from the game and would instead have to try to read it from the server’s memory somehow.