How can I fix my script?

Help and Feedback Scripting Support
#1

Hello, this is my first ever post so please don’t expect me to get everything right.

Here is my script that I have, I want the script to give tools to certain ranks, the tools are located in the server storage. but for some reason, the script is not working. Can someone help me please?

game.Players.PlayerAdded:Connect(function(plr)
if plr:GetRankInGroup(5090028) == 2 then
   local tool = game.ServerStorage["X-26"]:Clone()
   tool.Parent = plr:WaitForChild("Backpack")
elseif plr:GetRankInGroup(5090028) == 3 then
    local tool = game.ServerStorage["X-26"]:Clone()
   tool.Parent = plr:WaitForChild("Backpack")
elseif plr:GetRankInGroup(5090028) == 4 then
    local tool = game.ServerStorage["X-26"]:Clone()
   tool.Parent = plr:WaitForChild("Backpack")
elseif plr:GetRankInGroup(5090028) == 5 then
    local tool = game.ServerStorage["X-26"]:Clone()
   tool.Parent = plr:WaitForChild("Backpack")
elseif plr:GetRankInGroup(5090028) == 6 then
    local tool = game.ServerStorage["X-26"]:Clone()
   tool.Parent = plr:WaitForChild("Backpack")
elseif plr:GetRankInGroup(5090028) == 7 then
    local tool = game.ServerStorage["X-26"]:Clone()
   tool.Parent = plr:WaitForChild("Backpack")
elseif plr:GetRankInGroup(5090028) == 8 then
    local tool = game.ServerStorage["X-26"]:Clone()
   tool.Parent = plr:WaitForChild("Backpack")
 end
end)

And if someone can tell me is there a possible way to do this for only a certain team?

1 Like
#2

First let’s make the code clean and add CharacterAdded event becuase backpack is being removed and new one is created each time the character is added.

game.Players.PlayerAdded:Connect(function(plr)
    plr.CharacterAdded:Connect(function(char)
        local rank = plr:GetRankInGroup(5090028);
        local ranks = {2, 3, 4, 5, 6, 7, 8};

        for i,v in pairs(ranks) do
            if v == rank then
                local tool = game.ServerStorage["X-26"]:Clone()
                tool.Parent = plr:WaitForChild("Backpack")
                break;
            end
        end
    end)
end)
1 Like
#3

GetRankInGroup yields so you should call it once and store its result in a variable. You’re also cloning the exact same tool so you could do something like this to make it neater:

-- PlayerAdded event hook
game:GetService("Players").PlayerAdded:Connect(function(player)
	
	-- Store the player's rank, only yields once
	local groupRank = player:GetRankInGroup(5090028)
	
	-- If their rank is between 2 and 8 inclusively
	if groupRank >= 2 and groupRank <= 8 then
		-- Clone the tool
		local tool = game:GetService("ServerStorage")["X-26"]:Clone()
		
		-- Parent the tool
		tool.Parent = player:WaitForChild("Backpack")
	end

end)
5 Likes
#4

The for loop isn’t the most performance friendly solution in this case, it’s not necessary.

2 Likes
#5

It’s fast and you can set the exact ranks you want.

1 Like
#6

Thanks for making it more compact but is there a way to make it so only team “red” can obtain a tool or is it like a whole another script?

1 Like
#7 
game.Players.PlayerAdded:Connect(function(plr)
    plr.CharacterAdded:Connect(function(char)
        local teamColor = plr.TeamColor;
        if teamColor == BrickColor.new("Red") then -- here you set the team color you want
            local rank = plr:GetRankInGroup(5090028);
            local ranks = {2, 3, 4, 5, 6, 7, 8};

            for i,v in pairs(ranks) do
                if v == rank then
                    local tool = game.ServerStorage["X-26"]:Clone()
                    tool.Parent = plr:WaitForChild("Backpack")
                    break;
                end
            end
        elseif teamColor == BrickColor.new("Green") then
            -- you can add more colors and give different weapons if you want to that team
        end
    end)
end)

The structure of the code can be different depending on what exactly you want to achieve.

1 Like
#8

No reason to use for i, v in pairs() loop as you are in that script looping through an array of values. Instead, a more performance-friendly option could be using the well known for i = 1, #tbl loop which is far more efficient. However, depending on how many group ranks @clash_ofsss plans on looping through, I feel that a dictionary index to check if the value is there would be more appropriate (then again this all depends on the size of the table).

3 Likes
#9

This looks good! I suggest defining the services earlier on in the script in case @clash_ofsss wants to use them later on in the script (avoid repetitive actions).

1 Like