Passing in variables to functions as arguments vs. global variables

Hi all,

I’m a beginning scripter, and have come across a fairly technical issue that I would like to get resolved before I proceed with continuing to write code for my game. This issue at hand is basically this. Say I have some variable, target, which has as its value some humanoid that an NPC should be targeting. Now, if this humanoid were to die, I want this NPC to immediately forget about the target and search for a new one. Using something like

target.Died:Connect(function()
    target = nil
end)

would achieve that effect. But if I had passed target into some other function, because variables are passed by value, that function wouldn’t notice the change. So, for example, say I have this:

function main()
    local target = searchForTarget()
    target.Died:Connect(function()
        target = nil
    end)
    moveToTarget(target)
end

Say I’ve run main(), I’ve found a target with searchForTarget(), and now the NPC should be moving to that target with moveToTarget(), where target is passed in as an argument. Now suppose target.Died fires while the NPC is moving to the target. Well, moveToTarget() is going to continue using the original value of target, which is not what I want.

The solution to this problem to me would seem to be the following:

target = nil
function main()
    target = searchForTarget()
    target.Died:Connect(function()
        target = nil
    end)
    moveToTarget()
end

and now within moveToTarget() I can now run a check like if not target then return end to get the function to stop.

However, there seems to be almost a universal consensus among programmers that using variables in this way is very bad, and I do (sort of) understand the reasons, but I just can’t see what else I’m supposed to do in this instance. It’s also very likely that I’ve misunderstood something (again, I’m new to Lua and coding altogether), so if anyone could shed some light on this topic for me, I’d very much appreciate it!

3 Likes

Is there a specific reason why you can’t just check if a Humanoid’s health is above 0 and omit them from the search case if so? I don’t think checking for the Died event is necessary here unless your NPC targets one person and continues to do so until they die. Even then you probably don’t need Died.

So, your question is really about scope. Where should you store a reference to the current target such that you aren’t running code with outdated information (i.e. a dead target).

Personally, I would use an approach such as the following:

local target = nil
local connections = {}

local attackNewTarget, clearOldTarget -- preregister functions

function attackNewTarget()
    clearOldTarget()
    target = searchForNewTarget()
    table.insert(connections, target.Died:Connect(function ()
        attackNewTarget()
    end))
end

function clearOldTarget()
    for i = 1, #connections do
        table.remove(connections):Disconnect()
    end
    target = nil
end

game:GetService("RunService").Stepped:Connect(function ()
    if target then
        moveToTarget()
    end
end)
8 Likes

I do check if the humanoid’s health is above 0 when searching for them, but the issue is that a humanoid that has initially been targeted while alive may die later on (in the game, other NPCs and players can target a given NPC, so there’s really no way to predict when that humanoid might die). So I basically need a way of updating all of the relevant functions within my script with the information that a target has died as soon as it happens.

Thanks for sharing what your approach would be. The general idea seems to be: 1. Clear old target, 2. Find a new target, 3. When the target dies, call the function again, which will repeat the process of clearing the old target and finding a new target. I can see that this would work conceptually. However, I do have a few questions, especially as related to some of the syntax that’s new to me.

In particular, what are you doing with table.insert? Why are you making a table called connections and adding and removing it? What is the function of Disconnect() here?

Also, what exactly is game:GetService("RunService").Stepped:Connect(function() doing? Is it connecting the relevant function after a certain amount of time continually?

Finally – and more with respect to my initial question – I feel like target is sort of being used as a global variable here, even though it’s called local, because it’s outside of everything in the script, so all of the functions can modify its value. I was wondering if something like this was bad practice or not. But I also don’t even know if I’m right in referring to it as “sort of a global variable.” Would appreciate some more help with this!

This depends on what other functions are in scope / which other responsibilities the script has. If you have one huge script that controls your entire game, then yeah having a variable is “too global” / not encapsulated enough / pollutes the namespace. If the script only has a single responsibility, such as running the behavior of an NPC, then it’s not a problem to have variables like these.

You can think of it like this: is it at any place in the script obvious what is meant by the variable name “target”? If your script only controls a single NPC that tries to find and move to a target humanoid, then yes it’s clear enough. If the same script defines behavior for all the NPCs in your game, it’s less obvious. A reader would have to ask “who/what/which NPC is targeting the target?”, “what does it mean to target something?”.

1 Like

It connects an anonymous1 function to the RenderStepped2 Event3 of RunService. The RenderStepped event is fired once every frame, causing the anonymous function that was connected to be called every frame. The effect is to check every frame if there is a target, and in that case move to the target.

Connecting a function to an event returns a Connection4 object. Calling the Disconnect method of a Connection object causes the connection to cease to exist, meaning the function that was connected will no longer be called every time the Event fires.

If a Connection comes out of scope, it won’t be garbage collected. It’ll just continue to exist for as long as the Instance whose event it’s connecting. Storing the connection in a variable or a table means that we can go back later and disconnect it when we no longer want it. In this case it’s unnecessary to use a table, because there will never be more than 1 active connection. The only point where new connections are made is in the attackNewTarget function, which already makes sure to clean up after any previous calls to attackNewTarget. A simple variable would have been enough.

3 Likes

I think it’s possible that the advice you received about not using global variables wasn’t exhaustive or perhaps was talking about the global table _G which is global across all scripts in the game, as opposed to having a global variable in the way you’ve described.

As a programmer it’s all about picking the most appropriate place to put.your variables from the available options. Certain programmers will shout things like “never do this” and “never do that” without providing context, and it’s not a good idea to listen to these without understanding the context in which they’re speaking. There are very few absolutes in programming, and asking 10 programmers to make the same thing will produce 10 different answers.

You can put variables global to the current script’s functions and that’s totally fine. If you have a massive script and this is just one chunk of it, you can always put it all in a do … end structure to create your own little scope for anything target related so only the functions inside the do end block can see one another and any variables defined in there. It’s all dependent on your specific script and your specific situation.

7 Likes

I think most of these questions have been answered sufficiently by @ThanksRoBama, let me know if they haven’t.

1 Like

This has all been very helpful. Thanks @ComplexGeometry, @ThanksRoBama, @BanTech for the explanations.

2 Likes