Well, i have a bad laptop, and if i want to test my game, to load it takes 10-20 minutes, and i prefer ask you something about a script im testing, about firing events, well in my game if you touch something you will get cash but i want to make something more efficient, this is the script (if this works then you can use it on your game for free)
function onTouch(hit)
local h = hit.Parent:FindFirstChild("Humanoid")
if h ~= nil then
local ReplicatedStorage = game:GetService("ReplicatedStorage")
ReplicatedStorage.ResearchReward:FireServer()
end
end
--The code to send request to server
local Event = game:GetService("ReplicatedStorage")
Event.ResearchReward.OnServerEvent(function(player)
local leaderstats = player:FindFirstChild("leaderstats")
if leaderstats ~= nil then
local cash = leaderstats:FindFirstChild("Cash")
if cash ~= nil then
cash.Value = cash.Value + 10
end
end
end)
--Rewarding Player
So, this script works? that’s my question Thanks for responding.
Provided that the first code sample is a LocalScript and the second one is a server script, yes this code would work.
Guess I’ll take the time to note, albeit off-topic, that you should consider validation for your code. The first code should verify, as a LocalScript, if the part that touched the reward giver is part of the LocalPlayer’s character. The second code should check if the requesting player’s character is on the reward brick and change cash only if so. Both codes should implement some kind of debouncing.
This should work, but you will need to implement debounce yourself
Alright Here is what we will do :
--[[ Server--------------> Client
Client-------------->Server
and lastly, again, Server------------------>Client
Here is how ,
first a script in the part, a ServerScript
local players = game:GetService("Players")
script.Parent.Touched:Connect(function(hit)
local touchedPlayer = players:GetPlayerFromCharacter(hit.Parent)
if touchedPlayer then
coroutine.wrap(function()
local event = game.ReplicatedStorage.CashEvent--do getService rep..instead, did this for simplicity
event:FireClient(touchedplayer)
end)
basically we FireClient so that a local script can run , which will be in
StarterPlayerScripts with this code :
local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local player = Players.LocalPlayer
local event = ReplicatedStorage:WaitForChild("CashEvent")
local verify = ReplicatedStorage:WaitForChild("VerifyEvent")
event.OnClientEvent:Connect(function()
verify:FireServer()
end)
Lastly after the client fires the event , a script in serverscriptservice:
local Event = game.ReplicatedStorage.VerifyEvent
--when verify event fires
Event.OnServerEvent:Connect(function(Player)
--give him the cash
Player.leaderstats.Cash.Value=Player.leaderstats.Cash.Value+100
end)
This will probably also confuse exploiters with events lol
Summary : This will fire an event for a player , the one who touched the part, then a local script will run because of that (Fire:Client() ) and because of that, in turn a serverscript in ServerScriptService will run due to another event fired from the local script
the verify EVENT , i just named it that for some reason . That is there so when the client is “invoked”, another event is fired for that specific client upon which an action is done (and that event is just named “Verify” nothing more) .The function just fires the event , so the it can be connected to when there is and event.OnClientEvent, it will connect it to the function , that fires another event
edit * : I edited the script into one function for you (script 2)
first of all lets start with the line
local Players = game:GetService("Players")
-- this assigns the Players a variable so we can use it later on
local ReplicatedStorage = game:GetService("ReplicatedStorage")
--get replicated storage
local player = Players.LocalPlayer
--for one (local) player do this
local event = ReplicatedStorage:WaitForChild("CashEvent")
--first get the Cash Event , the event that was fired by the previous Script
local verify = ReplicatedStorage:WaitForChild("VerifyEvent")
-- This is the second event that we will fire after..
event.OnClientEvent:Connect(function()
-- then after the previous Script fired the Event "CashEvent" , for the player , we will ..
verify:FireServer()
--we will fire the Second server Event
end)
after this event is fired (meaning after the Verify event is fired ), the in the ServerScriptService runs another script When Verify event is fired
local Event = game.ReplicatedStorage.VerifyEvent
--when verify event fires
Event.OnServerEvent:Connect(function(Player)
--give him the cash
Player.leaderstats.Cash.Value=Player.leaderstats.Cash.Value+100
end)
It’s ineffecient and exploitable to have 2 events like this, the exploiter could just keep firing the event to gain Cash.
You may as well just use a RemoteFunction which OnClientInvoke returns true.
For example, in the Script:
local verify = RemoteFunction:InvokeClient(touchedplayer)
if verify then
--Reward them
end
Then inside the LocalScript:
RemoteFunction.OnClientInvoke = function()
return true
end
Following colbert2677’s advice, both your - and even XxELECTROFUSIONxX’s - server-side code, is easily exploitable, as there is no (server-side) validation checks of any kind (and it does not help trying to attempt “obfuscate” the Events’ communication sequence, when the final OnServerEvent still has no validation checks).
-- Server script, without any validation, easily exploitable
MyCustomNamedRemoteEvent.OnServerEvent:Connect(function(player)
-- When not checking the player's distance to the "cash-giver",
-- anyone - even miles away - will be able to fire this event.
player.leaderstats.Cash.Value = player.leaderstats.Cash.Value + 100
end)
A slightly “better” approach, would be to at least verify the player’s distance to the “cash-giver” part:
-- Server script, with simple distance validation, though still
-- missing debounce and other validation checks or cool-downs.
MyCustomNamedRemoteEvent.OnServerEvent:Connect(function(player)
if not player or not player.Character or not player.Character.HumanoidRootPart then
-- Player does not even have a valid character
return
end
local characterPosition = player.Character.HumanoidRootPart.Position
-- Calculate distance between player's (server-side) character position
-- and the part's position that is supposed to be touched.
-- If distance is less than 5 studs, then it is accepted.
if (characterPosition - cashGiverPart.Position).magnitude < 5 then
--print("Player",player.Name,"is within reach of",cashGiverPart.Name)
player.leaderstats.Cash.Value = player.leaderstats.Cash.Value + 100
end
end)
Don’t use OnClientInvoke of a RemoteFunction. A client can permanently hang your server by overwriting the function assigned to OnClientInvoke with a blank function that does not return a value. The server will be yielding for a value it will never get.
Scratch the idea of RemoteFunctions since colbert has corrected me.
Indeed, they can just find out about a RemoteEvent in a place and continously :FireServer it.
Debounce is required here, this debounce will be for each player individually.
I usually setup a metatable which for __index is true (if the player hasn’t been added to the table).
Here’s an example of what I mean:
local cooldown = 2 --How many seconds between each
local debounces = setmetatable({}, {
__index = true
})
--Then inside the OnServerEvent, player is the Player who fired the event
if debounces[player.UserId] then
debounces[player.UserId] = false
--Other code
wait(cooldown)
debounces[player.UserId] = true
end
@colbert2677 I’d like to know then, how could I get around that?
What’s the best way to check the exploiter hasn’t reassigned the function.
You don’t check because you can’t check. A method I have seen is to wrap OnClientInvoke in a separate thread and return any potential result to an outside thread. A wrapper function was posted to the function before, though not only can I not find it but it’s not recommended because for every OnClientInvoke that doesn’t return, that’s an alive thread that doesn’t get terminated, thus memory leak among other problems.
You don’t ever want the server to rely on the client to pass it information. It can accept such information but it should never wait for the client to reply. It should take action after a specified amount of time or do that check itself. I don’t know what the context behind your code is but I think that people were building off of my reply.
By verification and all, I mean that the client and the server each host their own debounce. The LocalScript should check to see if the part passed to Touched is part of the LocalPlayer’s character and if so, fire off the event. The server, on the other hand, should check presence on a part via regions or downcasting from the root to the given part. In shorter terms: the server should be checking if the touch is possible to occur at the time of the event being fired.
I feel like you should really just base this all on the server. It could actually help in a long shot, as exploiters could easily run any for loops or while loops to give them cash.
Try something like this instead.
local Players = game:GetService("Players")
function TouchPart(Hit)
local Humanoid = Hit.Parent:FindFirstChild("Humanoid")
if Humanoid and Hit.Parent ~= workspace then
local Player = Players:GetPlayerFromCharacter(Hit.Parent)
if Player then
local Leaderstats = Player:FindFirstChild("leaderstats")
if Leaderstats then
local Cash = Leaderstats:FindFirstChild("Cash")
if Cash then
Cash.Value = Cash.Value + 10
end
end
end
end
end
This would fix any remote firing problems if exploiters tried to abuse this power. Do think about exploiters and how they could exploit your remotes.
Thanks for responding.