Efficient/shorter way for my code

Help and Feedback Code Review
,
#1

Hey Developers!

I just make a match system,

This is the script

function UpdateClient(ChangeService)
	local Player1
	local Player1Score
	local Player2
	local Player2Score
	local Player1Image
	local Player2Image
	
	local Players = script.Parent.Players
	
	Player1 = Players[script.Parent.Set.Paddle.Plr.Value].Name
	Player1Score = Players[script.Parent.Set.Paddle.Plr.Value].Value
	Player2 = Players[script.Parent.Set.Paddle2.Plr.Value].Name
	Player2Score = Players[script.Parent.Set.Paddle2.Plr.Value].Value
	
	local userId = game.Players[script.Parent.Set.Paddle.Plr.Value].UserId
	local thumbType = Enum.ThumbnailType.HeadShot
	local thumbSize = Enum.ThumbnailSize.Size420x420
	local content, isReady = Players:GetUserThumbnailAsync(userId, thumbType, thumbSize)
	
	local userId2 = game.Players[script.Parent.Set.Paddle2.Plr.Value].UserId
	local thumbType2 = Enum.ThumbnailType.HeadShot
	local thumbSize2 = Enum.ThumbnailSize.Size420x420
	local content2, isReady2 = Players:GetUserThumbnailAsync(userId2, thumbType2, thumbSize2)
	
	Player1Image = content
	Player2Image = content2
	
	
	for i = 1,#Players do
		game.ReplicatedStorage.Remotes.UpdateGame:FireClient(game.Players[Players[i].Name],Player1, Player1Score, Player2, Player2Score, Player1Image, Player2Image, ChangeService)
	end
end

so what the code do is it updates the player match gui. (it update scores, name and player image)

It worked fine but i think its quite long, and i need it more shorter and efficient.

any helps would be appreciated

(btw its a 2 player game)

#2

You use paths like “script.Parent.Set.Paddle.Plr.Value” multiple times, I suggest capturing these into variables.

You have two blocks of code that do the same thing to get the content for each player. I suggest encapsulating this into a local function that takes the userId as a paramter and returns the content.

These two simple changes will make the code a bit shorter but definitely more readable.

You could additionally combine the declaration and assignment of all of the variables to make the code as short as possible.

In terms of efficiency, you stated it is a two player game, I wouldn’t bother with the loop at the bottom. A loop requires the script run time to setup structures to track the interation. As you know there are just two players make two calls to trigger the remote instead.

1 Like
#3

What is your goal with this code? So i can understand and help you.

#4

Did you even pay attention……….

#5

I really thing that you already have one of the shortest way to commit this action. I couldn’t come up with anything shorter.

#6

You could define Set, ReplicatedStorage and Players as variables to make the script more readable.

1 Like