Scripting Helpers is winding down operations and is now read-only. More info→
Ad
Log in to vote
0

My NPC dialog arrest selection won't work? (FE)

Asked by 6 years ago

I have tried it a couple of times but it does not seem to want to work I could be messing up majorly due to I normally stick with normal FE and have never touched an NPC custom script before, but am giving it a try below I have included the full script, and as the title of the question states it is filtering enabled (FE)

local player = game:GetService("Players").LocalPlayer
local dialog = script.Parent

dialog.DialogChoiceSelected:connect(function(player, choice)
    if choice == script.Parent.DialogChoice.ChoiceA then
    game.ReplicatedStorage.RemoteEvents.SubractEXP:FireServer(5,player)
    script.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent:Destroy()

    elseif choice == dialog.DialogChoice.ChoiceB then
    game.ReplicatedStorage.RemoteEvents.AddEXP:FireServer(25,player)
    script.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent.Parent:Destroy()
 end
end)

and here is a picture of the explorer.

https://gyazo.com/f3dfd75997c56f06169a9d172147ab49

P.S Please leave a detailed answer so that I may learn for the future!

0
The DialogChoiceSelected function doesn't work with FilteringEnabled. Precisionly 103 — 6y
0
Also, someone could just do "game.ReplicatedStorage.RemoteEvents.SubractEXP:FireServer(math.huge, game.Players.myworstenemy)" and they would be bankrupt. hiimgoodpack 2009 — 6y

1 answer

Log in to vote
1
Answered by 6 years ago
Edited 6 years ago

Problems:

  1. Try never to have that many .Parents in a row -- it's highly error prone. In this case, I can't tell what you're trying to Destroy, and it almost certainly isn't what you want.
  2. You only access the first DialogChoice before trying to access ChoiceA, but you have several in your hierarchy. You need to get the bottom DialogChoice, not the top. (I will demonstrate below how to do this.)
  3. Consider assigning things to local variables (like choiceA) at the beginning of your script so that you don't have long chains of "this.that.other". It makes it easier to follow.
  4. Due to a long-standing bug, DialogChoiceSelected only works on the client. You appear to be trying to do this, but your script needs to be a LocalScript in a place where LocalScripts run.
  5. The whole point of FilteringEnabled is to try and stop exploiters/hackers from messing up your game. However, it has weaknesses -- exploiters can have full control over their own client, allowing them to activate arbitrary scripts. Most of what the script does will not replicate to the server, but the following does:
  • RemoteEvent/RemoteFunction calls. If their script activates a RemoteEvent/RemoteFunction, Roblox cannot tell the difference between a legitimate use and an illegitimate one. For example, a person could call "AddEXP" to give any player any amount of XP (or, as hiimgoodpack mentions, to take it all away). (Note that you needn't pass in the player when calling a RemoteEvent/RemoteFunction because this is done automatically and safely by Roblox -- the first argument you receive on the server is always the player who sent in the request).
  • Character animations/position. Players can use this to teleport their player, move their player more quickly than they should, jump in mid-air, etc.
  • Any unanchored parts that are "owned" by the client (see NetworkOwnership) can be manipulated (at least their position/velocity).

Therefore, you should never have RemoteEvents that can be used to add an arbitrary amount of money or experience. Instead, tell the server what the client did (or wants to do) and the server can choose how much money/experience to give them.

A better script follows (with some "TODOs" that you need to fix). Note that it must be a LocalScript in a place where LocalScripts run (ex PlayerGui) -- NOT in the workspace.

local remoteEvents = game.ReplicatedStorage.RemoteEvents
local dialog = --TODO path to dialog -- it cannot be "script.Parent" because the LocalScript won't run in the workspace. If there will only ever be one NPC, you might do "workspace.NPC"
--Method 1 for getting "bottomDialog" (inferior if you might change how many DialogChoices there are as it won't adapt automatically)
local bottomDialog = dialog.DialogChoice.DialogChoice.DialogChoice.DialogChoice.DialogChoice
--Method 2
local bottomDialog = dialog
local nextDialog = dialog
while true do
    nextDialog = nextDialog:FindFirstChild("DialogChoice")
    if not nextDialog then break end
    bottomDialog = nextDialog
end
--TODO Pick Method 1 or 2 and delete the other one.

dialog.DialogChoiceSelected:Connect(function(player, choice)
    if choice == bottomDialog.ChoiceA then
        remoteEvents.NPCDialogChoiceA:FireServer()
    elseif choice == bottomDialog.ChoiceB then
        remoteEvents.NPCDialogChoiceB:FireServer()
    else -- TODO 'else' and 'return' only needed if you're doing to destroy something after the 'if'
        return
    end
    -- TODO you tried to destroy something in your script. If it's to be destroyed only for the player, feel free to destroy it here (as it won't replicate to the server) - also make sure to record on the server that the player has already sent in either ChoiceA or ChoiceB (to make sure an exploiter can't call those events multiple times). If it's to be destroyed for everyone, the server must take care of that.
end)

Note that the above script assumes you create NPCDialogChoiceA and B in your remote events and then handle them appropriately (by adding/removing experience as desired). Also, don't forget that an exploiter could call ChoiceA or ChoiceB at any time (and as many times as they like). The server should consider making sure that these aren't called multiple times per second (which would be impossible if you used the dialogs without exploiting) and that the NPC is still offering these choices to the player.

Notably, this is not a very general solution - you have to add a new event for every possible response. An alternate solution is to simply tell the server about every "DialogChoiceSelected" and let the server figure out everything else. This simplifies the client script, though the server is then responsible for all DialogChoices throughout the entire workspace. If you chose to do this, the "Connect" function in the above script would be reduced to one line: remoteEvents.DialogChoiceSelected:FireServer(choice), though a superior script would automatically know/find all dialogs and automatically connect them to this RemoteEvent. You could do this by using DescendantAdded to look for new dialogs and a recursive algorithm to find all pre-existing dialogs.

For future debugging purposes, make sure to check the Output window for errors. If none occur, make sure that the script is running at all (ex put a print statement at the top of the script). Check out Scripting Helper's blog on debugging with print statements. If you're testing online, use F9 to open the developer console.

Ad

Answer this question