Code duplicity for multiple parts with ModuleScripts

Help and Feedback Code Review
,
#1

Hi, I’ve made a simple unlock mini puzzle, where the game generates a secret 4 digit number, and it lights up the blocks in that order. Here’s a short video demonstration:

Here’s the structure and how the logic works:
image

Each blue pad (Unlocker1, Unlocker2, etc.) contains a ModuleScript which fires an event whenever it is touched by a character:

local onTouch1 = {} -- it is named onTouch2, onTouch3 for each pad and so on. The code is exactly the same.

local bindableEvent = Instance.new("BindableEvent")
onTouch1.Hit = bindableEvent.Event

local unlockerPart = script.Parent
local debounce = true

local function onTouch(touchedPart)
	local partParent = touchedPart.parent
	local humanoid = partParent:FindFirstChild("Humanoid")
	if humanoid then
		if(debounce) then
			unlockerPart.Color = Color3.fromRGB(10, 40, 210)
			bindableEvent:Fire()
			debounce = false
		end
	end
end

function onTouch1.resetDebounce()
	debounce = true
end

function onTouch1.lockDebounce()
	debounce = false
end

unlockerPart.Touched:Connect(onTouch)

return onTouch1

Then, the ModuleScript “UnlockerModule” in the Unlocker model handles these events shot by each of the pads to create a userInput string via concatenation. It then compares it with the secret it generated:

local unlockerModule = {}

local Unlocker1 = script.Parent.Unlocker1
local Unlocker2 = script.Parent.Unlocker2
local Unlocker3 = script.Parent.Unlocker3
local Unlocker4 = script.Parent.Unlocker4

local unlocker1Hit = require(script.Parent.Unlocker1.ModuleScript)
local unlocker2Hit = require(script.Parent.Unlocker2.ModuleScript)
local unlocker3Hit = require(script.Parent.Unlocker3.ModuleScript)
local unlocker4Hit = require(script.Parent.Unlocker4.ModuleScript)

local bindableEvent = Instance.new("BindableEvent")
unlockerModule.authenticate = bindableEvent.Event

local userInput = ""

local function generateSecret()
	local secret = ""
	local secretTable = {"1","2","3","4"}

	for i=1, 4 do
		local randomIndex = math.random(#secretTable)
		local randomElement = secretTable[randomIndex]

		secret = secret .. randomElement
		table.remove(secretTable, randomIndex)
	end
	return secret
end

local generatedSecret = generateSecret()
print("secret: "..generatedSecret)

local function authenticate()
	if(string.len(userInput) == 4) then
		if(userInput == generatedSecret) then
			bindableEvent:Fire(true)
		else
			bindableEvent:Fire(false)
		end
	end
end

local function animatePart(c)
	local unlockPart
	if c == "1" then unlockPart = Unlocker1
	elseif c == "2" then unlockPart = Unlocker2
	elseif c == "3" then unlockPart = Unlocker3
	else unlockPart = Unlocker4
	end
	unlockPart.Color = Color3.fromRGB(10, 40, 210)
	wait(0.5)
	unlockPart.Color = Color3.fromRGB(237, 234, 234)
	wait(0.5)
end

local function animateSecret(number)
	for i=1, #number do
		local c = number:sub(i,i)
		animatePart(c)
	end
end

animateSecret(generatedSecret)

function unlockerModule.reset()
	userInput = ""
	Unlocker1.Color = Color3.fromRGB(237, 234, 234)
	Unlocker2.Color = Color3.fromRGB(237, 234, 234)
	Unlocker3.Color = Color3.fromRGB(237, 234, 234)
	Unlocker4.Color = Color3.fromRGB(237, 234, 234)
	lockPads()
	generatedSecret = generateSecret()
	print("secret: "..generatedSecret)
	wait(1)
	animateSecret(generatedSecret)
	unlockPads()
end

function lockPads()
	unlocker1Hit.lockDebounce()
	unlocker2Hit.lockDebounce()
	unlocker3Hit.lockDebounce()
	unlocker4Hit.lockDebounce()
end

function unlockPads()
	unlocker1Hit.resetDebounce()
	unlocker2Hit.resetDebounce()
	unlocker3Hit.resetDebounce()
	unlocker4Hit.resetDebounce()
end



unlocker1Hit.Hit:Connect(function()
	userInput = userInput.."1"
	print("userInput: "..userInput)
	authenticate()
end)
unlocker2Hit.Hit:Connect(function()
	userInput = userInput.."2"
	print("userInput: "..userInput)
	authenticate()
end)
unlocker3Hit.Hit:Connect(function()
	userInput = userInput.."3"
	print("userInput: "..userInput)
	authenticate()
end)
unlocker4Hit.Hit:Connect(function()
	userInput = userInput.."4"
	print("userInput: "..userInput)
	authenticate()
end)


return unlockerModule

However, this method doesn’t really seem scalable and I find it quite redundant. First, I have to duplicate the ModuleScripts for all the blue pads (it is fine with 4 pads, but not very efficient if I have a lot of pads). Then in UnlockerModule, I also have to handle all of the similar events separately, which adds extra lines.

Is there a better way to structure this, such that we can eliminate duplicate ModuleScripts and event handlers? Would like to hear your opinions, thank you!

2 Likes
#2

All you have to do is make this controlled by server scripts in ServerScriptService and ServerStorage. But make sure you organize them in folders too.

2 Likes
#3

Hi, thanks for the input. I’m not too familiar with how Client-Server stuff works in the context of Roblox, I’ve only started learning scripting a few days ago. The official documentation seems a little too vague regarding this, could you recommend any resources to start understanding how to develop in the context of Client-Server architecture?

1 Like
#4

I’m not sure I can help documentation-wise, but what they’re probably thinking of is having all your pads in one folder, then doing stuff with :GetChildren().

Edit: letter

#5

The issue is that each Unlocker pad has a ModuleScript that essentially have the same code, except the handling for the events they fire is different (pad1 adds “1” to user input, pad2 adds “2”, etc.).

Is there a way to use only one event for this? I’m not sure if folders will help if they all still need to have separate ModuleScripts (apart from organization).

1 Like
#6

Consider the functionality shared by all buttons: All buttons need to fire an event when touched, then wait for a cooldown/debounce period. This code should go in a single ModuleScript. That module should provide a single function which sets up the button functionality for any part passed to it.

function SetupButton(part, uniqueNumber)
--Connect the part's Touched event to the BindableEvent
--The event should also send the uniqueNumber, so the recipient knows which button sent the event.

All buttons use the same animation and “reset” functions. You can make these functions take the part as a parameter instead of trying to find it from “c”.

I assume what you would like to have is a system that detects how many buttons it contains and automatically sets them all up, adjusting the code generation and checking to match the number of buttons?

2 Likes
#7

I did not think of it to that extent, but it’s certainly worth considering especially if I want to have a lot of buttons. Based on what you’ve said, here’s my thought process on achieving this:

  1. Put all pads in the same folder
  2. Iterate through the folder, and for each child of the Folder, call SetupButton()

Is there anything actually stopping us from putting it in the same script as “UnlockerModule” instead of a ModuleScript?

I realized that since we have now have a centralized approach to this and “UnlockerModule” is already acting as a kind of centralized control to the mechanism.

1 Like
#8

Here is an update on how I got rid of multiple ModuleScripts in each part using the suggestions above.

First, the the pads are organized in a folder as suggested by @Secretum_Flamma:
image
UnlockerPadModule handles events for all pads like @azqjanna suggested:

Code 
local module = {}

local folder = script.Parent

-- Get all parts in parent folder, removing this script
local unlockerPads = folder:GetChildren()
local index
for i, v in ipairs(unlockerPads) do
	if v == script then
		index = i
	end
end
table.remove(unlockerPads, index)

local events = {}

local function setUpPad(part, padID)
	--Create new event for when pad is touched, send padID
	local bindableEvent = Instance.new("BindableEvent")
	part:SetAttribute("debounce", true)
	part.Touched:Connect(function(touchedPart)
		local partParent = touchedPart.parent
		local humanoid = partParent:FindFirstChild("Humanoid")
		if humanoid then
			if(part:GetAttribute("debounce")) then
				part.Color = Color3.fromRGB(10, 40, 210)
				bindableEvent:Fire(padID)
				part:SetAttribute("debounce", false)
			end
		end
	end)
	
	--Insert event into events table
	table.insert(events, bindableEvent.Event)
end

function module.lockPads()
	for _, pad in ipairs(unlockerPads) do
		pad:SetAttribute("debounce", false)
	end
end

function module.unlockPads()
	for _, pad in ipairs(unlockerPads) do
		pad:SetAttribute("debounce", true)
	end
end

function module.getEvents()
	return events
end

function module.resetPads()
	for _, pad in ipairs(unlockerPads) do
		pad.Color = Color3.fromRGB(237, 234, 234)
	end
end

function module.animatePart(unlockPart)
	if(unlockPart) then 
		unlockPart.Color = Color3.fromRGB(10, 40, 210)
		wait(0.5)
		unlockPart.Color = Color3.fromRGB(237, 234, 234)
		wait(0.5)
	end
end

function module.animateSecret(number)
	for i=1, #number do
		local c = tonumber(number:sub(i,i))
		local part = unlockerPads[c]
		module.animatePart(part)
	end
end

function module.getNoOfPads()
	local size = 0
	for _, pad in unlockerPads do
		size = size + 1
	end
	return size
end

for i, pad in ipairs(unlockerPads) do
	setUpPad(pad, i)
end

return module

Some changes in UnlockerPadModule which may be of interest:

  • Originally, each ModuleScript in the pads has their own local debounce value. Since we no longer have separate modules for each pad, I decided to use Instance Attributes to store the debounce value in the part itself. An alternative I considered was to use a table mapping debounce values to each part, but using an Instance Attribute seems to be the cleaner and simpler way.
  • In order for our caller, UnlockerModule to be able to handle all the events in a any amount of pads we create, we have to create a table of events which can be acquired by the getEvents() function. The event table itself is created during setupPad().

Here is how UnlockerModule handles the events:

Code 
local unlockerModule = {}
local Players = game:GetService("Players")
Players.PlayerAdded:Wait()
local unlockerPadModule = require(script.Parent.UnlockPads.UnlockerPadModule)

local bindableEvent = Instance.new("BindableEvent")
unlockerModule.authenticate = bindableEvent.Event

local userInput = ""
local size = unlockerPadModule.getNoOfPads()

local function generateSecret()
	local secret = ""
	local secretTable =  {}
	for i=1, size do
		table.insert(secretTable, i)	
	end

	for i=1, size do
		local randomIndex = math.random(#secretTable)
		local randomElement = secretTable[randomIndex]

		secret = secret .. randomElement
		table.remove(secretTable, randomIndex)
	end
	return secret
end

local generatedSecret = generateSecret()
print("secret: "..generatedSecret)

local function authenticate()
	if(string.len(userInput) == size) then
		if(userInput == generatedSecret) then
			bindableEvent:Fire(true)
		else
			bindableEvent:Fire(false)
		end
	end
end

unlockerPadModule.animateSecret(generatedSecret)

function unlockerModule.reset()
	userInput = ""
	unlockerPadModule.lockPads()
	unlockerPadModule.resetPads()
	generatedSecret = generateSecret()
	print("secret: "..generatedSecret)
	wait(1)
	unlockerPadModule.animateSecret(generatedSecret)
	unlockerPadModule.unlockPads()
end

local events = unlockerPadModule.getEvents()

for _, event in events  do
	event:Connect(function(padID)
		userInput = userInput..padID
		print("userInput: "..userInput)
		authenticate()
	end)
end

return unlockerModule

Some notable changes:

  • The secret can now be of any length. So we first have to get the size (number of pads) before generating the secret.
  • I’ve put the animation, reset and locking functions in UnlockerPadModule as it seemed more appropriate.

#9

I suggest using OOP which allows you to do a lot with just a single module script to access. You can also make it configurable for what the color should be when activated.

1 Like
#10

That’s a great resource, I didn’t know OOP could be done in Lua. Thanks!

1 Like