Scoping in hump gamestate.lua

[56bitsago]

Hi all,

I recently starting exploring Lua and Love2D and so far I'm amazed by the ease and versatility of the framework! It's been a really fun journey so far

It didn't take long before I stumbled across the hump library and started playing with gamestate.lua, but I'm struggling to understand is how to scope my variables to a specific game state.

SITUATION
I have several states with menus (main menu, selection of number of players, level selection, etc.), with every menu implemented as a table. Next, I want to scope such menu to a game state and I would like to avoid manually destroying (for example by doing so on leaving a game state).

To me, the "right" solution seemed to be by doing something like self.menu = Menu:new(). I would expect this would scope menu to the specific game state, but that's not the case: the table instance "leaks" into another game state.

I've simplified my code to a bare minimum to demonstrate this behavior. But first, here's the output when running the code and switching from state1 to state2:

OUTPUT

Code: Select all

Entered state1
self.foobar.abc: xyz

Pushing into state2

Entered state2
self.foo: nil
self.foobar.abc: xyz

As you can see the self.foobar table carries over, but the self.foo value does not.

I have a hunch that it's due to how I instantiate Foobar, but having read the PIL on scoping in Lua didn't bring me closer to how I should structure my code.

SPECIFIC QUESTIONS
I appreciate any help to get me closer to understanding:
1. Why is this happening in Lua?
2. How should I structure my code to avoid this?

CODE

main.lua

Code: Select all

Gamestate = require "libs.hump.gamestate"

require("foobar")

local state1 = require("state1")
local state2 = require("state2")

function love.load()

	Gamestate.registerEvents()
	Gamestate.switch(state1)

end

function love.keypressed(key)

    if (key == "tab" and Gamestate.current() ~= state2) then

    	print("\nPushing into state2")
        return Gamestate.push(state2)

    end

end

foobar.lua

Code: Select all

Foobar = {}

function Foobar:new()

	return self

end

function Foobar:set_property(property, value)

	self[property] = value

end

function Foobar:get_property(property)

	return self[property]

end

state1.lua

Code: Select all

state1 = {}

function state1:enter()

	print("\nEntered state1")

	self.foo = "foo"
	self.foobar = Foobar:new()
	self.foobar:set_property("abc", "xyz")

	print("self.foobar.abc: " .. self.foobar:get_property("abc"))

end

function state1:resume()

	print("\nResumed state1")

end

return state1

state2.lua

Code: Select all

state2 = {}

function state2:enter()

	print("\nEntered state2")

	self.foobar = Foobar:new()

	print("self.foo: " .. tostring(self.foo))
	print("self.foobar.abc: " .. self.foobar:get_property("abc"))

end

function state2:keypressed(key)

	if (key == "tab") then

		print("\nPopping out of state2")

		return Gamestate.pop()

	end

end

return state2

[zorg]

Hi and welcome to the forums!

Forget the foobar script for now; let's just fix the two state lua files - you need to put local in front of the state1 and state2 definitions so they don't show outside of these files/blocks (you were already returning the tables, that's good)

Now, if you want to use foobar from any of the states, require foobar inside the state files, not in main.lua; you probably also would want to do the same there as well (localize the table, and return it)

The reason for leakage is that anything you don't define as local will exist in the global scope, i.e. the _G table by default, and that is accessible from any file within the same lua instance (in löve, that means in one specific thread).
Using local constraints things to lesser scopes, be it a file, a function, a loop, or a do ... end block.


Edit: pgimeno's right too, but my observations stand as well.

[pgimeno]

Well, I think that the problem is in Foobar:new().

Foobar is a global variable, equivalent to this (because of how colon syntax works):

Code: Select all

Foobar = {
  new = function (self)
      return self
  end
}

When you do self.foobar = Foobar:new(), what you do is equivalent to this (again, because of how colon syntax works):

Code: Select all

self.foobar = Foobar.new(Foobar)

which in turn, is equivalent to this, given how Foobar.new is defined:

Code: Select all

self.foobar = Foobar

So, every state will share the same Foobar table, so they will all see the same data.

One fix is to define Foobar.new as follows:

Code: Select all

function Foobar.new()
  return {}
end

I recommend you to read this part of the PIL: https://www.lua.org/pil/13.4.1.html; then read the whole section 16 https://www.lua.org/pil/16.html - pay special attention to the use of dot syntax vs colon syntax (Table.new vs Table:new) and try to understand why one or the other is used. You need to be clear on what the colon syntax exactly does.

Edit: Additionally, you need to understand this:

Code: Select all

t = {}
u = t
u.x = 1
print(t.x) -- prints 1

The {} operation creates a table object. The = operator creates a reference to this table object, it does not copy it. So, doing t = {} will make t reference the table just created, but doing u = t will make u reference the SAME table, not an independent copy of the table. Modifying the table modifies the object, and since all references refer to the same object, all references will see the values changed.

[56bitsago]

First of all: zorg and pgimeno, many thanks for your help!

I've re-read chapters 13.4.1 and 16 and followed your advise, but the behavior is still the same.

I'll probably facepalm myself once I realize what I overlooked / misunderstood, but I still haven't been able to wrap my head around it.

Here's the current code:

main.lua

Code: Select all

Gamestate = require "libs.hump.gamestate"

local state1 = require("state1")
local state2 = require("state2")

function love.load()

	Gamestate.registerEvents()
	Gamestate.switch(state1)

end

function love.keypressed(key)

    if (key == "tab" and Gamestate.current() ~= state2) then

    	print("\nPushing into state2")
        return Gamestate.push(state2)

    end

end

foobar.lua

Code: Select all

local Foobar = {}

function Foobar.new()

	return {}

end

function Foobar:set_property(property, value)

	self[property] = value

end

function Foobar:get_property(property)

	return self[property]

end

return Foobar

state1.lua

Code: Select all

local state1 = {
	foobar = require("foobar")
}

function state1:enter()

	print("\nEntered state1")

	self.foo = "foo"
	self.foobar:set_property("abc", "xyz")

	print("self.foobar.abc: " .. self.foobar:get_property("abc"))

end

function state1:resume()

	print("\nResumed state1")

end

return state1

state2.lua

Code: Select all

local state2 = {
	foobar = require("foobar")
}

function state2:enter()

	print("\nEntered state2")

	print("self.foo: " .. tostring(self.foo))
	print("self.foobar.abc: " .. self.foobar:get_property("abc"))

end

function state2:keypressed(key)

	if (key == "tab") then

		print("\nPopping out of state2")

		return Gamestate.pop()

	end

end

return state2

[pgimeno]

If you don't use new(), you're not creating new objects. I don't see any call to new() anywhere in your last code.

Also, if Foobar is meant to be a class, you probably want a metatable for it as explained in the PIL chapters I linked earlier.

[56bitsago]

Ha, that was the missing part. As I'm not setting any default values or passing defaults upon object construction, I was under the impression I didn't need a metatable (which I do use for game entities). I understand now that it's actually the missing piece.

Full working code below for whomever comes across this post in the future.

@pgimeno, just two more questions if I may:
1. Am I correct that I could drop the local in state1.lua and state2.lua, as they are in separate files (code chunks) and Lua limits scope to that?
2. Is there anything else in the code that you would consider bad practice?

Thanks again for helping out!

CODE

main.lua

Code: Select all

Gamestate = require("libs.hump.gamestate")

local Foobar = require("foobar")
local state1 = require("state1")
local state2 = require("state2")

function love.load()

	Gamestate.registerEvents()
	Gamestate.switch(state1)

end

function love.keypressed(key)

    if (key == "tab" and Gamestate.current() ~= state2) then

    	print("\nPushing into state2")
        return Gamestate.push(state2)

    end

end

foobar.lua

Code: Select all

Foobar = {}

function Foobar:new(o)

	o = o or {}
	setmetatable(o, self)
	self.__index = self
	return o

end

function Foobar:set_property(property, value)

	self[property] = value

end

function Foobar:get_property(property)

	return self[property]

end

return Foobar

state1.lua

Code: Select all

local state1 = {
	foobar = Foobar:new({})
}

function state1:enter()

	print("\nEntered state1")

	self.foobar:set_property("abc", "xyz")

	print("self.foobar.abc: " .. tostring(self.foobar:get_property("abc")))

end

function state1:resume()

	print("\nResumed state1")

	print("self.foobar.abc: " .. tostring(self.foobar:get_property("abc")))

end

return state1

state2.lua

Code: Select all

local state2 = {
	foobar = Foobar:new({})
}

function state2:enter()

	print("\nEntered state2")

	print("self.foobar.abc: " .. tostring(self.foobar:get_property("abc")))

end

function state2:keypressed(key)

	if (key == "tab") then

		print("\nPopping out of state2")

		return Gamestate.pop()

	end

end

return state2

[pgimeno]

1. Am I correct that I could drop the local in state1.lua and state2.lua, as they are in separate files (code chunks) and Lua limits scope to that?

No, that's not correct. The local is what limits scope to it. Otherwise they would be global, meaning shared by all files. Like `Foobar` in your case. It is possible to limit the scope of globals to a file/chunk using sandboxing techniques, but not with plain require(), and I don't recommend it for this kind of usage.

2. Is there anything else in the code that you would consider bad practice?

Just one thing. This bit of Foobar:new in particular:

Code: Select all

	self.__index = self

The constructor of a new object should not have any business modifying the class. I suggest removing that line and doing this instead:

Code: Select all

Foobar = {}
Foobar.__index = Foobar

The rest is more a question of personal preferences. I'd personally limit the scope of Foobar:

Code: Select all

local Foobar = {}

and then add this to each file that needs it:

Code: Select all

local Foobar = require("foobar")

And similarly with Gamestate. Ideally, you can have a globals-free project.

A little cosmetic thing is that Lua allows you to omit the parentheses when calling a function with just 1 argument, when the argument is a constant string or table. For example, you can write:

Code: Select all

require "foobar"  -- instead of require("foobar")
Foobar:new{}      -- instead of Foobar:new({})

In particular, Foobar:new{} looks in my opinion a bit cleaner than Foobar:new({}), but again, that's a question of taste and personal preferences.

[56bitsago]

Right, I understand. I've implemented your suggestions (except for omitting parentheses), many thanks for your review.

Two follow-up questions then:

1. Is local scoping everything always the ideal scenario, or are there exceptions? For example, I'm declaring fonts and sizes to be used throughout the entire game. Should I pass them between all game states, load them from a "shared" file or is this the exception and it's best to keep them in the global namespace?

2. I implemented your suggestion to move __index out of the constructor, but I also have something else there in my actual code: I declare a table holding menu options (o.options = {}), just before returning o. If I put it in the table declaration it again gets shared between game states (my guess that's because it's simply a pointer). Is that OK, or would you suggest a different way of doing that?

Thanks for your patience with me

[pgimeno]

1. Is local scoping everything always the ideal scenario, or are there exceptions? For example, I'm declaring fonts and sizes to be used throughout the entire game. Should I pass them between all game states, load them from a "shared" file or is this the exception and it's best to keep them in the global namespace?

If it complicates your life too much, go for globals, but there are a few reasons why to keep everything in locals if possible. These are the ones I can mention right away:

1. Locals are faster to access and write than globals. Globals need to be looked up in the global table, _G, so that's one additional indexing that Lua needs to do in order to access the variable. Locals don't have that penalty.
2. It's easier to make mistakes if you have too many globals.
3. It's a somewhat common mistake to forget declaring a variable that should be local as local, but there are tools that can find your global references. However, if you use globals, you need to account for them when searching for missing local declarations.

2. I implemented your suggestion to move __index out of the constructor, but I also have something else there in my actual code: I declare a table holding menu options (o.options = {}), just before returning o. If I put it in the table declaration it again gets shared between game states (my guess that's because it's simply a pointer). Is that OK, or would you suggest a different way of doing that?

With the way you wrote the `new` constructor, `o` is the object being created, while `self` is the class. The problem with the previous code was that you were modifying `self`, i.e. the class. It's the constructor's task to initialize the object that it will return, so it's perfectly OK for it to set fields of the object.

And yes, if you want independent values, you need to call {} to create a table for every object. It's normal that if you place the table in the class, all objects will share the same table.

[56bitsago]

Ok, thanks for all the help!