Having trouble drawing each object in table

[goopdev]

Hey, I'm making a test project that draws ten objects in a table. The problem is that it only draws a single one of the blocks that I instantiate:
Here's the code:

Code: Select all

texture = nil

-- Block class
Block = {pos, colorIndex}
function Block:new(x, y)
  self.x = x
  self.y = y
  colorIndex = love.math.random(5)
  return self
end

function Block:draw(self)
  if(colorIndex == 1) then
    love.graphics.setColor(170, 252, 74)
  end
  if(colorIndex == 2) then
    love.graphics.setColor(232, 148, 12)
  end
  if(colorIndex == 3) then
    love.graphics.setColor(255, 0, 100)
  end
  if(colorIndex == 4) then
    love.graphics.setColor(12, 15, 232)
  end
  if(colorIndex == 5) then
    love.graphics.setColor(13, 255, 148)
  end

  love.graphics.draw(texture, self.x, self.x)
  love.graphics.setColor(255, 255, 255)
end

playerX = nil
playerY = nil

blocks = {1, 2}

function love.load()
  texture = love.graphics.newImage("assets/Prototype.png")
  for i=1,10 do
    blocks[i] = Block:new(love.math.random(300), love.math.random(300))
  end
  playerX = 20
  playerY = 10
end

function love.draw(dt)
  love.graphics.draw(texture, playerX, playerY)
  for i,k in ipairs(blocks) do
    k:draw(k)
  end
end

function love.update(dt)

end

[s-ol]

You call Block.new using :, so "self" refers to the 'Block' Variable. You never create a new one and all 10 table entries are the same 'Block' table, with the same values (duh). Block.new shouldn't use the : call but the . call and create a new table to return.

You may want to read the PiL chapters about :/. Calls and probably metatables (again).

[raidho36]

Yep, colon notation automatically passes class instance to the function as "self". You only need to call the function like this with dot notation, but since colon notation exists it's not very useful.

But more importantly, you never create new table in the "new" function, instead you assign new values to fields of already existing single table. You should instead use something like this:

Code: Select all

Foo = { }
function Foo.new ( bar )
  self = setmetatable ( { }, { __index = Foo } )
  self.bar = bar
  return self
end

[airstruck]

But more importantly, you never create new table in the "new" function, instead you assign new values to fields of already existing single table. You should instead use something like this:

You are contradicting yourself, you just created a new table in your "new" function.

Code: Select all

 self = setmetatable ( { }, { __index = Foo } )

This sounds like pretty strange advice, maybe you meant to suggest that "new" only instantiates objects and passes them to a separate constructor function (maybe "init") that assigns values to fields? That's certainly a useful pattern if you need a separate (reusable) constructor. You need this kind of thing for inheritance, but otherwise people might say you're "over-engineering."

[zorg]

@goopdev:

Code: Select all

local texture
local playerX, playerY
local blocks

local Block = {}

Block.colors = {
	{170, 252,  74},
	{232, 148,  12},
	{255,   0, 100},
	{ 12,  15, 232},
	{ 13, 255, 148}
}

function Block.draw(block) -- I defined block explicitly, because i detest using the implicit variable "self".
	love.graphics.setColor(Block.colors[block.colorIndex])
	love.graphics.draw(texture, block.x, block.y)
end

function Block.new(x,y)
	local result = {} -- creates a new table
	result.x, result.y = x,y
	result.colorIndex = love.math.random(5)
	result.draw = Block.draw -- assigns the function to this field; this could also be done with metatables or a number of other ways
	return result
end

function love.load()
	texture = love.graphics.newImage("assets/Prototype.png")

	blocks = {}
	for i=1,10 do
		blocks[i] = Block.new(love.math.random(300), love.math.random(300))
	end

	playerX, playerY = 20, 10
end

function love.draw(dt)
	love.graphics.draw(texture, playerX, playerY)
	for i,k in ipairs(blocks) do
		k:draw() -- or k.draw(k)
	end
end

Also, assigning nil to any variable is pointless, unless you have the local keyword before them, but then (i think) you don't need to assign anything to them either to make them exist.

Yep, colon notation automatically passes class instance to the function as "self".

Just a minor corrections to this, the only thing colon notation does is that if it is used when defining a function, it will put a hidden first parameter into the argument list, that can be referred to by "self", and if used when calling a function, the first parameter passed will be wherever you called the function from.
These needn't correlate, so they are a playground for blunders.

Code: Select all

local t = {}
function t.f(a,b) return self,a,b end
function t:g(a,b) return self,a,b end
t.f(1,2) --> nil, 1, 2
t:f(1,2) --> nil, t, 1
t.g(1,2) --> 1, 2, nil
t:g(1,2) --> t, 1, 2

You only need to call the function like this with dot notation, but since colon notation exists it's not very useful.

Hopefully you don't mean that it's not very useful in general, since it does have its uses, that said, i do prefer to write out an extra first param explicitly myself as well, at least in the definition part; calling with : is fine as long as you know what you're doing.

Edit:

But more importantly, you never create new table in the "new" function, instead you assign new values to fields of already existing single table. You should instead use something like this:

You are contradicting yourself, you just created a new table in your "new" function.

Well, the op didn't, i think he meant that; also:

Code: Select all

Foo = { }
function Foo.new ( bar )
  self = setmetatable ( { }, { __index = Foo } )
  self.bar = bar
  return self
end

Nothing really wrong with this example code, though i'd not include the new "constructor" in the table of "methods" one gives to an "object", unless it'd be a "copy constructor".

[airstruck]

Well, the op didn't, i think he meant that

Ah, right, if you read it as "you never created" it makes more sense.

Nothing really wrong with this example code, though i'd not include the new "constructor" in the table of "methods" one gives to an "object", unless it'd be a "copy constructor".

This is why I like __call over new, keeps it out of instances nicely, and removes ambiguity between :new and .new (and instantiation looks nicer, I think). I think of new as an "instantiator" instead of a constructor; it can do some constructor-like stuff, but you can't use it the way you'd use a constructor to perform part of the initialization in a subclass. As in, this won't work:

Code: Select all

Guy = {}

function Guy.new (name)
    return setmetatable({ name = name }, { __index = Guy })
end

BadGuy =  setmetatable({}, { __index = Guy }) -- BadGuy extends Guy

function BadGuy.new ()
    local t = {}
    t.isEvil = true
    Guy.new(t, name) -- this won't work
    return setmetatable(t, { __index = BadGuy })
end

Instances can have an :init constructor for that, though. Of course, another option is to do away with classes entirely.

Code: Select all

-- Guy factory
local function Guy (name)
    return { name = name }
end

-- BadGuy factory
local function BadGuy (name)
    local t = Guy(name)
    t.isEvil = true
    return t
end

If you need more flexibility, decorators can do the job of constructors.

Code: Select all

-- Guy decorator
local function makeGuy (t, name)
    t.name = name
    return t
end

-- Bad decorator
local function makeBad (t)
    t.isEvil = true
    return t
end

-- Guy factory
local function Guy (...)
    return makeGuy({}, ...)
end

-- BadGuy factory
local function BadGuy (...)
    return makeBad(makeGuy({}, ...))
end

This neatly sidesteps inheritance, and is as powerful as multiple inheritance or mixins.

[raidho36]

I do use __call in my own code, I just gave some simple basic directions on getting started. The snippet almost directly comes from PIL so I don't think it's a bad advice to give, either.