I am trying to make a quest system where you can redo the quest again after you finish the quest; I have tried to do this with a Boolean value but it doesn’t seem to work, as it always returns true even when I have the quest.
Here’s the script:
local cd = script.Parent:WaitForChild("NPCInteraction")
local event = game:GetService("ReplicatedStorage"):WaitForChild("RemoteEvents"):WaitForChild("OpenQuest")
local canGetQuest = true
cd.MouseClick:Connect(function(player)
if canGetQuest then
event:FireClient(player,canGetQuest)
canGetQuest = false
end
if not player.PlayerGui:WaitForChild("Quest").Enabled then
canGetQuest = true
else
canGetQuest = false
end
print(canGetQuest) -- always prints 'true'
end)
Alright, it sends the value whether or not enabled or enabled; Problem is, it still sets canGetQuest as false.
Here’s the new script:
local cd = script.Parent:WaitForChild("NPCInteraction")
local event = game:GetService("ReplicatedStorage"):WaitForChild("RemoteEvents"):WaitForChild("OpenQuest")
local canGetQuest = true
local event2 = event.Parent:WaitForChild("NewQuest")
cd.MouseClick:Connect(function(player)
if canGetQuest then
event:FireClient(player,canGetQuest)
canGetQuest = false
end
event2.OnServerEvent:Connect(function(plr,guiEnabled)
if not guiEnabled then
canGetQuest = true
else
canGetQuest = false
end
print(guiEnabled)
end)
print(canGetQuest) -- prints false even though the gui is not enabled
end)
You’re creating a memory leak by having a connection inside of another connection. You would now need to wait until the client fires back (ie. local player, guiEnabled = event2.OnServerEvent:Wait() until player == playerFromClickDetector) to the server before printing canGetQuest, or just restructure your logic.
What I’d do personally is just do everything affiliated with the click detector on the client. If you need the server to return back something to the client, you can use a remote function in place of a remote event, however in either case the logic can be exploited because the player can still fire the remote with what ever values they want.
What I’d do personally to avoid granting the player the ability to exploit your logic is to simply hold all cooldowns, player statuses etc on the server
-- client
clickDetector.MouseClick:Connect(function(player)
assert(player == localPlayer) -- just to be safe
local canGetQuest, quest = getQuestFunction:InvokeServer()
if canGetQuest then
-- do what ever you need with the quest information
end
end)
-- now here you would dynamically fire the server as parts of the quest are completed, ensure you do sanity checks and whatnot on the server to again minimize the ability to exploit your quests
Now on the server,
local remoteFunction = path.to.remote
local playerQuestInfo = {}
remoteFunction.OnServerInvoke = function(player)
if not playerQuestInfo[player] then -- if the player doesn't have any active quest in progress,
local questInfo = getQuestInfo()
playerQuestInfo[player] = questInfo
return true, questInfo -- return true because the player doesn't have any quest in progress, and then generate a quest for them to complete.
end
end
-- now again on the server,
questObjective.OnServerEvent:Connect(function(player, objectiveName)
-- suppose you do your magnitude checks, requirement checks etc in this function, let's say objectiveName is "FinishObby" or something
local activePlayerQuest = playerQuestInfo[player]
-- if no player quest exists right now,
if not activePlayerQuest then
return
end
-- now, we set the player status to true
activePlayerQuest[objectiveName] = true
-- now check if all objectives have been completed in another function "checkIsQuestCompleted"
local isQuestCompleted = checkIsQuestCompleted(activePlayerQuest)
if isQuestCompleted then
-- now here, reward the player for completing the quest if you want, etc
-- now set the player's index in the quest info to nil because they no longer have an in progress quest
playerQuestInfo[player] = nil
end
end)
The biggest takeaway here is to put in as little trust to the client as posisble.