Don't know why argument is nil

Help and Feedback Scripting Support
#1

I posted earlier about creating a spellcasting system with OOP. I have encountered an issue with a class in which it says the Player parameter is nil even though I have defined it.

image
image769Ă—23 2.08 KB

Here is the LocalScript code:

local Player = game.Players.LocalPlayer
-- Input (To Server)
UserInputService.InputBegan:Connect(function(key,chatting)
	if chatting or Move_Break then return end
	local B = tostring(key.KeyCode) 
	Move_Break = true delay(1,function() Move_Break = false end)
	for i,v in pairs(MoveModules) do
		local Move = require(v)
		if Move.Key == B then 
			local NewMove = require(v)
			NewMove.new(Player.Name, Move.Element, Move.Cooldown, Move.Stamina, Move.LastUsed, Mouse.Hit)
			NewMove:DoMove()
		end
	end
end)

And this is the module:

Move = require(script.Parent.Parent.Essentials.Move)

local AirBlast = {

-- Settings
Key = Enum.KeyCode.G,
Name = "AirBlast",
Stamina = 30,
Element = "Air",
Cooldown = 6,
LastUsed = 0,	

}

AirBlast.__index = AirBlast
setmetatable(AirBlast,Move)

function AirBlast.new(Player, Element, Cooldown, Stamina, LastUsed, Mouse)
	local newAirBlast = Move.new(Player, Element, Cooldown, Stamina, LastUsed)
	setmetatable(newAirBlast, AirBlast)
	newAirBlast.Mouse = Mouse
	return newAirBlast
end

function AirBlast:DoMove()
	print(self.Player)
	local now = tick()
	if now - self.LastUsed > self.Cooldown then 
		self.LastUsed = now
		local BodyGyro = Instance.new("BodyGyro", game.Players[self.Player].Character.HumanoidRootPart)
		BodyGyro.MaxTorque = Vector3.new(0,math.huge,0)
		BodyGyro.CFrame = CFrame.new(game.Players[self.Player].Character.HumanoidRootPart.CFrame.p,self.Mouse.p)
		BodyGyro.P = 1e9
		delay(0.3, function() BodyGyro:Destroy() end)
		self.Player.Humanoid.WalkSpeed = 0
		self.Player.Humanoid.JumpPower = 0
		local AirBlastAn = game.Players[self.Player].Character.Humanoid:LoadAnimation(game.ReplicatedStorage.BendingResources.AirBending.Animations.AirBlast)
		AirBlastAn:Play()
		AirBlastAn:AdjustSpeed(1.5)
	end
end


return AirBlast

Whenever :DoMove is called, self.Player is nil even though it is defined in the MainLocalScript, can anyone explain why this is happening?

#3

The value is nil, it is not getting a value at all.

#4

If you print player in the AirBlast.new function, is it still nil?

#5

Yes, it is still nil.
I think that the class does not like local values being passed through it, even though it is in a LocalScript.

I came to this conclusion since not only was Player.Name nil, Mouse.hit was nil as well.

The way I worked around this was changing my module’s function to accept parameters:

function AirBlast:DoMove(Target, Mouse)
	local now = tick()
	if now - self.LastUsed > self.Cooldown then 
		self.LastUsed = now
		local BodyGyro = Instance.new("BodyGyro", game.Players[Target].Character.HumanoidRootPart)
		BodyGyro.MaxTorque = Vector3.new(0,math.huge,0)
		BodyGyro.CFrame = CFrame.new(game.Players[Target].Character.HumanoidRootPart.CFrame.p,Mouse.p)
		BodyGyro.P = 1e9
		delay(0.3, function() BodyGyro:Destroy() end)
		game.Players[Target].Character.Humanoid.WalkSpeed = 0
		game.Players[Target].Character.Humanoid.JumpPower = 0
		local AirBlastAn = game.Players[Target].Character.Humanoid:LoadAnimation(game.ReplicatedStorage.BendingResources.AirBending.Animations.AirBlast)
		AirBlastAn:Play()
		AirBlastAn:AdjustSpeed(1.5)
		delay(1, function()
			game.Players[Target].Character.Humanoid.JumpPower = 65
        	game.Players[Target].Character.Humanoid.WalkSpeed = 26
		end)
	end
end

And changing the LocalScript to this:

NewMove.new(Player.Name, Move.Element, Move.Cooldown, Move.Stamina, Move.LastUsed, Mouse.Hit)
NewMove:DoMove(Player.Name, Mouse.Hit)

If anyone else can explain why this is the case though, and can provide a solution with the previous code, please let me know.

#6

Could I see the module where Move.new is made?

#7

Sure.

local Move = {}
Move.__index = Move

-- Creates framework for all future moves
function Move.new(Player, Element, Cooldown, Stamina, LastUsed)
	local newmove = {}
	setmetatable(newmove, Move)
	newmove.Element = Element 
	newmove.Cooldown = Cooldown
	newmove.Stamina = Stamina
	newmove.LastUsed = LastUsed
	newmove.Player = Player
	return newmove
end

return Move

This is the module for Move.new

#8

I think that the problem is that you’re trying to setmetatable on an already set metatable, so alternatively you could just return a table created based off all the passed arguments because you aren’t really utilizing the point of OOP with the Move module.

#10

This created a whole new error,
image

And to the previous reply I was intending on the Move module being a superclass since not all moves use the mouse’s location, i.e AOE attack.

#11

ok i just noticed the error, try doing this

local Move = {}
local newmove = {}

Move.__index = Move
setmetatable(Move,newmove)

-- Creates framework for all future moves
function Move.new(Player, Element, Cooldown, Stamina, LastUsed)
	newmove.Element = Element 
	newmove.Cooldown = Cooldown
	newmove.Stamina = Stamina
	newmove.LastUsed = LastUsed
	newmove.Player = Player
end

return Move
#12

@RomeoEDD OOP convention says that a constructor should be dot syntax. Changing it to a colon doesn’t make it a constructor and instead a member function, which is a silly mutation of OOP. As for your new post, that’s not how you handle object construction with OOP.

@Rhaoke Metatables can be nested. I don’t explicitly remember how that’s done, but it’s in regards solely to the __index metamethod.

Overall, there are a couple of OOP practice mistakes that should be corrected as far as the Move module goes.

  • If you set the metatable first, then setting indices in your table can invoke metamethods. You should be using rawset in this case.

  • Constructing a table without all its elements pre-filled is slightly more costly than not. I’d suggest writing your module a different way, such as filling the elements first and then setting the metatable or doing that all in one go.

  • If Move doesn’t need to be an object, don’t force it into OOP convention. This is assuming that you don’t have any member methods attached to the Move table which serves as your base class. Just return a standard table.

  • If you’re following Lua conventions, arguments should be camelCase.

local Move = {}
Move.__index = Move

-- Get rid of any metatables if there are no member functions to Move
function Move.new(player, element, cooldown, stamina, lastUsed)
    local newMove = setmetatable({
        Player = player,
        Element = element,
        Cooldown = cooldown,
        Stamina = stamina,
        LastUsed = lastUsed
    }, Move)

    return newMove
end

return Move

If Move is supposed to serve as a base class and AirBlast, for example, is meant to serve as an inherited subclass, then don’t set a metatable for Move, just have it return a table of data that you can add member functions to through your inherited class.

1 Like
#13

But in the other function when he wants to get the player how would it be done because there is no metabtable attached to the move module itself so when it references it would it be nil.

#14

Thank you for your reply, I am very new to OOP and am not very familiar with the ins and outs of how all of it works, I will try what you have provided for me.

#15

Multiple arguments are being returned back as nil, meaning the constructor is invalid or not receiving arguments as intended (which I’m still trying to figure out). Changing the syntax only passes self as the first argument but that doesn’t explain why several subsequent parameters are nil.

In OOP, object construction is not done with colons.

#16

wouldnt you just need to attach the newMove as a metatable to the Move module itself so when it first notices that the module itself doesnt not have the index it will check if the metatable would have it like i did in this line -

 setmetatable(Move,newmove)
--so this is how it should be done:

local Move = {}
Move.__index = Move

-- Get rid of any metatables if there are no member functions to Move
function Move.new(player, element, cooldown, stamina, lastUsed)
    local newMove = setmetatable(Move,{
        Player = player,
        Element = element,
        Cooldown = cooldown,
        Stamina = stamina,
        LastUsed = lastUsed
    })
    --return newMove -- i am not sure why this is here you dont have the function equal to a variable when your calling it in the local script
end

return Move
#17

To provide you with more information on what exactly I am trying to do, as said in OP I posted earlier about making a move system, with different elements.

One of my friends, Stratiz, suggested I used OOP to load the move modules for a certain element into a table and iterate through them to determine which one was needed based on a keypress.

This is why I was using OOP in the first place.

#18

The problem with the code you provided is that the table is static and constructed only once.

local Move = {}
local newmove = {}

This means that every time the module is required, it’s going to see the same table. For OOP-in-Lua, objects are represented by tables or userdata (created via newproxy). When you call the new method of your pseudoclass, the code calling new expects a new object to be returned. Thus, rather than creating it as an upvalue, you’re meant to create it in the function itself.

The main purpose of metatables in classes is attaching member functions to created objects. The way you’ve done it is backwards and incorrect. What your code does is attempt to set a new table as a metatable to the Move table without __index.

Some of these articles may help you understand the choices I made in writing out that code:
https://devforum.roblox.com/search?q=oop%20category%3A71

You’ll notice a pattern of how to create objects.

This is here so that a module that calls new can receive the newly created object. Without it, Move.new is creating a blank table without references that’ll just get discarded and garbage collected (it’s purely data, minus the Player if it’s an Instance).

1 Like
#19

Would you mind if I contacted you in the future about OOP since I have seen little resources, other than of course, this post:

I promise I only want to learn. :smiley:

1 Like
#20

i think i know what your are doing wrong first change the local script to this

local Player = game.Players.LocalPlayer
-- Input (To Server)
UserInputService.InputBegan:Connect(function(key,chatting)
	if chatting or Move_Break then return end
	local B = tostring(key.KeyCode) 
	Move_Break = true delay(1,function() Move_Break = false end)
	for i,v in pairs(MoveModules) do
		local Move = require(v)
		if Move.Key == B then 
			local NewMove = Move.new(Player.Name, Move.Element, Move.Cooldown, Move.Stamina, Move.LastUsed, Mouse.Hit)
			NewMove:DoMove()
		end
	end
end)

then in the module script do this

local Move = {}
Move.__index = Move

-- Get rid of any metatables if there are no member functions to Move
function Move.new(player, element, cooldown, stamina, lastUsed)
    local newMove = setmetatable({
        Player = player,
        Element = element,
        Cooldown = cooldown,
        Stamina = stamina,
        LastUsed = lastUsed
    }, Move)

    return newMove
end
return Move
#21

You removed this, which means the module isn’t defined at all.

local NewMove = require(v)
#22

The information is helpful. Sometimes I struggle a bit with OOP, especially in regards to inheritance. That’s what I see your code is trying to accomplish; Move is a superclass and AirBlast is an inherited subclass. I don’t know if there are any articles on OOP inheritance but they’re worth taking a look.

What I’m thinking is that you will want Move to return a table of pure data and not make it a pseudoclass, so that AirBlast (an inherited class) can add member functions to it. For example, we’ll have the Move module return some data and then attach the AirBlast functions on.

Here’s what I’m thinking roughly would work out. Keep the Move class the same but don’t actually reference it. Your move modules will instead take advantage of the Move’s constructor to make a data table before adding things to it. If AirBlast doesn’t need to create any properties in the table and only contains functions to be attached, all the better.

Move constructor:

local Move = {}

function Move.new(args)
    return dictionaryWithoutMetatable
end

return Move

AirBlast module:

local AirBlast = {}
AirBlast.__index = AirBlast

local Move = require(pathToMoveModule)

function AirBlast.new(args)
    local newMove = Move.new(args)
    return setmetatable(newMove, AirBlast)
end

function AirBlast:Something()
    for key, value in pairs(self) do
        print(key, value)
    end
end

return AirBlast

Point I’m trying to make here is that if your AirBlast class contains only functions and your Move contains only a constructor that makes a table of properties, we can split the work up. Move becomes a data constructor that other classes make use of. Therefore, when we call the new method of a move (e.g. AirBlast), it first sets up data via Move.new and then attaches its functions on.

2 Likes