Code review please

Questions about the LÖVE API, installing LÖVE and other support related questions go here.
Forum rules
Before you make a thread asking for help, read this.
Post Reply
knuxyl
Prole
Posts: 35
Joined: Sat Aug 13, 2016 4:40 am

Code review please

Post by knuxyl » Fri Aug 23, 2019 1:59 pm

I just switched from pygame due to lack of hardware acceleration and I'm not sure the best way of coding in lua. If there is anything I should do differently in the code below please let me know. It is just a simple sprite sheet loader that'll display a quad based on what frame number you give. I made it as simple as possible. Before I get too far into porting what I've done with pygame I want to make sure I'm doing this the most efficient and easy to read way. Thanks

Code: Select all

-------------------------------------------------------------
player = {}
player.q = {}
player.spritesheet = {}
scale = 4
frame = 1
love.graphics.setDefaultFilter("nearest")
-------------------------------------------------------------
function player:create(f, w, h)
  self.w = w
  self.h = h
  self.spritesheet.f = f
  self.spritesheet.i = love.graphics.newImage(self.spritesheet.f)
  self.spritesheet.w, self.spritesheet.h = self.spritesheet.i:getDimensions()
  self.spritesheet.c = self.spritesheet.w / self.w
  self.spritesheet.r = self.spritesheet.h / self.h
  count = 1
  for r = 0, self.spritesheet.r - 1 do
    for c = 0, self.spritesheet.c - 1 do
      self.q[count] = love.graphics.newQuad(c * self.w, r * self.h, self.w, self.h, self.spritesheet.w, self.spritesheet.h)
      count = count + 1
    end
  end
end
-------------------------------------------------------------
function love.load()
  player:create("assets/sprites/rocket.png", 32, 32)
end
-------------------------------------------------------------
function love.update(dt)

end
-------------------------------------------------------------
function love.draw()
  love.graphics.setBackgroundColor(131/255, 192/255, 240/255, 1)
  love.graphics.draw(player.spritesheet.i, player.q[frame], 0, 0, 0, scale)
end
-------------------------------------------------------------

User avatar
raidho36
Party member
Posts: 2031
Joined: Mon Jun 17, 2013 12:00 pm

Re: Code review please

Post by raidho36 » Fri Aug 23, 2019 3:10 pm

First of all, you defined all of your variables as global, which you probably shouldn't, try using local variables whereever possible (keep in mind the hard-coded limit of 200 locals in any given scope).

Second of all, initialization functions should go into the load callback (your filtering setup function and the like).

Third, you set up entities as basic tables instead of using classes (technically, metatables). You can set up metatables manually as well as use any of the myriad of class libraries, I of course recommend my own.

Lastly, you use 8 bit integer color values, LOVE uses floating point color.

TheHUG
Citizen
Posts: 55
Joined: Sun Apr 01, 2018 4:21 pm

Re: Code review please

Post by TheHUG » Fri Aug 23, 2019 6:50 pm

You have some one-letter variable names where it isn't immediately obvious what they are.
(Q, I ,f w, c), I think they'd be better off as full words.

Most coding standards agree that it is best to limit the number of characters per line to around 80 characters.

knuxyl
Prole
Posts: 35
Joined: Sat Aug 13, 2016 4:40 am

Re: Code review please

Post by knuxyl » Fri Aug 23, 2019 10:51 pm

raidho36 wrote:
Fri Aug 23, 2019 3:10 pm
First of all, you defined all of your variables as global, which you probably shouldn't, try using local variables whereever possible (keep in mind the hard-coded limit of 200 locals in any given scope).

Second of all, initialization functions should go into the load callback (your filtering setup function and the like).

Third, you set up entities as basic tables instead of using classes (technically, metatables). You can set up metatables manually as well as use any of the myriad of class libraries, I of course recommend my own.

Lastly, you use 8 bit integer color values, LOVE uses floating point color.
well player is really the only global, while everything else is a variable in player. I don't know which variables ill need later on so i think this should be ok. is it better on cpu to use locals when possible? i could probably localize player.c and player.r but i might need everything else. if someone could explain the importance of localized variables in my code it would help me wrap my head around it.

by the load cappback do you mean the love.load()?

and player doesnt really need to be a class since there will only be one right? havent heard of metatables, ill look into it.

and about the one letter variables, my code gets hard to read with long variable names and i always use the same letter for what its referencing, ie; i = image, w= width, c = columns, q = quad, etc

bobbyjones
Party member
Posts: 720
Joined: Sat Apr 26, 2014 7:46 pm

Re: Code review please

Post by bobbyjones » Sat Aug 24, 2019 10:24 am

The importance of using only locals is to mainly prevent name conflicts. This can introduce some pretty crazy bugs. For example, if you declare player in two files and each has conflicting data and or operations done to them it would cause the player to act unpredictably. You can't localize player.c or player.r. Only the table containing those variables. You have to design your code to only declare player in one location and pass it to any function that needs it. For example:

Code: Select all

	--using globals
	player = {}
	player.x = 1
	player.y = 1
	
	function movePlayer() --this will always affect the player in the current scope. It can cause problems if this function is called in another file and you forget what exactly it does.
		player.x = 100
		player.y = 100
	end
	
	--using locals
	local player = {}
	player.x = 1
	player.y = 1
	
	function movePlayer(player) --You can also make this function a generic moveObject or moveEntity function since it will be operating on a passed in table
		player.x = 100
		player.y = 100
		return player --tables are passed by reference you do not have to return the table at all. This is effectively useless. But you can make the function create a new table to overwrite the old one.
	end
Do you frequently have great ideas but immediately lose them? Check out the MVP for my website called IdeaVault. It is designed to solve that problem. Desktop browsers only currently.

knuxyl
Prole
Posts: 35
Joined: Sat Aug 13, 2016 4:40 am

Re: Code review please

Post by knuxyl » Sat Aug 24, 2019 12:32 pm

Ok, I wasn't sure if global or local took longer to load or something. What I'm creating is rather simple so having global variables shouldn't be an issue. Thanks!

TheHUG
Citizen
Posts: 55
Joined: Sun Apr 01, 2018 4:21 pm

Re: Code review please

Post by TheHUG » Sat Aug 24, 2019 2:48 pm

knuxyl wrote:
Sat Aug 24, 2019 12:32 pm
Ok, I wasn't sure if global or local took longer to load or something. What I'm creating is rather simple so having global variables shouldn't be an issue. Thanks!
In this case, count, for example, is a global variable. In lua, unlike python, variable scope is global unless explicitly declared to be local. Globals can very easily get out of hand in lua because of this, so I'd really suggest getting into the habit of declaring all variables local unless you specifically need them to be global.

User avatar
pgimeno
Party member
Posts: 1978
Joined: Sun Oct 18, 2015 2:58 pm

Re: Code review please

Post by pgimeno » Sat Aug 24, 2019 5:51 pm

TheHUG wrote:
Sat Aug 24, 2019 2:48 pm
In lua, unlike python, variable scope is global unless explicitly declared to be local.
Furthermore, in Python, globals are not actually global, but module-local; however, in Lua, they are really global, i.e. application-wide, and therefore in the same namespace for all modules.

Globals are a bit slower than locals, because they usually need to be looked up in a table called the environment (by default you can access the environment using _G), while locals are "hardcoded" into the bytecode.

knuxyl
Prole
Posts: 35
Joined: Sat Aug 13, 2016 4:40 am

Re: Code review please

Post by knuxyl » Sun Aug 25, 2019 5:58 am

pgimeno wrote:
Sat Aug 24, 2019 5:51 pm
TheHUG wrote:
Sat Aug 24, 2019 2:48 pm
In lua, unlike python, variable scope is global unless explicitly declared to be local.
Furthermore, in Python, globals are not actually global, but module-local; however, in Lua, they are really global, i.e. application-wide, and therefore in the same namespace for all modules.

Globals are a bit slower than locals, because they usually need to be looked up in a table called the environment (by default you can access the environment using _G), while locals are "hardcoded" into the bytecode.
That is exactly what I was wondering, if it had to be looked up or hard coded.

Post Reply

Who is online

Users browsing this forum: No registered users and 25 guests