Hello, im starting to use more advanced programming stuff (Metatables, Self, __index) or atleast they seem advanced to me. so ive made a npc script that handles pathfinding and chasing a player and i need some feedback on how to Optimize it or just make it better in general. Thanks
local Npc = {}
Npc.__index = Npc
function Npc:Create(Damage : number, Speed : number,range,Character)
self.Damage = Damage
self.Speed = Speed
self.Range = range
self.State = "Patroling"
self.Character = Character
return setmetatable(self,Npc)
end
function Npc:Start()
for _,Player in game.Players:GetPlayers() do
local PlayerCharacter = Player.Character or Player.CharacterAdded:Wait()
if PlayerCharacter.Humanoid.Health <= 0 then return end
local DistanceFromPlayer = (self.Character.HumanoidRootPart.Position - PlayerCharacter.HumanoidRootPart.Position).Magnitude
if DistanceFromPlayer <= self.Range then
self.Character.Humanoid:MoveTo(PlayerCharacter.HumanoidRootPart.Position)
self:SetState("Chasing")
else
self:Patrol()
if self.State ~= "MovingToPatrolLocation" then
self.State = "Patroling"
end
end
end
end
function Npc:Patrol()
if self.State == "Patroling" then
local RandX,RandZ = math.random(-50,50), math.random(-50,50)
self.Character.Humanoid:MoveTo(Vector3.new(RandX,0,RandZ))
self:SetState("MovingToPatrolLocation")
self.Character.Humanoid.MoveToFinished:Connect(function()
self:SetState("Patroling")
end)
end
end
function Npc:SetState(NewState)
assert(NewState,"This state did not Work!")
self.State = NewState
end
return Npc
this is in a module script if you couldn’t tell btw
create a new array in npc.create instead of using the main class.
function Npc.new(your args)
return setmetatable({
Character = character,
--- ...other properties
}, Npc)
end
This way you’re actually instantiating different objects instead of modifying the type.
Also if you’re into types you could add a type definition for use in other scripts:
export type Npc = typeof(setmetatable({} :: {
Character : Model,
--other properties
}, Npc)
Next, don’t return from a loop, use continue instead, unless you actually want to ignore other players when one player is dead.
--use ipairs for clarity
for _, plr in ipairs(game.Players:GetPlayers() do
--...
if Character.Humanoid.Health <= 0 then continue end
--...
end
Also you’re using self.Character and PlayerCharacter.Humanoid more than once, so consider making a variable for it for readability.
This is just my opinion but I don’t like game.member statements unless they’re at the top of the script, so I’d define a Players variable, because I think that those statements cause clutter.
Can you tell me what the point of self.State is here? Because it currently doesn’t do anything and adds useless overhead from setting and checking states. Will it do something in the future?
Finally, use proper indentation.
Edit: It doesn’t really use pathfinding does it? If the target player is behind a wall it will just walk into that wall.
Consider using PathfindingService instead. (Or make your own pathfinding)
So luau is a weakly typed language, it has types, but it doesn’t enforce them.
If you declare a variable as a number and then assign a string, it won’t complain like it does in cpp.
local mynum : number = 5
mynum = "hello"
The identifier : type syntax is a type annotation, which is basically just a suggestion on what to put there.
Type annotations are for other developers that will use your fns, or your future self when you come back to your project after 5 weeks.
You can add them to fns, vars etc.
This is an example of an fn with type annotations.
local function myfn(mynum: number, mystr: string, myplayerOrnull : Player?): boolean
return myplayer ~= nil
end
The ? Indicates you may pass null(nil) as an argument here. The : type after the () indicate the type of object the fn returns. In this case a boolean.
If you want your type to be anything you can use any.
now, you can define your own types using type declarations
This is an example of a type declaration:
type name = value
This defines a type in the current context which can be used like all other types:
local var : name
the export keyword allows the type to be used outside of the module, like so:
local module = require(somthibg)
local myvar : module.name
The typename would become the name of the module + . + type outside of the module.
typeof gets the type of the object in parenthesis as a string.
You can check an objects type using it:
if typeof(obj) ~= "Instance" then
print("incorrect type")
return
end
in this case, it evaluates the table in setmetatable.
The next type thing is the type cast operator. The type cast operator can be used to convert a type to another.
local var : Part = workspace:FindFirstChild("Part") :: Part
The return type of FindFirstChild is Instance? but if you are sure that object exists and will be a part we can cast it to a part using double colons to get intellisense for it.
In this case it is casting an empty array into an array with some keys and values.
Now the type casting operator doesn’t change the underlying object, if FindFirstChild does return nil it will still be nil and not a part.
You might be wondering what the point of types is?
Well types make your code safer, it tells other people or your future self how a function should be used. It is like comments, they don’t affect the code, but are there for clarity.
It also allows you to get intellisense suggestions for a given type.
If you were to type self normally, you’d see no autocomplete suggestions, but after casting self to your type:
self = (self :: Npc)
It will suggest you api for an Npc.
If you want the analyser to warn you when you’re using wrong types, you can enable strict mode for a script by adding --!strict to the top of the script.
Intellisense will start to give you type errors
--!strict
--rest of code
Finally, I said type annotations only matter for clarity, but I lied. They also matter when using Native Compilation to optimize performance. By using type annotations, the compiler doesn’t have to guess and possibly slow down your code instead when compiling fns
@native
function add(vec, vec1)
return vec + vec1
end
Will be slower because the compiler assumes x + y is an operation for integers and assumes x and y are numbers. So it’ll generate code for numbers instead of vectors so it has to switch to vectors again which slows your code down.
function add(vec: Vector3, vec1:Vector3): Vector3
return vec + vec1
end
Now the compiler knows it has to generate bytecode for vectors.
Hope this helps!
Edit:
–
Typeof is slightly more useful because you can eg: secure your remotes from exploiters.
If you have a remote, an exploiter could fire it with invalid arguments to error the server, you can secure it by checking the type first. For example, imagine a shooter game and the client passes the instance it thinks it hit:
Remote.OnServerEvent:Connect(function(plr, ins, origin)
--some hit detect code
end
An exploiter could fire this with a fake instance to maybe damage a player it didn’t hit at all:
i could not find a way to put this into my code, make it work, and make sense.
You dont have to write a whole paragraph this time for me because i know your probably getting annoyed with this. or you dont gotta respond at all because you already helped so much.
(it just does not really work with my server script for it)
local NpcClassModule = require(game.ServerScriptService.NpcClass)
local Character = script.Parent
local Damage = 100
local Speed = 10
local Range = 15
local Npc = NpcClassModule:Create(Damage,Speed,Range,Character)
local WhileLoopCoroutine = coroutine.create(function()
while wait() do
Npc:Start()
end
end)
Character:FindFirstChild("Humanoid").WalkSpeed = Speed
coroutine.resume(WhileLoopCoroutine)
i forgot to put it in but this is where i actually create the npc
Alright, so the idea behind oop, is that you have a type(a blueprint) and you create instances(objects) of that type.
These instances own their properties and manage their own state. Let’s analyse your previous create fn to see what is wrong with it:
(I’m not annoyed)
So this is what will happen when you call create:
Inside of the Npc array returned from the module(NpcClassModule), new fields are created called Damage, Speed, Range, State, Character etc
The Npc array gets a new metatable, itself. Which is useless. Because:
When an index isn’t found in an array, lua will look through its metatable instead, to see if it is defined there. If you assign a table itself as its metatable, it will look through the table, find nothing, look through the same table again because it’s the metatable, and find nothing again. Ergo, useless second search.
Your object is ready for use, so let’s say you call :Patrol(), the global state field(the field inside Npc) will be set to patrolling.
You want to spawn a new npc, so you call create again. So:
The global fields are set to the args passed to create. Which will cause all created npcs to have the same values, because they use a global field inside Npc.
Example, consider this:
local npc = Npc:Create(..., workspace.Npc1Char)
npc:Patrol()
--some time later
local nwNpc = Npc:Create(..., workspace.Npc2Char)
nwNpc:Patrol()
--some more time later
print(nwNpc.Character, npc.Character)
--prints workspace.Npc2Char (×2)
It’s clear these instances don’t actually own or manage their own state, but rather one global state.
To fix this, you create a new array instead of using self.
--you can use this fn instead
function Npc:Create(Character: Model): Npc
local npc = {}
npc.Character = Character
--other properties
setmetatable(npc, Npc)
return npc
end
Now they will modify their own state. You do not have to change any of the other functions, because self will now equal to the correct array automatically.
Now let’s look at my function:
function Npc.new(char: Model): Npc
return setmetatable({
Character =char
}, Npc)
end
This is a short version of the previous create fn. How does it work?
Well instead of defining a local npc variable it creates the array inside of the setmetatable call, and instead of using npc.Character it just assigns the keys a value inside of the array.
Instead of setting the metatable separately it relies on setmetatable’s return value, which is the array you passed to it, the array which had their metatable set. So it does the same thing, but shorter.
Why is it called .new then?
Well, that’s just a convention, all built in types have a .new fn as their constructor(special name for the fn you call to create an obj of the given type) .
Eg: see Vector3.new, Color3.new, RayCastParams.new, TweenInfo.new
It got accepted by the community and is now widly used as the name for the constructor of a class. You don’t have to name it new though.
Now, as to why you don’t need to change any of the other fns?
Well, self is implicitly passed as the first argument to an fn inside of an array defined with the :.
When you call arr:SomeFn() self will be equal to the array that SomeFn got called from, so it will be equal to arr.
When you define an fn using the colon it internally looks like this:
--defined with colon
function arr:Some()
end
--without colon
function arr.Some(self: table?)
end
You know this because you can call it like this:
arr.Some(arr)
I suggest you try this, but anyway, obviously this looks ugly, so they added the colon definition to hide the ugly parts.
If you define a function with the . self will be nil, which is why dot definitions are used for global fns, (like .new for example!)
If your class fn doesn’t need an instance to work, use the dot. Otherwise colon looks nicer.
Now since classes and instances are just tables, the dot is also syntatic sugar to hide ugly parts!
This is how it actually looks like:
arr["Some"] = function(self: table?)
end
Which is ugly, similarly you can also call it like that:
--these are all equivalent and all call the same fn Some
arr["Some"](arr)
arr:Some()
arr.Some(arr)
So that’s why you don’t need to change the other fns!
So where does my code go in your script? It replaces the npc:Create function in your script!
You can just modify every call to the ;Create fn to .new, or if you don’t want to:
function Npc:Create(Character: Model):Npc
return setmetatable({
Character = Character,
}, Npc)
end
You can just copy paste the content of the fn
Edit: I just read your entire code, and you can use coroutine.wrap instead of .create & .resume
Like so:
But you should generally avoid infinite while loops, for sure if you’re planning to add multiple npcs. At least use RunService.Heartbeat instead.
Also use task.wait() instead of wait since it is better, and wait is deprecated
