Hello, i have a part and when a player touches the part it clones a gui from lighting and puts the clone in playerGui. Then if the player stops touching the part the clone is destroyed. This all works fine but there is errors in the console for some reason. Also keep in mind i'm a beginner at scripting. So please just answer as if your talking to a beginner and not an experienced programmer.
error 1: Workspace.OpenPCgui.ShowPcScreen:17: attempt to index local ‘guiClone’ (a nil value)
error 2: Workspace.OpenPCgui.ShowPcScreen:16: attempt to index local ‘Player’ (a nil value)
Here is the script inside the part...
local Gui = game.Lighting.ComputerGui
function GiveGui(Player)
if Player.PlayerGui:FindFirstChild(Gui.Name)~=nil then return end
Gui:Clone().Parent=Player.PlayerGui
end
script.Parent.Touched:Connect(function(hit)
local Player=game.Players:GetPlayerFromCharacter(hit.Parent)
if Player==nil then return end
GiveGui(Player)
end)
script.Parent.TouchEnded:Connect(function(hit)
local Player=game.Players:GetPlayerFromCharacter(hit.Parent)
local guiClone = Player.PlayerGui:FindFirstChild(Gui.Name)
guiClone:Destroy()
end)
can someone just tell me why this works fine but there is errors saying that player and the gui clone is equal to nil? And possibly help me fix the code to not have these errors please.
There are couple of problems with your code that i can spot, firstly in the touch ended function you are assuming that hit is the player, which is not always the case and can cause the Player is nil error to occur. This will also directly cause your second error to occur. Secondly i would recommend adding a Debounce to your touched function. With that being said changing your Touched Ended function should fix your problem:
---if this is local script using IsDescendantOf is way easier
script.Parent.TouchEnded:Connect(function(hit)
if hit:IsDescendantOf(player.Character) then -- or get player from character
local Player = hit.Parent
local guiClone = Player.PlayerGui:FindFirstChild(Gui.Name)
guiClone:Destroy()
end
end)
That’s a bad way to do it. It’s pretty messy. Cloning and destroying and not using remote firing events. Not the best.
Really, you want to have the gui in the player, offscreen. So it’s there but just not visible.
When you touch the part, you trigger the debounce to stop it from triggering numerous times a second. Then that touch also tells the client to run the open gui code. Just have the touched part FireServer.
The tricky part is getting the Hit to return the Player. I think you need to use the get player service, and then you have to find the player that matches the “Hit.Parent.Name”
Click detectors return the player well but the legs aren’t in a simple tree in the player. I think that “Hit.Parent” isn’t the same as ClickDetector “Player”
That is the hardest part of this.
I wish I could see a part tree for a player in game.
I don’t know a lot of these words. I never used a remote event and i’m not sure what “FireServer” means. I’m just trying to make this thing with the knowledge i have. So ill just use it the way i have it now. Also the code works fine but it says player is nil. Is there a way i can just make a global variable for player(not in a local script) and use that instead of trying to get it from hit.parent?
It’s reporting nil anytime the part is not a character or if the hierarchy is deeper than 1 child (such as in accoutrements, tools, etc). You should first check if the hit.Parent has a humanoid in it. If yes, then check if the player is obtainable, then continue. This would look like this (this is pseudocode AKA you need to edit it for your needs; I just typed it as demonstration):
-- touched event function(hit)
if hit.Parent:FindFirstChild”Humanoid” then
local plr = game.players:GetPlayerFromCharacter(hit.Parent)
if plr then
-- do stuff
end
end
end)
You aren’t declaring what guiClone is anywhere on your code, and there is a lack of checks to prevent logic errors.
local LightingService = game:GetService("Lighting")
local PlayersService = game:GetService("Players")
local Gui = game.Lighting.ScreenGui --I advise using ServerStorage over Lighting
local GivenGui = {}
local function GiveGui(Player)
local ClonedGui = Gui:Clone()
ClonedGui.Parent = Player:WaitForChild("PlayerGui")
GivenGui[Player.Name] = ClonedGui
end
local Debounce = true
local Delay = 5 --time in seconds
local function OnTouched(Object)
if Object.Parent:FindFirstChild("Humanoid") then
if Debounce then
Debounce = false
GiveGui(PlayersService:GetPlayerFromCharacter(Object.Parent))
wait(Delay)
Debounce = true
end
end
end
local function OnTouchEnded(Object)
if Object.Parent:FindFirstChild("Humanoid") then
if GivenGui[Object.Parent.Name] ~= nil then
GivenGui[Object.Parent.Name]:Destroy()
end
end
end
script.Parent.Touched:Connect(OnTouched)
script.Parent.TouchEnded:Connect(OnTouchEnded)
Some things I added:
• Debounce
• Support for Multiple Players
• Modular-ish Structure
Some things to Consider:
• Avoid using Part.Touched as it is unreliable for your use case
• Use server storage to store the Gui as exploiters could easily look through Lighting (objects in Lighting are replicated to the client)
Note:
As I’ve mentioned, Part.Touched is not reliable for your use case, so this does not always work as intended, however it gives and destroys the Gui.
ComputerGui Giver.rbxl (18.7 KB)
Here’s a place file with the reccomended setup for the best results. Take note of how the part was made:
• Part size as tall or taller than the character
• CanCollide is false
Edit:
Considering what @colbert2677 said, it’s better to just place the Gui in StarterGui and toggle the visible property when the part is touched.
If you gui has any remote events/functions related, the server will do the checks. It is unnecessary to reparent the Gui as you could simply hide it when parented to PlayerGui in the first place.
No please don’t do either, using Lighting as a storage or using the server to parent a ScreenGui on touch unless you reasonably require server-side verification. This entire script should be done on the client. I don’t know what it is so I can only assume but based on the structure, this looks like something that should be done from the client.
Remember that when players are able to interact with a Gui up to closing it, the server isn’t going to know what the client did. The server should always have little to no say in interfaces aside from cloning them to the PlayerGui one way and without any guiding information.
I thought about it and you’re right actually. The gui should be on the client, and if there are any security related systems, the server verifies remote events, data sent etc.
• The client handles whether the ComputerGui is open or not
• The server will be able to verify data accuracy
Some things to consider:
Instead of using Part.Touched, I just thought that Proximity may be a better alternative
Example:
local Part = workspace:WaitForChild("Part")
local Character = Player.Character or Player.CharacterAdded:Wait()
local CharacterRoot = Character:WaitForChild("HumanoidRootPart")
local Scanning = true
local Distance = 10
while Scanning do
if (Part.Position - CharacterRoot.Position).Magnitude <= Distance then
--show gui
else
--hide gui
end
wait(1)
end
This will work better than touched events. However, if you’re not a fan of infinite loops, you’ll have to rely on another looping method.
If you place the Gui in StarterGui, it will get cloned into PlayerGui. So you must do:
local PlayersService = game:GetService("Players")
local Player = PlayersService.LocalPlayer
local PlayerGui = Player:WaitForChild("PlayerGui")
local ComputerGui = PlayerGui:WaitForChild("ComputerGui")