What is wrong with this code?

[CentreWorks]

You can write your topic however you want, but you need to answer these questions:

1. A script that gives a GUI to group members with ranks 3 and above

2. It doesn’t load the GUI when I join

3. I have no solution

Here is the code:

[ScytheSlayin (akorn)]

Please format your code, it’s easier to read.

[CentreWorks]

Done, it’s now a screenshot to make it easier.

[D1CEL]

There is no need to use the Changed event to see when the player has a new Character since there is a proper event for this (CharacterAdded). Additionally, you shouldn’t connect a function that runs when respawning when the player first joins as they might not yet have a character upon first joining.

As far as the function itself goes, you don’t need to call both IsInGroup and GetRankInGroup since GetRankInGroup will just return a 0 if they are not in the group. I’m confused as to why you’re cloning the children of HandtoGui rather than the entire thing itself. What exactly is “HandtoGui” in this?

[CentreWorks]

HandtoGui is the name of the GUI

[D1CEL]

Try cloning HandtoGui rather than its children. Also, are you running this on the client or the server? Only the client can access PlayerGui. UI elements must also be inside of a ScreenGui to be displayed.

[McRocketNuggets]

Yep, you can’t access the player GUI through a server script. Instead, you’ll need to use a remote event that gives the UI through the client. You can read more about remote events here.

[SupaOverpowered (Supa)]

It’s possible to access the player gui with a normal script. On one game I used no local scripts whatsoever and the gui’s still worked.

[colbert2677 (ImagineerColbert)]

Your code is doing a lot of unnecessary work, reinventing the wheel for certain events already provided by the engine and displays bad organisation. You can cull most of this work and make it simple. You could additionally alleviate the work of having to always clone this Gui per respawn by just toggling ResetOnSpawn off and adding it for specific players.

local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")

local function onPlayerAdded(player)
    -- No wait at the beginning & no redundant calls, use only one group check
    if player:GetRankInGroup(0) >= 0 then
        -- Use ServerStorage for storing, NOT Lighting
        -- May need to WaitForChild on PlayerGui
        ServerStorage.HandtoGui:Clone().Parent = player.PlayerGui

        -- Within rank check passed scope, so we can automatically pass and give
        player.CharacterAdded:Connect(function ()
            ServerStorage.HandtoGui:Clone().Parent = player.PlayerGui
        end)
    end
end

Players.PlayerAdded:Connect(onPlayerAdded)

for _, player in ipairs(Players:GetPlayers()) do
    onPlayerAdded(player)
end

---

@2dull @McRocketNuggets

The server is able to access PlayerGui as well. Changes to PlayerGuis however, including destruction of them, will not replicate to the server. The only time in which the server should be involved directly with ScreenGuis is to replicate them to PlayerGui. The engine does this internally for StarterGui as well.

[Nabfam (Mooshua)]

Oooookay there’s a few problems here

1. You create a connection to playerAdded twice…
2. You don’t use player.CharacterAdded; and
3. You end up calling onPlayerRespawned a total of 2 times.
4. You don’t have to create a new HandtoGui every time

So here’s what we do:

Unfamiliar with group endpoints in roblox, unsure if they work on client. forgive me

1. Move the ui to StarterGui and uncheck resetonspawn
2. Make this localscript in Startergui:

local ui = game.Players.LocalPlayer.PlayerGui:WaitForChild("HandtoGui")
local player = game.Players.LocalPlayer

-- every 30 seconds, we perform these actions.
-- depending on the game, you can change this.
while true do
    local rank = player:GetRankInGroup(0)
    -- get rank, et cetera.
    -- enable/disable ui as needed.
    if rank >= 3 then
        ui.Enabled = true
    else
        ui.Enabled = false
    end
    -- I moved this to the end, so the player receives their handToGui immediately.
    wait(30)
end

[Deferend]

FYI, when a player is not in the group provided to :GetRankInGroup(), it’ll return zero. Therefore, your statement could evaluate to 0 or 0.

[colbert2677 (ImagineerColbert)]

@Nabfam

You create a connection to playerAdded twice…

Non-problem. In this specific scenario it’s not necessary and everything can be handled within a single connection, otherwise there’s no issue with multiple connections. Unless that’s what you meant.

Refer to this thread if you’d like some information (it’s primarily about whether or not connections run pseudoasynchronously or something close to it, but I did ask about whether connecting multiple functions to a signal was bad or not):

Do connected functions run relatively at the same time?

I searched beforehand. If I missed the thread discussing this, please let me know! I’ve generally been conditioned to believe that connecting to a signal several times over is bad. I don’t know where I picked that up from but I did and that’s causing me bouts of counterproductivity where I try forcing my code to follow some elegant or overcomplicated pattern that it ends up becoming inelegant. With that said, simple question. When you connect a function to a signal (for example, you have sever…

[McRocketNuggets]

It’s not possible if filtering is enabled.

[Nabfam (Mooshua)]

This is an extremely bad practice, for many security reasons, but mainly because any lag between the user and server will hinder basic functions that might not need the server in the first place. I highly recommend not doing that in client-side operations.