[SOLVED] Constructor function malfunction

[applebappu]

I have a Mob class defined as follows:

mob.png (21.78 KiB) Viewed 7427 times

In my main.lua, I spawn what should be two instances of said class, like so:

instances.png (16.39 KiB) Viewed 7427 times

However, the slime instance is overwriting the position of the Player instance, and they both move together around the screen. It feels almost as if me calling the constructor method is behaving like a variable assignment, or something, except that somehow Player and slime are being linked together.

Screenshot from 2021-07-12 19-28-32.png (1.04 KiB) Viewed 7427 times

I've gone round and round on this for weeks, now, and I'm at my wits' end. I'm missing something simple, I know it. According to every Lua OOP tutorial I can find, this code SHOULD be working. And yet here I am.

If you replace "self = {}" with "self = self or {}", as in some Lua class tutorials, instead of both the Player and the slime, you just get the slime. I've tried rearranging the syntax, moving the functions of the class outside of the initial table of member variables, I've created a separate metatable that gets the setmetatable assignment. None of it makes a difference.

Here are the full source code of main.lua and Mob.lua, for reference:

Code: Select all

Map = require "./Classes/Map"
Mob = require "./Classes/Mob"

font = love.graphics.newFont("courier.ttf", 20)
love.graphics.setFont(font)

m1 = Map:New(20,10)
map1 = m1:makePlane(".")

Player = Mob:New(5,5,"@")
slime = Mob:New(1,1,"s")

global_timer = 0

function love.keyreleased(k)
	if k == "escape" then
		love.event.quit()
	end

	if k == "kp5" then -- wait
	elseif k == "kp4" then -- move west
		Player:Move(-1, 0)
	elseif k == "kp6" then -- move east
		Player:Move(1, 0)
	elseif k == "kp8" then -- move north
		Player:Move(0, -1)
	elseif k == "kp2" then -- move south
		Player:Move(0, 1)
	elseif k == "kp1" then -- move SW
		Player:Move(-1, 1)
	elseif k == "kp7" then -- move NW
		Player:Move(-1, -1)
	elseif k == "kp9" then -- move NE
		Player:Move(1, -1)
	elseif k == "kp3" then -- move SE
		Player:Move(1, 1)
	end
end

function love.draw()
	love.graphics.setColor(255,255,255)
	for i = 1, m1.board_size.x do
		for j = 1, m1.board_size.y do
			if i ~= Player.position.x or j ~= Player.position.y then
				love.graphics.print(map1[i][j], i * Map.tile_size, j * Map.tile_size)
			end
		end
	end
	
	love.graphics.setColor(255,0,0)
	love.graphics.print(Player.character, Player.position.x * Map.tile_size, Player.position.y * Map.tile_size)
	
	love.graphics.setColor(0,255,0)
	love.graphics.print(slime.character, slime.position.x * Map.tile_size, slime.position.y * Map.tile_size)
end

Code: Select all

Mob = {
	position = {
		x = 0,
		y = 0
	},
	character = " ",
	
	New = function(self, x, y, char)
		self = {}
		setmetatable(self, Mob)
		self.position.x = x
		self.position.y = y
		self.character = char
		return self
	end,

	Move = function(self, dx, dy)
		self.position.x = self.position.x + dx
		self.position.y = self.position.y + dy
	end
}
Mob.__index = Mob
return Mob

[ReFreezed]

You have to create a new 'position' table for each mob, in the New() function. Currently the table at Mob.position is reused for everyone.

Code: Select all

self.position = {}

[Gunroar:Cannon()]

Yes, what ReFreezed said should fix it. Tables need to be copied or made new or else they reference to the origional.

Code: Select all

table1 = {x=0,y=0}
table2 = table1
table2.x = 5
print(table1.x) --> prints 5

--could use, for later,
function copyTable(tb, booleanDeep)
    local new = {}

    for x, i in ipairs(tb) do
        if not booleanDeep or type(i)~="table" then
            new[x] = i
        else    
            new[x] = copyTable(i)
        end
    end

    for x, i in pairs(tb) do
        if not booleanDeep or type(i)~="table" then
            new[x] = i
        else    
            new[x] = copyTable(i)
        end
    end
    return new
end

table2 = copyTable(table1, true)
table2.y = 5
print(table1.y) --> prints 0

[applebappu]

Oh lord. I knew it would be something like that. Thank you both very much!

edit: Oh, before I leave this as-is, I should ask: is that true for ANY member variable that's a table?
edit again: Never mind, it's made clear by Gunroar's code that the answer is yes, for both tables and booleanDeep types.

one more edit: YES, preliminary testing shows that this was exactly what was needed.

test.lua - Visual Studio Code [Administrator].png (10.38 KiB) Viewed 7358 times

Anyway, thanks again, you two.

[ReFreezed]

From the Lua manual: Tables, functions, threads, and userdata values are objects: variables do not actually contain these values, only references to them. Assignment, parameter passing, and function returns always manipulate references to such values; these operations do not imply any kind of copy.

I also suggest reading up on metatables, especially the section about the "index" metamethod.

Finally, there seems to be some confusion about the colon syntax for function definitions and calls, and the 'self' argument. Mob:New(5,5,"@") means the same thing as Mob.New(Mob,5,5,"@"), which means that 'self' is 'Mob' in the New() function. Doing self={} (as you do) is important, otherwise you'd manipulate and return the 'Mob' table from the function instead of creating a new "instance".

Here is a revised version of 'Mob':

Code: Select all

Mob = {
	-- Because you initialize every member in New() anyway, there's no need to put the
	-- members here as well. Just keep the methods and the New() function in 'Mob'.

	New = function(x, y, char) -- Remove the confusing 'self' argument.
		local self = {
			position = {
				x = x,
				y = y,
			},
			character = char,
		}
		setmetatable(self, Mob)
		return self
	end,

	Move = function(self, dx, dy)
		self.position.x = self.position.x + dx
		self.position.y = self.position.y + dy
	end,
}
Mob.__index = Mob
return Mob

And at the calling site use Mob.New(5,5,"@") (with no colon) instead.

[applebappu]

Oh, nice, I like that as well! Thank you. That should cut down quite a bit on the duplication hell I was headed towards.

And yes, I was quite confused. The advice in the Lua manual is what led me to that confused state, actually. I've soaked my brain in the official documentation since the beginning, but I don't feel like it gives a good explanation as to what metatables and metamethods actually ARE. Or at least, I found their explanations confusing. They might be perfectly good in an objective sense lol.

I get it now, in that I THINK the __index metamethod just tells an object where to look when you try to call on something not defined in it. So for instance, once I say "Mob.__index = Mob" and "setmetatable(Foo, Mob)", if I try to call on some method that Foo doesn't know about, it looks in Mob for that method. The thing is, I kept running into all kinds of problems with duplication, because the examples given in every tutorial I could find (including in Lua's documentation) are just beyond simple to the point of uselessness. Or maybe it just wasn't clicking for me, I don't know. If you know that "metamethods" are just methods contained in the metatable, and that there's only so many, of which __index is one (and stuff like __add is another), it gets easier to parse.

I just need to remember that Lua isn't OO by default and I need to manually code everything, including making copies of each member variable and table when I initialize a new instance, and not get misled by what I call things. In other words, I might be saying "constructor function" but it's not like, doing all the constructing unless I explicitly tell it to.

[pgimeno]

As for the colon syntax for constructors, it's useful if the constructors themselves are inherited. Consider this primitive object system, for example:

Code: Select all

local Object = {
  new = function (self, init)
    return setmetatable(init or {}, self)
  end;
  extend = function(self)
    local newclass = self:new()
    newclass.__index = newclass
    return newclass
  end
}
Object.__index = Object

local Mob = Object:extend()
function Mob:fly()
  self.y = self.y - 1
end

local mob1 = Mob:new{x = 1, y = 2}
local mob2 = Mob:new{x = 3, y = 4}
print(getmetatable(mob1),Mob,Mob.__index)
mob1:fly()
assert(mob1.y == 1)
assert(mob2.y == 4)
assert(Object.fly == nil)

The constructor is inherited by the children, and passing the class via the colon syntax determines the class of the instances.

[ReFreezed]

I THINK the __index metamethod just tells an object where to look when you try to call on something not defined in it. So for instance, once I say "Mob.__index = Mob" and "setmetatable(Foo, Mob)", if I try to call on some method that Foo doesn't know about, it looks in Mob for that method.

Yeah, Mob is like a fallback for things that don't exist in Foo (or specifically, if a field is nil in Foo, Lua will try to get the field from Mob instead). And yeah, the manual definitively has both a lot of information crammed in it, and not enough information about things at the same time. It's just that it's pretty common that people ask questions that the manual answers.

One advice, that's kind of too late now I guess, is to not use metatables or anything like "classes" or OOP at all to begin with - just stick to using tables for storing related values, and using plain old functions for logic.

[applebappu]

As for the colon syntax for constructors, it's useful if the constructors themselves are inherited. Consider this primitive object system, for example:

...

The constructor is inherited by the children, and passing the class via the colon syntax determines the class of the instances.

Oh neat, I'll file that away for later.

One advice, that's kind of too late now I guess, is to not use metatables or anything like "classes" or OOP at all to begin with - just stick to using tables for storing related values, and using plain old functions for logic.

Yeah sadly I looked at doing this in a functional/procedural style for a looooooooong time. I really wanted to avoid OOP (I actually kind of dislike a lot of things about it) but for this particular project, it's a natural fit. Thankfully I think I've got my head round it now and can move forward. I made a lot of progress last night actually, after I implemented the class fixes y'all suggested, I was able to get a lot more basic mob functions in place. I'm up to the point of having several wandering mobs on a blank map, with collision detection and a turns/time system in place that will all scale very well. Gonna start working on bump interactions and map generation next, I think.

[ReFreezed]

Nice. Good luck