I’m trying to make a tool that will delete for everyone once used, then add effects to the player who used it. However, the Remote function I made doesn’t return to the LocalScript.
Server Script
local mushbreak = script.Parent:WaitForChild("BlueMushroomBreak")
mushbreak.OnServerInvoke = function(player, tool)
tool:Destroy()
end
LocalScript
local tool = script.Parent
local lighting = game.Lighting
local anim = script:WaitForChild("Animation")
local mushbreak = script.Parent:WaitForChild("BlueMushroomBreak")
tool.Activated:Connect(function()
local Player = game.Players.LocalPlayer
local Character = Player.Character
local Humanoid = Character:WaitForChild("Humanoid")
local sound = game.Workspace:WaitForChild("Heartbeat")
local animplay = Humanoid:LoadAnimation(anim)
animplay:Play()
wait(1)
Humanoid.Health = Humanoid.Health + 10
mushbreak:InvokeServer(tool)
wait(2)
print("worked")
local high = Instance.new("ColorCorrectionEffect", lighting)
high.TintColor = Color3.new(1, 0, 1)
high.Contrast = 3
local blur = Instance.new("BlurEffect", lighting)
blur.Size = 10
sound:Play()
Humanoid.WalkSpeed = 5
wait(10)
high:Destroy()
blur:Destroy()
sound:Stop()
Humanoid.WalkSpeed = 16
end)
Is there something I’m doing wrong? Any help is appreciated.
Well, you are destroying the tool, which is also destroying the Script and RemoteFunction, preventing them from functioning… I would recommend using an external script.
This code is very dangerous. It allows a client to request the server-sided deletion of any instance they want.
“If you need the result of the call, you should use a RemoteFunction instead. Otherwise a remote event is recommended since it will minimize network traffic/latency and won’t yield the script to wait for a response.”
Which you aren’t needed a response as your not returning a value back to the client.
You can do many different types of Sanity Checks. I am going to use this code as an example of what you could add:
mushbreak.OnServerInvoke = function(player, tool)
tool:Destroy()
end
First, we should make sure tool , is really a tool. So that way the client can’t just delete anything they want… You should also make sure that tool is the right tool, So Ima check the Name and Make sure there is a local script Named “mushroomeffect” on the server, as well as a Handle (And I am going to check that Handle is a Part)… With these, this this the new version:
mushbreak.OnServerInvoke = function(player, tool)
if tool:IsA("Tool") and tool.Name == "Blue Mushroom" and tool:FindFirstChildWhichIsA("LocalScript").Name == "mushroomeffect" and tool:FindFirstChild("Handle"):IsA("Part") then
tool:Destroy()
else
warn("Tool failed sanity checks")
end
mushbreak.OnServerInvoke = function(player, tool)
if not tool:IsA("Tool") then return end
if tool.Name = "Blue Mushroom" and tool:FindFirstChildWhichIsA("LocalScript").Name == "mushroomeffect" and tool:FindFirstChild("Handle"):IsA("Part") then
tool:Destroy()
else
warn("Tool failed sanity checks")
end
Also, i kinda forgot to say this but the red dude said something like “it allows players to delete anything on the server”… Well, there’s a new problem. Now, player can delete other players tools whenever they fire the remote without even needing to use the tool you creating.I suggest u create a table on the server then when the remote is fired it will add the player to a table. Why do this? This is so that when they fire event the first thing you will check is that they’re not in the table, if they’re in the table then the rest of the code wouldn’t function and it would return nothing.
To address you other potential issues with this idea like " what if the player can re-use the tool every once in a while ". Well for this, when the player pickups up/gets the tool again (on the server and previous tool needs to be destroyed), you will fire a bindable event to check the table if the player is inside of there then remove them, this’ll allow them to fire the delete tool event again
They’re not the same because your code accessed the name before confirming if its an existing instance. I on the other hand check if it is an instance first (nil would make IsA return false) so the other code will run fine.
Well so My code does check if it’s an existing Instance, before we ever check the name of anything. In Luau the order matters as the engine reads line to line, left to right.
You can test this with something like this:
local function TestFunction(Object : BasePart)
if Object:IsA("BasePart") and Object.BrickColor == BrickColor.new("White") then
print("Success!")
else
print("Object is not A BasePart or BrickColor is not white ")
end
end
local ExampleOne = Instance.new("TextLabel")
local ExampleTwo = Instance.new("Part")
ExampleTwo.BrickColor = BrickColor.new("Really red")
local ExampleThree = Instance.new("Part")
ExampleThree.BrickColor = BrickColor.new("White")
TestFunction(ExampleOne) -- prints "Object is not A BasePart or BrickColor is not white "
TestFunction(ExampleTwo) -- prints "Object is not A BasePart or BrickColor is not white "
TestFunction(ExampleThree) -- prints "Success!
You can try this yourself with this function. ^
Both methods work. But how luau renders it, it wouldn’t cause this to error at all.
Now yours might be more flexable if the OP wanted to do something if the Object wasn’t a Tool instance. But both methods work perfectly fine and basicially the same in this case, as long as you order it correctly
Edit: I changed my function from using Names, into BrickColors as BrickColor isn’t a thing in TextLabels, where Names are shared with all Instances…