local cooldown = 10 local hurt = true local loss = 5 function onTouched(part) local humanoid = part.Parent:FindFirstChild("Humanoid") local currentHealth = part.Parent:FindFirstChild("Humanoid").Health if hurt == true then if part.Parent:FindFirstChild("Fire") == false then local fire = Instance.new("Fire") fire.Parent = part.Parent if fire.Parent == part.Parent then hurt = false wait(cooldown) hurt = true while true do if fire.Parent == part.Parent then currentHealth = currentHealth - loss wait(1) end end end end end end script.Parent.Touched:connect(onTouched)
Just getting into scripting, and so I tried to do something a wee-bit harder. I wrote this script so whenever you are on fire, you take damage, but it doesnt seem to be working?
There's a lot of problems with this script. Slow down, and test lots. Understand what you're doing before you get to 20 lines of wrong code.
First: tab your code properly. It makes it much easier to understand.
You never use the humanoid
local variable.
In fact, you should first check that it exists. If it doesn't (e.g., if some random brick touches this) then the script will break when you try to access humanoid.Health
.
I would recommend not using the word hurt
since it's not really clear when you mean. canHurt
would be a much better name. I also suggest quitting early with a return
instead of wrapping the whole thing in an if canHurt then
, to make your script a little shorter and more compact.
function onTouched(part) if not canHurt then return end local humanoid = part.Parent if not humanoid then return end
Similarly, you should quit if there is already a Fire to avoid more indentation.
Note: FindFirstChild does not return false
, it returns nil
in the absence.
In general, you should not use == false
, == nil
, or == true
. You should just use the thing itself, or not
when appropriate:
if part.Parent:FindFirstChild("Fire", true) then return end --
I'll explain the , true
in a bit.
Next, make your thing like you did before. But there's no reason to check it's parent -- you just set it. It can't not be set.
Instance.new
takes a second parameter: the parent to set it in.
From the Wiki:
The fire object must be placed inside of a BasePart class in order to render, and the particles are affected by several properties.
That means we should be putting it in a part, not the model. To make sure that none of the model is on fire, we use :FindFirstChild("Fire", true)
to search in all objects as well.
local fire = Instance.new("Fire", part) --
Next, disable canHurt
as before.
Presumably, you want damage to be done during the cooldown.
You can use spawn
to start a function "in the background". This means it never has to finish, but the rest of the script can go on.
Instead of a while true
loop that will never end, move the condition of your if
into the while
(we don't expect the fire to come back once it leaves, so this makes more sense).
Modifying the variable currentHealth
just modifies currentHealth
. It does not change the humanoid
because you did not tell it to change the humanoid. (The earlier assignment just copies the number, say, 100, to the variable -- it does not somehow "link" the variable and property)
canHurt = false -- Start damage loop spawn(function() while fire.Parent == part do wait(1) humanoid.Health = humanoid.Health - LOSS end end) --
Next re-enable canHurt
after a few seconds.
It's a general programming style to use ALL_CAPS for 'constants' that you use to configure your scripts.
wait(COOLDOWN) canHurt = true end --