Hello, I am creating an Obby game I wanted to know if this script that makes touching a block kill you is well done:
function onTouched(part)
local h = part.Parent:findFirstChild("Humanoid")
if h~=nil then
h.Health = h.Health-100
end
end
script.Parent.Touched:connect(onTouched)
Looks good, findFirstChild should be FindFirstChild - Capital F at the beginning.
Also if the Script is intended to kill then you could just do h.Health = 0
But yeah looks fine 
NOTE: FindFirstChild will work fine with lower case f, more of just a personal preference to use uppercase
Looks good, nothing much to say 
I was going to say maybe you want to add a debounce or disconnect the event, but I don’t think you would need this in your case.
Overall, no issues!
Hi!
My only critique is to add a debounce; though, this being a kill script, that’ll probably controversial among developers since the player will be dead anyways. Basically, a debounce helps prevent a player from touching something more than once.
Other than that, looks great! I recommend going to #help-and-feedback:code-review the next time you would like to have your code reviewed
Actually, a debounce prevents the players to repeat the script like 6 times in 0 seconds.
So does my script need a bounce?
yeah if you have a specific line in it that is buggy when done multiple times also it’s debounce
Will adding the bounce make the game less lag? Is that sometimes players touch the block but do not die instantly
nothing will change but it will be specially useful later on, debounces are like a bool that most people set true then make an if statement if that debounce is set true then after that they set the debounce to false then after some waits it will be set back to true
It wouldn’t reduce lag and you probably don’t need one in this case since the player would be dead. Its more for if the player is going to be able to touch it multiple times in one life.
It is that when I touch the block it takes up to 2 seconds to kill me, and it should be instantaneous
The script is fine but just a few things you should change.
Use :Connect() and not :connect as :connect() is deprecated (basically no longer “updated/maintained” by roblox and not recommended to use).
You should indent your code so its easier to read (unless its just the post format thats making it look not indented)
This is optional but along with your h ~= nil if statement. You could also check to see if the player’s health isn’t already 0. E.g
if h ~= nil and h.Health > 0 then
-- damage and do whatever
Asi esta bien?
script.Parent.Touched:Connect(function(h)
if h.Parent:FindFirstChild('Humanoid') then
h.Parent:WaitForChild('Humanoid').Health -= 100
end
end)
Yep, thats better but you could reference the humanoid instead of having to constantly do h.Parent:FindFirstChild(“Humanoid”)
E.g
script.Parent.Touched:Connect(function(Hit)
local Humanoid = Hit.Parent:FindFirstChild("Humanoid") -- Reference the humanoid so we can use it throughout the rest of the function, that way we don't have to keep doing "Hit.Parent.Humanoid"
if Humanoid and Humanoid.Health > 0 then
Humanoid.Health -= 100
end
end)
Could using the old script cause lag in my game?
No, you aren’t doing anything performance heavy that would cause the client or server to lag.
I have like 1500 cheats with that old script. Still nothing will happen?
It should be fine as there are/were many front page obbys that use the exact same script but have like 5000 kill parts, etc.
However, it might be annoying to have 2000 scripts in every single part (just for editing reasons), so what I recommend is putting all your “killParts” into a folder and loop through that folder, then connect all of them to the touch event.
Or you could use collection service and tag certain things you want to kill:
If your game is lagging however, then try going to the dev console or microProfiler to see what is causing the issues.
This is a really minor thing, but since you’re asking for code review, I’d stay away from single-character variable names. All variable names should be entirely self-explanatory; if you see a variable name, you shouldn’t have to search the code to find out what it is.
So first, the lines of code are quite a bit too far spaced out and the formatting isn’t great. If you wanna see what “good” formatting (according to most scripters) looks like, you can use the format tool on Roblox Studio on the top of the screen when having a script open, or just look around on the Roblox Developer API. But that’s just something we all have to improve on over time.
In the above, like @RBX_Lua said, it’s likely better to use more recognizable variable names in situations like this. In this case, if you still want it to be short, you could use hum instead of the longer humanoid, but it’s much more understandable than h either way. Of course, that’s personal preference. But in the future, if you have, let’s say, 20 variables, you probably won’t have a fun time working with 1-letter variable names… 
Also, I assume that findFirstChild with the lowercase f works, but it’s more typically used as FindFirstChild, with the first “F” capitalized (small nitpick).
So this is a smart idea to check if it even exists - nice job. But when we think about readability and having our code be easy to understand, there’s a couple better ways to do this same thing. Isn’t “~=nil” the same thing as “== true”? Going further, a helpful shortcut is instead of doing if something == true then or if something == false then, you can simply do if something then, and if not something then. To be clearer, here’s it layed out:
if something == true then
if something then -- Same as "== true" shown above
if something == false then
if not something then -- Same as "== false" shown above
So with that being said, if h~=nil then can be redone to become, if h then.
This will work just fine, and it can be shortened to h.Health -= 100 using the wonderful compound operators (+=, -=, *=, /=, etc.). However, I personally would prefer to do it the most explicit way possible, which would be setting the humanoid’s health to 0. This way, in case there’s some sort of glitch with the health going over 100 somehow, or whatever happens, it explicitly says to rid of all of the humanoid’s health. So, you could do it like this: h.Health = 0. Not a big deal, but something I’d personally feel safer doing.
When it comes to everything else, it’s just fine. So putting that all together, and doing some “better” formatting, here’s the result:
function onTouched(hit)
local hum = hit.Parent:FindFirstChild("Humanoid")
if hum and hum.Health > 0 then
hum.Health = 0
end
end
script.Parent.Touched:Connect(onTouched)
Like @Ze_tsu said, you should use :Connect() instead of :connect(). I changed a couple of capitalizations, and switched part to hit purely out of preference.
I hope this helps!