You can write your topic however you want, but you need to answer these questions:
What do you want to achieve? Keep it simple and clear!
I want players to be able to click on a mesh to equip it as an accessory, but if they already have that accessory equipped it doesn’t allow them to equip another. I also want players to be able to remove an accessory from their player and equip it again later on.
What is the issue? Include screenshots / videos if possible!
The problem I’m having is that even after setting to nil as shown in the output, the accessory is still detected as being equipped. Destroy() similarly doesn’t work.
Here is the script which removes the chosen accessory
function removeSelectedAccessory()
-- Update players accessory data/the list of what they have equipped
accessoryData()
--Set the target name to the name of the currently selected accessory the player wants to delete
local targetName = playerAccList[selectedAcc][3]
--Loop through every accessory the player has
for _, accessory in humanoid:GetAccessories() do
--If the accessory matches the to be deleted one, proceede
if accessory.Name == targetName then
print("Found accessory: " .. targetName)
--Loop through each of the descendants of the accessory, setting them to nil
for i, part in accessory:GetDescendants() do
part = nil
end
warn(accessory)
accessory.Parent = nil
accessory = nil
task.wait(0.1)
warn(accessory)
break
end
end
accessoryData()
end
Here is my code that detects if the player already has the accessory equipped
local function onClicked(player, accessoryName)
local HasAccessory = false
-- detect if the player has the accessory already or not
for _, accessory in player.Character:GetChildren() do
if accessory.Name == accessoryName then
HasAccessory = true
print ("Accessory is already equipped")
break
end
end
if HasAccessory then
return
end
game.ReplicatedStorage.RemoteEvents.UpdateGUI:FireClient(player, accessoryName)
-- create a copy of the accessory and attach it to the player
local clone = game.ReplicatedStorage.Accessories:FindFirstChild(accessoryName):Clone()
local playerModel = game.Workspace:WaitForChild(player.Name)
local humanoid = playerModel:WaitForChild("Humanoid")
humanoid:AddAccessory(clone)
end
for _, accessory in workspace.AccessoryFolder:GetChildren() do
accessory.ClickDetector.MouseClick:Connect(function(player)
onClicked(player, accessory.Name)
end)
end
I will note that they are setup like this so that each individual part of the accessory can be recolored by the player.
What solutions have you tried so far? Did you look for solutions on the Creator Hub?
So far I have tried using both Destroy() and setting to nil on the descendants of the accessory, on the accessory itself, and the parent, and nothing has worked.
When destroying an instance, you need to do just that, destroy the instance. When you write part = nil, you are not deleting the part. You are just saying “I am now defining part as nil”, i.e. “I am now calling nil, part”. When you call part:Destroy() you are saying “Destroy the thing that is called part”. Anyway, Destroy gets rid of descendants and parents and so on so replace:
for i, part in accessory:GetDescendants() do
part = nil
end
warn(accessory)
accessory.Parent = nil
accessory = nil
task.wait(0.1)
warn(accessory)
Check if the accessory is actually being destroyed by loading into the game, finding your character model in workspace, finding the accessory in the character model and seeing if it disappears when your function runs.
If it doesn’t, you’re destroying the wrong accessory/have the wrong target.
If it does, then there’s something wrong in the way your OnClicked is searching for the accessory (maybe the accessoryName is wrong?)
In that case, could you print accessoryName when you run OnClicked, and see if it’s valid? You can also send me the game file if you don’t mind, it is difficult to assess everything when I don’t know where every argument/function is coming from/doing.
I’ve found the issue. It’s because you are destroying the accessory on the client, but adding it on the server. If you affect workspace on the client (like destroying items) it doesn’t carry over to the server, which is where you’re checking for whether the item is already equipped.
Add a remoteevent called RemoveAccessory
Replace on client:
function removeSelectedAccessory()
accessoryData()
--Set the target name to the name of the currently selected accessory the player wants to delete
local targetName = playerAccList[selectedAcc][3]
--Loop through every accessory the player has
game.ReplicatedStorage.RemoteEvents.RemoveAccessory:FireServer(targetName)
accessoryData()
end
On server add:
local function RemoveAccessory(player, accessoryName)
local humanoid = player.Character:WaitForChild("Humanoid")
for _, accessory in humanoid:GetAccessories() do
--If the accessory matches the to be deleted one, proceede
if accessory.Name == accessoryName then
print("Found accessory: " .. accessoryName)
--Loop through each of the descendants of the accessory, setting them to nil
accessory:Destroy()
break
end
end
end
game.ReplicatedStorage.RemoteEvents.RemoveAccessory.OnServerEvent:Connect(RemoveAccessory)
Also, the way your code is structured on the client is (no disrespect) one of the worst things I have ever seen. You should really think about generalising things and storing metadata differently to how you are now (putting things in modules if needed). Your script being ~1000 lines for some extremely basic ui function is completely unnecessary
omg, thank you so much it works now. And I am well aware of that, I’m still learning and I used a gui to lua plugin just so I could make something that works, so if you have any tutorials or reading recommendations related to that please let me know as I am also working on figuring out how to optimize my code in a number of different ways which you can see in another forum post I made recently.
What do you mean by “GUI to lua”? As in, does it convert all of you GUI to code so that you don’t need to store any instances on the game’s storage?
If I am understanding that correctly, I am not sure if there are any benefits to that, I have always just made UI and stored it in StarterGUI (disabled by default) or ReplicatedStorage, and referenced things in scripts on a need-to basis. If there are benefits, chuck it all into a module so that your actual script is easier to navigate.
There are also a ton of functions you have that are near-identical to each-other. By adding extra arguments you can have massively reduce your line count and make editing that function way easier in the future (you should almost never have to copy-paste anything).
As far as tutorials go there are no “code structure” tutorials I know of, there are just best practices for conventions that you pick up over time. Rule of thumb is, unless you’re storing metadata (which can just be very repetitive, and belongs in a module), code should never really look repetitive.
there’s literally a plugin called GUI to LUA that I use to convert my gui into lua script.
I personally prefer to work with scripts instead of gui elements because to me at least it’s easier to use.
I’ll keep this in mind for the future and what I’m already working on.
Would you have any recommendations on how to make it so that creating the GUI within a code is less repetitive? as well as how to handle multiple buttons with just 1 function.
Honestly, if you like the plugin, keep using it, if it works it works. The mess isn’t really something you need to deal with unless you need to work in it, which is what I initially assumed. If you’re just using the plugin, keep the code that generates all of the buttons in a separate script, or a module if you want easier button referencing, so that you never have to deal with it when you’re writing the actual behaviour of your UI.
If you’re writing code that generates UI by hand, I can give some advice, but if you’re just using a plugin I wouldn’t rewrite code for the sake of it, just make sure it isn’t interfering with your workflow.
