Why is button not working

local Click = game.StarterGui.ScreenGui.Click

Click.MouseButton1Click:Connect(function(clicked)
local char = clicked.Parent
local plr = game.Players:GetPlayerFromCharacter(char)

if plr  then
	plr.leaderstats.Coins.Value = plr.leaderstats.Coins.Value +1
end

end)

3 Likes

Instead of using one script use a remote event and send over the player information like this

Inside the button:

local player = game.Players.LocalPlayer

local Cash = player.leaderstats:FindFirstChild("Cash")

script.Parent.MouseButton1Click:Connect(function()
	if Cash.Value > 0 then
		game.ReplicatedStorage.RE:FireServer(player)
	end
end)

ok and for the server script:

game.ReplicatedStorage.RE.OnServerEvent:Connect(function(PLR)
	PLR.leaderstats:FindFirstChild("Cash").Value = PLR.leaderstats:FindFirstChild("Cash") + 1
end)

1 Like

You’re listening for the button in StarterGui. Listen for the one in PlayerGui instead.

Also, pretty sure server-sided scripts won’t work in StarterGui anyway…

If Click is an item inside a ScreenGui then clicked.Parent would not be the Character.

clicked.Parent would be the ScreenGui since the button is inside that folder.

So, local plr = game.Players:GetPlayerFromCharacter(char) would not work because you are using the ScreenGui folder to find the player you already have.

At least, that is what it looks like you are doing.

2 Likes

so what would the new code be??

1 Like

Well, taking in to account what @mc7oof said above, you would probably want to listen in ServerScriptService (I would heavily recommend implementing the system @stormmaster9090 recommended because it’s way better, but this would require minimal changes)

local function listen(player:Player)
    local function addCash()
        player.leaderstats.Coins.Value += 1
    end
    local button = player:WaitForChild("PlayerGui"):WaitForChild("ScreenGui"):WaitForChild("Click")
    button.MouseButton1Click:Connect(addCash)
end

game:GetService("Players").PlayerAdded:Connect(listen)

This looks messy and isn’t the best for efficiency, please use the method suggested by @stormmaster9090 .

MouseButton1Click is not a valid event of a gui object. Use Activated or MouseButton1Down instead.

It is a valid event of a gui button object, along with MouseButton1Down and MouseButton1Up. It fires when a full click of the object has completed.

1 Like

Oh my, that is quite a mistake! I never used it, good to know. Thanks for the correction :slight_smile:

1 Like

I’m 100% sure this video will fix you issue:

You should use a RemoteEvents instead …
Also server/legacy scripts won’t run on client focused services like StarterGui
Here’s how your scripts should look:
For the localscript in StarterGui

local Button = script.Parent

Button.MouseButton1Click:Connect(function()
    game:GetService('ReplicatedStorage').CoinEvent:FireServer()
end)

And for the server script in ServerScriptService

game:GetService('ReplicatedStorage').CoinEvent.OnServerEvent:Connect(function(Player)
    Player.leaderstats.Coins.Value += 1
end)

Don’t forget to create a RemoteEvent in ReplicatedStorage and rename it to ‘CoinEvent’

The only problem with that is it’s extremely easy for exploiters to give themself a ton of money really easily. Do not trust the client with anything like that. Instead, manage it all on the server.

Yes, I understand. I just wanted to keep it simple for the OP, since I could see that he’s a beginner scripter, and also due to this being in StarterGui exploiters do not have “an easier” way of getting money, since it’s replicated for everyone. However a debounce should be the minimum in the server script.

I get what you mean about keeping it simple, but exploiters with script injectors, etc. can spam the event, and even with a debounce they can get a big advantage.

1 Like

I don’t need script injection to spam the event, it’s replicated for everyone, just use an autoclicker and it will result in almost the same thing as using external software, however neither will bypass a server-side debounce, but yeah if it’s not replicated for every client then exploiters will have an advantage.

Here’s what you’ll need to do:

  1. Place the server Script inside of the button named Click
  2. Replace the code with this:
local click = script.Parent

local player = click:FindFirstAncestorWhichIsA("Player")

click.MouseButton1Click:Connect(function()
	player.leaderstats.Coins.Value += 1
end)

Do note that MouseButton1Click doesn’t provide a parameter, so an alternative method to get the player needs to be used, like the one in my example :slight_smile::+1: