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

Is this the correct way of using Recursive functions?

Asked by 9 years ago

I attempted to put it into use by creating a script that will bring up a Gui if the player steps into a certain range of a Part containing the String Value named "Interaction"

Does this look right?

function GetDescendants(ancestor)
local descendants = {}

local function recursiveGetChildren(object)
pcall(function()
if #object:GetChildren() == 0 then return end
for i,v in pairs(object:GetChildren()) do
descendants[#descendants+1] = v
recursiveGetChildren(v)
end
end)
end

recursiveGetChildren(ancestor)

return descendants
end


for i,v in pairs(GetDescendants(game)) do


function findInteract(part)
    if (part ~= nil) then
        if (part:IsA("BasePart")) then
            if (part:findFirstChild("Interaction")) then
                local Interaction = part.Interaction
                if (Interaction:IsA("StringValue")) then
                    return Interaction;
                end
            elseif (part.Parent:findFirstChild("Interaction")) then
                local Interaction = part.Parent.Interaction
                if (Interaction:IsA("StringValue")) then
                    return Interaction;
                end
            end
        else
            return nil;
        end
    end
end

end

--Run Interaction


function RunInteraction()

function GetGui()
    local InteractGui = game.ReplicatedStorage.InteractionGui
end

local Gui = GetGui() -- incase the name changed
local InteractPart = GetDescendants(ancestor) --Get the part that is a parent of the StringValue
Gui.Name = script.Parent.Parent.Name .. "InteractGui"
script.Parent.Parent.Changed:connect(function() GetGui().Name = script.Parent.Parent.Name .. "InteractGui" end)
while true do wait(.1)
    for i, v in pairs(game.Players:GetChildren()) do
        if v.Character ~= nil then
            if v.Character:FindFirstChild("Torso") ~= nil and (InteractPart - v.Character.Torso.Position).magnitude <= 3 and v.PlayerGui:FindFirstChild(Gui.Name) == nil then
                Gui:Clone().Parent = v.PlayerGui
            elseif v.Character:FindFirstChild("Torso") ~= nil and (InteractPart - v.Character.Torso.Position).magnitude > 3 then
                if v.PlayerGui:FindFirstChild(Gui.Name) then v.PlayerGui[Gui.Name]:Remove() end
            end
        end
    end
end 
end

1 answer

Log in to vote
5
Answered by
BlueTaslem 18071 Moderation Voter Administrator Community Moderator Super Administrator
9 years ago

Tab your code correctly.


The recursive GetDescendants looks correct, but I would suggest defining it differently for cleanliness.

It's bad to define functions inside of functions (it's not performant, and it's confusing), especially when it's not necessary. If we make descendants a parameter, then we don't need to make a new function, nor we do we need a separate functions for GetDescendants and recursiveGetChildren.

Let's not use pcall, either, since that's a crummy solution. In this case you should only be caring about physical things, you don't need to worry about errors -- just use workspace instead of game (which will be "more correct" anyway since things hidden in ReplicatedStorage or wherever won't trigger it for no apparent reason).

-- REQUIRES: model and descendants of model can have :GetChildren() called
-- on them.
-- PURPOSE: Given a model, returns a list of all of the descendants of that model.
-- [If result is specified, it will add the descendants of result to model.]
function descendants( model, result )
    result = result or {} -- No need to pass in result
    for _, child in pairs(model:GetChildren()) do
        table.insert(result, child) -- Much cleaner way to add to list
        descendants(child, result)
    end
    return result -- Return the accumulated children
end

There's no reason to define findInteract inside of a loop. Though findInteract also isn't used, so I'm not sure why you included it.

I would suggest instead making this function:

-- PURPOSE: Returns whether or not this part is an interaction site.
-- REQUIRES: part must be a ROBLOX object.
function isInteraction( part )
    return part:IsA("BasePart")
        and part:FindFirstChild("Interaction")
        and part.Interaction:IsA("StringValue")
end

Now we can easily filter the list of everything with just the interactable parts:

local interactions = {} -- List of all interaction sites.
local everything = descendants( workspace )
for _, object in pairs( everything ) do
    if isInteraction( object ) then
        table.insert( interactions, object )
    end
end

GetGui does absolutely nothing. It doesn't return anything, and has no side effects. Despite not having anything to do with its enclosed function, you have defined it inside of another function. Bad.

The remainder can be otherwise much simpler.

-- Define the GUI we give to players
local interactionGui = game.ReplicatedStorage.InteractionGui

-- REQUIRES: player is a Player, they have a Character (with torso) and PlayerGui.
-- PURPOSE: Shows or hides (by deleting/inserting) the InteractionGui
-- based on whether or not they are close to any interaction site (in `interactions`)
function update(player)
    -- We defined the table of "interactions" above
    local torso = player.Character.Torso
    local near = nil -- Near nothing.
    for _, interaction in pairs(interactions) do
        if (interaction.position - torso.Position).magnitude <= 3 then
            -- Within three studs of interaction's center
            near = interaction
        end
    end

    local playerGui = player.PlayerGui
    local hasGui = playerGui:FindFirstChild( interactionGui.Name )
    if near then
        -- I am near an interaction
        if not hasGui then
            interactionGui:Clone().Parent = playerGui
        end
    else
        -- I am NOT near ANY site
        if hasGui then
            hasGui:Destroy()
        end
    end
end

-- Update all players
function updateAll()
    for _, player in pairs(game.Players:GetPlayers()) do
        if player.Character
            and player.Character:FindFirstChild("Torso")
            and player:FindFirstChild("PlayerGui") then
            update(player)
        end
    end
end

while wait(0.1) do
    updateAll()
end

This is overall much simpler and cleaner.

Slow down. When writing code, think about what needs to be done. Describe in words a function that needs to be done, and figure out what things it might need to do that other functions could do.

Once you have that tiny description, write out the code.


Since all you're doing is deleting and inserting the interaction gui, it might make more sense to just show or hide it. That would mean there would be less to think about and keep track of.

1
Thank you, this helped clarify so much! It also changed my understanding on how code is written. :) IntellectualBeing 430 — 9y
Ad

Answer this question