Seeking Constructive Criticism on a Snake Clone

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.
User avatar
walesmd
Prole
Posts: 22
Joined: Fri Oct 08, 2010 3:11 am
Location: Augusta, GA
Contact:

Seeking Constructive Criticism on a Snake Clone

Post by walesmd »

I've always had a passion for programming (developer for a DoD intelligence agency, primarily focused on the web) but have yet to find an engine that really feels right to me. I have licenses for Torque and Torque Game Builder, played with PyGame, UDK, Ogre - I never really progressed anywhere.

Love has been on my radar for awhile but I never really played around with it. Today, I decided to go ahead and give it a go. I read through the LUA PIL for an hour or so while at work, came home and cranked out a Snake clone in about an hour. By no means is this perfect - it actually has a really nasty dependency on FPS that I want to figure out (please don't provide tips on this, I want to figure it out), but I am seeking constructive criticism on the code as a whole. Would you have done things differently?

One area I think could be improved is the collision detection - right now I am using generic for loops, but I think a reverse table would speed things up (if Lua allows using a table as a key for another table). I just didn't feel like playing with that part tonight and wanted to get a working prototype done.

The code is available at http://goo.gl/FlN3 and I welcome your feedback! Looking forward to taking part in this community - seems like a great group of people.
User avatar
bmelts
Party member
Posts: 380
Joined: Fri Jan 30, 2009 3:16 am
Location: Wiscönsin
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by bmelts »

Welcome! Nice to see proof positive of how easy Lua is to get started with :)

I'll leave the FPS dependency alone, as you request, but there are a few other things I'd like to point out:
  • Instead of using table.maxn, you should just use the # operator - table.maxn has its place, but when you're manipulating the table through table.insert and table.remove, # works just fine. Example:

    Code: Select all

    if #player.points >= player.max_length then
  • At line 74, I think you mean math.randomseed(os.time()). It also shouldn't really be necessary to call math.randomseed more than once - I usually do it in love.load(). I guess it is a way of throwing more perceived randomness into the mix, though.
  • One of the quirks of Lua's PRNG (and, by extension, rand()) is that on some platforms, the first number is not nearly as random as it ought to be, and will often be the same even with different starting seeds. Thus, to ensure the illusion of randomness, it's generally a good idea to burn a math.random after each math.randomseed call (which is another reason why simply doing it once in love.load is kinda nice).
Otherwise, it looks pretty solid to me! I haven't actually tried playing it, but glancing over the code doesn't immediately reveal any other real flaws (besides the FPS dependency).
User avatar
Robin
The Omniscient
Posts: 6506
Joined: Fri Feb 20, 2009 4:29 pm
Location: The Netherlands
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by Robin »

Looks good! It's obvious you are already experienced in programming, so there aren't many problems with your code. In addition to the points anjo raised, I only want to point out some things that I'd have done differently, because they fit better with Lua's paradigms.
  • Code: Select all

    UP = {x = 0, y = -1}
    DOWN = {x = 0, y = 1}
    LEFT = {x = -1, y = 0}
    RIGHT = {x = 1, y = 0}
    Using a single table to contain them is more Luaic:

    Code: Select all

    directions = {up = {x = 0, y = -1}, down = {x = 0, y = 1}, ...}
    Usually, people do this then:

    Code: Select all

    player.dir = 'up'
    and

    Code: Select all

    player.x = player.x + directions[player.dir].x * player.speed
    If you go a bit further, you can replace the first part of love.keypressed with three lines, as a general way of changing direction.
  • Code: Select all

    apples.color = {r = 255, g = 0, b = 0}
    Colors are usually done like:

    Code: Select all

    apples.color = {255, 0, 0}
    This has the nice implication that you can use

    Code: Select all

    love.graphics.setColor(unpack(apples.color))
  • Also, you use setColor in a loop. You can pull them out of there, you only need to set the color once.
Help us help you: attach a .love.
User avatar
bartbes
Sex machine
Posts: 4946
Joined: Fri Aug 29, 2008 10:35 am
Location: The Netherlands
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by bartbes »

I know you said you didn't want to hear about FPS independent coding, so I won't tell you anything about a solution, but if you're looking for it, both the wiki and all other good games should contain it.
User avatar
walesmd
Prole
Posts: 22
Joined: Fri Oct 08, 2010 3:11 am
Location: Augusta, GA
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by walesmd »

Thanks for the feedback - I've updated the code with many of the suggestions you offered. Oddly enough this game is running at 300FPS at work and only 60FPS at home (my home computer blows my work computers out of the water - I guess I need to reboot at home). Needless to say the FPS dependency made it a vastly different, and faster, game today. I've yet to include that - going to work on it later on today.
anjo wrote:Instead of using table.maxn, you should just use the # operator
Thanks - I was unaware of this operator, definitely cleans the code up a bit.
anjo wrote:One of the quirks of Lua's PRNG (and, by extension, rand()) is that on some platforms, the first number is not nearly as random as it ought to be, and will often be the same even with different starting seeds.
I'll have to keep this in mind - thanks for the tip!
Robin wrote:Using a single table to contain them is more Luaic
I like this solution much better than the global variable route I was taking. I wasn't too confident in being able to clean up the keypressed() function, because I wanted to eliminate the possibility of death when attempting to move in the opposite direction as the current movement. After a quick look back at the code building the directions table I realized the absolute values of opposite directions were ==. JACKPOT! Thanks for pointing me in this direction - much happier with the keypressed() function now.

Code: Select all

if key == "up" or key == "down" or key == "left" or key == "right" then
  if math.abs(directions[player.dir].x) ~= math.abs(directions[key].x) and math.abs(directions[player.dir].y) ~= math.abs(directions[key].y) then
    player.dir = key
  end
end
Robin wrote:Colors are usually done like
I remembered reading about the unpack() function when reading through the LUA PIL, but didn't make the connection to use them here. This is, syntactically, how I attempted to implement colors initially as it is very similar to Python's tuples.

Thanks again everyone!
User avatar
walesmd
Prole
Posts: 22
Joined: Fri Oct 08, 2010 3:11 am
Location: Augusta, GA
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by walesmd »

Not sure this is how most others have implemented framerate independence, but this is what I came up with - would love to hear your ideas.

In love.update(), I've added the following code at the beginning of the function (well, right after I check to see if the game is still running):

Code: Select all

game.update_time = game.update_time + dt
if game.update_time < 0.001 then do return end end
game.update_time = 0
Of course, game.update_time is set to 0 initially within love.load().

Thoughts?
User avatar
leiradel
Party member
Posts: 184
Joined: Thu Mar 11, 2010 3:40 am
Location: Lisbon, Portugal

Re: Seeking Constructive Criticism on a Snake Clone

Post by leiradel »

walesmd wrote:if Lua allows using a table as a key for another table.
It does, but:

Code: Select all

local t1 = { x = 0, y = 0 }
local t2 = { x = 0, y = 0 }

if t1 == t2 then
  -- never happens, t1 and t2 are two distinct tables
end

local t = {}
t[ t1 ] = 1
print( t[ t2 ] ) -- prints nil
walesmd wrote:Oddly enough this game is running at 300FPS at work and only 60FPS at home
Sounds like you have vsync turned on at your home computer.

Cheers,

Andre
User avatar
bartbes
Sex machine
Posts: 4946
Joined: Fri Aug 29, 2008 10:35 am
Location: The Netherlands
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by bartbes »

It makes a lot more sense to change your speeds from pixels/update to pixels/second and then multiply by time (dt).
User avatar
walesmd
Prole
Posts: 22
Joined: Fri Oct 08, 2010 3:11 am
Location: Augusta, GA
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by walesmd »

Yeah, I messed around with that but was coming across issues where the snake would have "blank" spaces within him. Nonetheless, I do agree - a much simpler implementation and for the purposes of this little prototype and learning the language/library I'm calling this project done.

Time to start working on the tutorial I promised my Twitter followers. :)
User avatar
Robin
The Omniscient
Posts: 6506
Joined: Fri Feb 20, 2009 4:29 pm
Location: The Netherlands
Contact:

Re: Seeking Constructive Criticism on a Snake Clone

Post by Robin »

walesmd wrote:

Code: Select all

if key == "up" or key == "down" or key == "left" or key == "right" then
  if math.abs(directions[player.dir].x) ~= math.abs(directions[key].x) and math.abs(directions[player.dir].y) ~= math.abs(directions[key].y) then
    player.dir = key
  end
end
The first line could be made even simpler by using something similar to if key in dict: often used in Python: in Lua, if a table doesn't have a value for a certain key, it is nil. Since all the values in the directions table are tables (which never evaluate to false), you could do if directions[key] then.

The second line could be massively simplified as well, but that requires you to complicate other parts of your code, for example adding an "opposite" key to the tables in directions:

Code: Select all

up = {x = 0, y = -1, opposite = 'down'}
then you could use:

Code: Select all

if directions[key] and directions[key].opposite ~= player.dir then
  player.dir = key
end
Help us help you: attach a .love.
Post Reply

Who is online

Users browsing this forum: Bing [Bot], Google [Bot] and 46 guests