Scripting HelpersARCHIVE
A little piece of Roblox history. Preserved for everyone, one question at a time.Read about the archive →
COMMUNITY QUESTION

Damage over time not working?

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?

1 answer

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
--

This discussion is preserved as it was. New answers, comments, and votes are closed.