How to Improve an Object Oriented Fire-Spreading Script

[lgregvcxs]

I made a fire-spreading script a few months ago, and I’ve recently came back to it to completely rewrite the whole thing, specifically using OOP.

I was able to turn this undecipherable string of seemingly random words and numbers:

local tag = game:GetService("CollectionService")
local Tofiresever = game.ReplicatedStorage:WaitForChild("Tofiresever")
local spreadrate = 1
local keepchecking = false
Tofiresever.OnServerEvent:Connect(function(p,target)
	if #tag:GetTags(target) == 0 then
		tag:AddTag(target,"OnFire")
		local fire = Instance.new("Fire")
		fire.Size = (target.Size.X + target.Size.Y + target.Size.Z)/3
		fire.Parent = target
	end
end)

while wait(spreadrate) do
	for _, v in pairs(tag:GetTagged("OnFire")) do
		if v == nil then return end
		
		local tic = v:FindFirstChild("Ticked")
		if tic then
			if tick() - tic.Value > 3 then
				v.Color = Color3.fromRGB(0,0,0)
				local burnt = Instance.new("IntValue")
				burnt.Name = "burnt"
				burnt.Parent = v
				if v.Anchored then
					v.Anchored = false
				end
				tag:RemoveTag(v,"OnFire")
				for _, got in pairs(v:GetChildren()) do
					if got.Name ~= "burnt" then
						got:Destroy()
					end
				end
			end
		else
			local ticked = Instance.new("IntValue")
			ticked.Name = "Ticked"
			ticked.Value = tick()
			ticked.Parent = v
		end
		
		local ishuman = v.Parent:FindFirstChild("Humanoid")
		if ishuman then
			ishuman:TakeDamage(5)
		end
		if v.Anchored then
			for _, hit in pairs(workspace:GetPartsInPart(v)) do
				local hitTags = tag:GetTags(hit)
				local isflamable = hit.Material == Enum.Material.Wood or hit.Material == Enum.Material.WoodPlanks or hit.Parent:FindFirstChild("Humanoid")
				if hitTags and #hitTags == 0 and isflamable and hit:FindFirstChild("burnt") == nil then
					tag:AddTag(hit, "OnFire")
					local fire = Instance.new("Fire")
					fire.Size = (hit.Size.X + hit.Size.Y + hit.Size.Z)/3
					fire.Parent = hit
				end
			end
		else
			local firespreaded
			firespreaded = v.Touched:Connect(function(hit)
				if hit == nil then return end
				local hitTags = tag:GetTags(hit)
				local isflamable = hit.Material == Enum.Material.Wood or hit.Material == Enum.Material.WoodPlanks or hit.Parent:FindFirstChild("Humanoid")
				if hitTags and #hitTags == 0 and isflamable and hit:FindFirstChild("burnt") == nil then
					tag:AddTag(hit, "OnFire")
					local fire = Instance.new("Fire")
					fire.Size = (hit.Size.X + hit.Size.Y + hit.Size.Z)/3
					fire.Parent = hit
				end
				wait(1)
				firespreaded:Disconnect()
			end)
		end
	end
end

Into this slightly-more-decipherable string of seemingly random words and numbers:

local TS = game:GetService("TweenService")
local FireHandler = {}
FireHandler.__index = FireHandler

function FireHandler.Ignite(obj,burningtime,displacement,chance)
	local self = setmetatable({
	},FireHandler)

	self.obj = obj
	self.burningtime = math.random(burningtime-displacement,burningtime+displacement)
	self.Spread = function()
		while self.obj.Parent ~= workspace.Burnt do
			if self.obj.Parent == workspace.Burnt then return end
			task.wait(1)
			local dup = self.obj:FindFirstChild(self.obj.Name.." Fire Hitbox")
			if dup == nil then
				if self.obj.Parent == workspace.Burnt then return end
				dup = self.obj:Clone()
				dup.Name = self.obj.Name.." Fire Hitbox"
				dup.CFrame = self.obj.CFrame
				dup.Size = Vector3.new(self.obj.Size.X+1,self.obj.Size.Y+1,self.obj.Size.Z+1)
				dup.Material = Enum.Material.Plastic
				dup.Transparency = 1
				dup.CanCollide = false
				dup.Anchored = true
				dup.Parent = self.obj
			end
			if self.obj.Parent == workspace.Burnt then return end
			for _, hit in pairs(workspace:GetPartsInPart(dup)) do
				if self.obj.Parent == workspace.Burnt then return end
				local random = math.random(1,100)
				if random >= chance then continue end
				local isflamable = hit.Material == Enum.Material.Wood or hit.Material == Enum.Material.WoodPlanks or hit.Parent:FindFirstChild("Humanoid")
				if isflamable and hit.Parent ~= workspace.Burnt and hit.Parent ~= workspace.Burning then
					FireHandler.Ignite(hit,burningtime,displacement,chance)
				end
			end
		end
	end

	local fire = Instance.new("Fire")
	fire.Parent = obj

	obj.Parent = workspace.Burning
	
	local info = TweenInfo.new(self.burningtime,Enum.EasingStyle.Linear,Enum.EasingDirection.In)
	local prams = {Color = Color3.fromRGB(0,0,0)}
	local tween = TS:Create(self.obj,info,prams)
	tween:Play()

	local function Extinguish()
		for _, part in pairs(self.obj:GetChildren()) do
			if part:IsA("Weld") or part:IsA("WeldConstraint") or part:IsA("Fire") or part == self.obj:FindFirstChild(self.obj.Name.." Fire Hitbox") then
				part:Destroy()
			end
		end
		self.obj.Anchored = false
		self.obj.Color = Color3.fromRGB(0,0,0)
		self.obj.Parent = workspace.Burnt
	end

	task.spawn(self.Spread)
	task.delay(self.burningtime,Extinguish)
	return self
end

return FireHandler

I was wondering if any improvements can be made in this new script (the second one, if you were wondering), and if there is anything unnecessary within it. The main thing that I’m wondering about is if returning the self variable is even necessary, or if using OOP is necessary, especially when I call the function like this:

Fire.Ignite(object,burningtime,displacement,chance) 

There aren’t any variables being defined by the function, and when I tested it out without returning the self variable, it worked just fine. I also want to know if any improvements can be made concerning OOP, and also concerning performance, since burning up whole buildings can be pretty lag. Thank you in advance .

(PS - I would have included a compilation of me burning random stuff, but Roblox Studio lags way to hard for me to do so lol.)

[Men1os (men1osUGC)]

And it really looks strange, Unclear

[lgregvcxs]

Could you please elaborate? What’s unclear? What looks strange? I am very confused here.

[PhoenixRessusection (PhoenixCausesOof)]

Yeah, OOP seems pretty unnecessary here. In the future, you should look for simpler approachs, such as returning a single function from this ModuleScript:

return function(object, burnTime, displacement, chance)
    ...
end

One thing I noticed was that you were frequently checking if the burn effect had finished (if self.obj.Parent == workspace.Burnt then return end). I think that’s needless in this case because the code doesn’t require that kind of microsecond precision.

Also, instead of using the object’s parent to determine if the effect has ended or not, you could use an attribute as such:

while not obj:GetAttribute("Burnt") do
    ...
end
...
task.delay(burningTime, function()
	-- Order matters: we want to stop the code from executing before destroying remaining parts
	obj.Anchored = false
	obj.Color = Color3.fromRGB(0,0,0)
	obj:SetAttribute("Burnt", true); -- any value other than nil will stop the loop

	task.wait(); -- Might be unnecessary, but I thought it would be useful to give it some time to process
	for _, part in obj:GetChildren() do -- pairs syntax is optional
		if part:IsA("Weld") or part:IsA("WeldConstraint") or part:IsA("Fire") or 
			string.find(part.Name, "Fire Hitbox", 1, true) then
			part:Destroy()
		end
	end
end)

Hope this helps somehow, if you have any questions, feel free to ask.

[lgregvcxs]

Thank you so much for the information! I’ll be sure to implement your suggestions into the module.