Help Make Script More Efficient [SOVLED]

Help and Feedback Code Review
#1

Provide an overview of:

  • What does the code do and what are you not satisfied with?
    My code checks if my mouse, while holding a tool, is pointed at a specific part. If it is, then it fires a server event on click.
  • What potential improvements have you considered?
    I have considered using a table/array (I am not sure which is which or much about each yet to use one in this script).
  • How (specifically) do you want to improve the code?
    I simply want to make it shorter. I saw a post of someone replacing a lot of if and elseif statements with a table and made me consider doing it for my own script.
function onButton1Down(player)
	if mouse.Target == ComicPlacement1 then
		CopyComic7P1:FireServer()
		print("Copying Comic7 to Placement1")
	elseif mouse.Target == ComicPlacement2 then
		CopyComic7P2:FireServer()
		print("Copying Comic7 to Placement2")
	elseif mouse.Target == ComicPlacement3 then
		CopyComic7P3:FireServer()
		print("Copying Comic7 to Placement3")
	elseif mouse.Target == ComicPlacement4 then
		CopyComic7P4:FireServer()
		print("Copying Comic7 to Placement4")
	elseif mouse.Target == ComicPlacement5 then
		CopyComic7P5:FireServer()
		print("Copying Comic7 to Placement5")
	elseif mouse.Target == ComicPlacement6 then
		CopyComic7P6:FireServer()
		print("Copying Comic7 to Placement6")
	elseif mouse.Target == ComicPlacement7 then
		CopyComic7P7:FireServer()
		print("Copying Comic7 to Placement7")
	elseif mouse.Target == ComicPlacement8 then
		CopyComic7P8:FireServer()
		print("Copying Comic7 to Placement8")
	elseif mouse.Target == ComicPlacement9 then
		CopyComic7P9:FireServer()
		print("Copying Comic7 to Placement9")
	elseif mouse.Target == ComicPlacement10 then
		CopyComic7P10:FireServer()
		print("Copying Comic7 to Placement10")
	elseif mouse.Target == ComicPlacement11 then
		CopyComic7P11:FireServer()
		print("Copying Comic7 to Placement11")
	elseif mouse.Target == ComicPlacement12 then
		CopyComic7P2:FireServer()
		print("Copying Comic7 to Placement12")
	elseif mouse.Target == ComicPlacement13 then
		CopyComic7P3:FireServer()
		print("Copying Comic7 to Placement13")
	elseif mouse.Target == ComicPlacement14 then
		CopyComic7P4:FireServer()
		print("Copying Comic7 to Placement14")
	elseif mouse.Target == ComicPlacement15 then
		CopyComic7P5:FireServer()
		print("Copying Comic7 to Placement15")
	elseif mouse.Target == ComicPlacement16 then
		CopyComic7P6:FireServer()
		print("Copying Comic7 to Placement16")
	end
end

mouse.Button1Down:Connect(onButton1Down)
#2

elseif is not very efficient in your case but you could do the following:
make a module in which you have each function or variable and then make a table of that module and after that you connect the function

#3

by module you mean modulescript?

#4

You do not need that many elseifs, you can just use a single for loop.

for i = 1, 16 do
	if mouse.Target.Name == ("ComicPlacement" .. i) then
		RemoteEvents["CopyComic7P" .. i]:FireServer()

		print("Copying Comic7 to Placement" .. i)

		break
	end
end
3 Likes
More Efficient Scripting (Local to Server script Remote Events) [SOLVED]
#5

Thanks I will try this out! I’ll make sure to reply later and let you know if it worked out c:

1 Like
#6

Maybe the different remote events could be changed to one remote event with an argument? (this would depend on the event logic though)
if so:

function onButton1Down(player)
    -- Check if mouse.Target is one of the desired parts (this should be done on server too)
    CopyComic7P:FireServer(mouse.Target)
end
1 Like
#7

Sorry its been 2 days but I finally had time to hop back on roblox studio and this worked perfectly! Was able to shrink my script soooo much so thanks sm! Marked as the solution!

1 Like
#8

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.