Repetitve Code (DRY)

Help and Feedback Scripting Support
1

I’ve made portals that gives buffs to players whenever they are touched and I’ve created this script for a Max Health Buff portal but I feel like I’ve been reptitively using this script too much. I’m only modifying a few things just for the script to work. I’m trying to learn and use module scripts to avoid repeatedly using the same script over and over but I don’t know how.

-- This is the script I've been repeatedly using

local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local SoundService = game:GetService("SoundService")

local MainFrame = script.Parent

local Sounds = ReplicatedStorage:WaitForChild("Sounds")
local Buff_Sound = Sounds:WaitForChild("dio_heal")
local Error_Sound = Sounds:WaitForChild("Windows XP Error")

local Health_Owner = script:WaitForChild("HealthOwner")
local Health_Time = script:WaitForChild("HealthTime")

local Modules = ReplicatedStorage:WaitForChild("Modules")
local Portal_Modules = Modules:WaitForChild("Portal_Modules")
local Random_FFA_Spawn = require(Portal_Modules:WaitForChild("Random_FFA_Spawn"))

local Owner_Character = nil
local COOLDOWN_IS_RUNNING = false
local debounce = false

local function Cooldown()
	if COOLDOWN_IS_RUNNING == true then return end -- Prevent from running if it's already running
	COOLDOWN_IS_RUNNING = true
	
	while COOLDOWN_IS_RUNNING == true and Health_Time.Value > 0 do
		if COOLDOWN_IS_RUNNING == false then break end
		Health_Time.Value -= 1
		task.wait(1)
	end
	
	if Health_Time.Value <= 0 and Owner_Character ~= nil then
		local Humanoid = Owner_Character:WaitForChild("Humanoid")
		local HRP = Owner_Character:WaitForChild("HumanoidRootPart")
		
		if Humanoid and HRP then
			Humanoid.MaxHealth = 100
			HRP:FindFirstChild("Health Sparkes"):Destroy()
		end
		
		Health_Owner.Value = "nil"
		Owner_Character = nil
	end
	
	COOLDOWN_IS_RUNNING = false
end

MainFrame.Touched:Connect(function(hit)
	if debounce == true then return end
	debounce = true
	
	if hit.Parent:FindFirstChild("Humanoid") then
		local character = hit.Parent
		local Humanoid = character:FindFirstChild("Humanoid")
		local HRP = character:FindFirstChild("HumanoidRootPart")
		
		if Health_Owner.Value == "nil" and Health_Time.Value <= 0 and not HRP:FindFirstChildOfClass("Sparkles") then
			Humanoid.MaxHealth = 200
			Health_Owner.Value = character.Name
			Health_Time.Value = 180
			Random_FFA_Spawn.Random_Spawn(character)
			
			local Health_Sparkles = Instance.new("Sparkles")
			Health_Sparkles.Name = "Health Sparkles"
			Health_Sparkles.SparkleColor = Color3.fromRGB(0, 255, 0)
			Health_Sparkles.Parent = HRP
			Owner_Character = character
			
			local Buff_Sound_Clone = Buff_Sound:Clone()
			Buff_Sound_Clone.Parent = HRP
			Buff_Sound_Clone:Play()
			Buff_Sound_Clone.Ended:Wait()
			Buff_Sound_Clone:Destroy()
		else
			Random_FFA_Spawn.Random_Spawn(character)
			
			
			local Error_Sound_Clone = Error_Sound:Clone()
			Error_Sound_Clone.Parent = HRP
			Error_Sound_Clone:Play()
			Error_Sound_Clone.Ended:Wait()
			Error_Sound_Clone:Destroy()
		end 
	else
		error("Failed to find Humanoid")
	end
	
	task.wait(1)
	debounce = false
end)

Players.PlayerAdded:Connect(function(player)
	player.CharacterAdded:Connect(function(character)
		local Humanoid = character:WaitForChild("Humanoid")

		Humanoid.Died:Once(function()
			if player.Name == Health_Owner.Value then
				COOLDOWN_IS_RUNNING = false
				Health_Time.Value = 0
				Health_Owner.Value = "nil"
				Owner_Character = nil
			end
		end)
	end)
end)

Players.PlayerRemoving:Connect(function(player)
	if player.Name == Health_Owner.Value then
		COOLDOWN_IS_RUNNING = false
		Health_Time.Value = 0
		Health_Owner.Value = "nil"
		Owner_Character = nil
	end
end)
2

Can anybody give me some ideas for some functions to store in the module script so I can avoid not repeating code over and over? :thinking:

3

Why do you need that
You better have everything in the stack so you have dirrect fast access.
You can wrap some code into functions but as far as i see you use closures a lot so that not a great solution.
Avoid any high ego JS skid who know nothing he is talking about from giving you bad ideas
As long as code is optimized and works well there is nothing bad in repeating it.
Coding standarts should not influence quality of your code.

4

Just a thought, but you could have a script that simply clones your script into where it’s required, just to prevent having to manually edit them one-by-one.

if game:IsLoaded() == false then
	game.Loaded:Wait()
end
local PortalScript = <path>
local WhereTheyGo = <path>

for _, obj in ipairs(WhereTheyGo:GetChildren()) do
	local NewPortalScript = PortalScript:Clone()
	NewPortalScript.Parent = obj
	NewPortalScript.Disabled = false
end
5

I don’t know what that means? :thinking:

6

I think the perfect place where you could start using module scripts is by building a cache.

This is beginner level.

Define the structure by making a type. This is your blueprint.

local DEFAULT_MAX_CACHE = 3

export type CacheMap = {
	Store: (self: CacheMap) -> (),
	Retrieve: (self: CacheMap) -> Instance,

	_Instance: Instance,
	_cache: { Instance? },
	_max_cache: number,
}

Write a simple class & constructor.

local module = {}
module.__index = module

function module.new(Object: Instance, max_cached: number?): CacheMap
	local self = setmetatable({}, module)
	self._Instance = Object
	self._cache = {}
	self._max_cache = max_cached or DEFAULT_MAX_CACHE

	return self
end

return {
	new = module.new,
}

Handle storing of Instance’s. I parent all to the script, which is usually in the ReplicatedStorage, outside of render.

function module.Store(self: CacheMap, Object: Instance)
	if #self._cache < self._max_cache then
		table.insert(self._cache, Object)

		Object.Parent = script
	else
		Object:Destroy()
	end
end

Handle how you get Instance’s from the cache. Get one from the cache & remove it, filling down all other cached Instance’s. If none exist, create one.

function module.Retrieve(self: CacheMap): Instance
	local CachedObject = self._cache[1]

	if CachedObject then
		table.remove(self._cache, 1)
	end

	return CachedObject or self._Instance:Clone()
end
Cache 
local DEFAULT_MAX_CACHE = 3

export type CacheMap = {
	Store: (self: CacheMap) -> (),
	Retrieve: (self: CacheMap) -> Instance,

	_Instance: Instance,
	_cache: { Instance? },
	_max_cache: number,
}

local module = {}
module.__index = module

function module.Store(self: CacheMap, Object: Instance)
	if #self._cache < self._max_cache then
		table.insert(self._cache, Object)

		Object.Parent = script
	else
		Object:Destroy()
	end
end

function module.Retrieve(self: CacheMap): Instance
	local CachedObject = self._cache[1]

	if CachedObject then
		table.remove(self._cache, 1)
	end

	return CachedObject or self._Instance:Clone()
end

function module.new(Object: Instance, max_cached: number?): CacheMap
	local self = setmetatable({}, module)
	self._Instance = Object
	self._cache = {}
	self._max_cache = max_cached or DEFAULT_MAX_CACHE

	return self
end

return {
	new = module.new,
}

This makes instance management very simple and improves performance.

PartCache, for all your quick part-creation needs - Resources / Community Resources - Developer Forum | Roblox

7

WOAH. I’m just learning how to just store functions and variables & calling the module scripts. I don’t know what self, index, and cache thing means

8

You also reference SoundService which will almost always be used again.

You can write a ModuleScript with methods like PlayInPart or Play to further simplify how you use audio.

local EffectGroup = game:GetService("SoundService"):FindFirstChild("Effects") or error(`No "Effects" Folder in SoundService.`)

local soundMap = {}

local function GetRandomSound(category: string): Sound
	local soundArray = soundMap[category]

	return soundArray[math.random(1, #soundArray)]
end

function PlayInPart(category: string, Part: BasePart)
	local Sound = GetRandomSound(category)
	local ClonedSound = Sound:Clone()

	ClonedSound.Parent = Part
	ClonedSound.Ended:Once(function()
		Sound:Destroy()
	end)

	ClonedSound:Play()
end

function Play(category: string)
	local Sound = GetRandomSound(category)

	Sound:Play()
end

for _, Folder in EffectGroup:GetChildren() do
	soundMap[Folder.Name] = Folder:GetChildren()
end

return {
	PlayInPart = PlayInPart,
	Play = Play,
}
9

if speaking about caching why dont just straight up return constructor as a function?
That literally just makes life easier and better

10

I don’t know what a constructor is :frowning:

11

he is maching a singular table (that acts as a metatable aswell) and returns that even through it only has one value
That makes no sense becouse constructor (OOP object maker) can be returned dirrectly and be used instantly instead of having to index it inside a table.

12

I’ve heard of OOP and metatable but barely know anything about it

13

OOP as it is doesnt exist in Luau at all
There is metatable OOP aproach (the one person above me had used), closure OOP, C like OOP (pointer OOP),full C OOP (buffer hell) and that basically it.
Metatable is a custom behavior for tables or userdata, it may sound cool on papper but honestly its more of toying with code and it hits optimization pretty severely i’d say if you use it everywhere.There so it doesnt even worth bothering to learn metatables IMO but if you want to 100% Luau then go for it.

14

I recommend you watch some videos on the basics of Lua.
For this I’ll give a brief rundown.

From the top, I specified a type and called it “CacheMap” which is a dictionary that has two methods.

Types are related to type checking which enable auto completion inside the module script but also outside of it. When you require, you can use any exported types.

self is a common variable used with metatables. By saying that self will return CacheMap It’s allowing you to reference the entirety of CacheMap inside of the function.

To make a normal table into a metatable, you have to define module.__index = module. This will merge the methods tied to module into metamethods inside the metatable.

I used all of that to make a function that stores objects and one that retrieves them from storage.

Unfortunately, I am not experienced enough to teach you the full spectrum of how metatables work. The way I use them works for me, but I know it’s not the best way to do it.

There are a ton of resources around YouTube if you’re interested.

15

I will try to learn more about this! :smiley:

16

For me object-oriented-programming totally revolves around constructors.

Visually I find that classes are a lot easier to use and manage. Rather than returning an Instance, you can return a set of functions that dictate behavior.

This approach is seen in Roblox’s API as well. Instance’s have methods, properties, all of which resemble metatable’s.

Really, it’s all about preference. Lua is such an easy language to get around and Roblox manages everything behind the scenes. There’s really nowhere you can go wrong. It almost invites you to play around and try different approaches to common problems in coding.

17

There is alternative OOP (stricly typed OOP)
It should be less confusing and more perfomant than metatable OOP

18

For me that doesnt metter
It adds more complexity and makes worse perfomance
Dont compare Luau globals to your code
GETIMPORT does not get called on your code.
There so return only what you need.
You only have 1 constructor then return only that, no need of table.

19

so first off, you should def use modularized codebase from now on, just try learning it asap since it adds a ton of readability and your scripts will become more universal. Anyways, as you might already now, module scripts can return only 1 value when required but it can also be a table thats why when you create a new module you will see

local module = {}

return module

so now how can we use it so we dont have to repeat our code again and again?
There are many ways how you can do this but ill just give an example what i would do.
This might be not related at all to your script but lets say i have 2 portals, 1 that gives me health boost and one that gives me idk speed boost so in this module we can put a function which will give boosts to player

local PortalModule = {}


function PortalModule.GiveBuffs(plr, buff)

end

return PortalModule

so after this in the other script we can just do

local someModule = require(<pathOfModule)
local buff = touchedPart:GetAttribute("BuffType")
someModule.GiveBuffs(plr, buff)

something like that, basically module scripts are like normal scripts but you dont need to use bindable events, functions, you can think like that xd
ah and btw someModule is a variable for value that u get from requiring a module so in this case its a table with a function inside of it

20

You need to unwrap functions after requiring to not hurt perfomance:

local someModule = require("./Module")
local GiveBuffs = someModule.GiveBuffs