Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: Just another Snake clone  (Read 5158 times)

0 Members and 1 Guest are viewing this topic.

arnavb

  • Newbie
  • *
  • Posts: 18
    • View Profile
Just another Snake clone
« on: August 07, 2017, 12:49:51 am »
Hey guys, I recently started using SFML, and made a clone of the classic game Snake! I am really enjoying usage of the library, and plan on making more games with it. Anyways, I want to get feedback on my app (other than my questionable font and color choices ;P), and there are links to its download (sadly only for Windows (x64/x86)) below. Any and all feedback is appreciated. Thanks!

Links to .zip files (Just run the application in the folder)

Windows (x64): https://drive.google.com/file/d/0B5LejbIN6T8gUGJXNFotZ0dJZTA/view?usp=sharing
Windows (x86): https://drive.google.com/file/d/0B5LejbIN6T8gVmNBNXFJSHl1ckU/view?usp=sharing

Thanks Again!!

A screenshot is attached
« Last Edit: August 07, 2017, 01:01:30 am by arnavb »

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: Just another Snake clone
« Reply #1 on: August 07, 2017, 07:56:26 am »
I have made a snake clone a while back too: https://en.sfml-dev.org/forums/index.php?topic=22048.0

Yours is very fine too.
Will you share the code too?
It'd also be nice if the screen was resizeable, it's a bit hard to play comfortably on a 1080p screen and reacting to screen resizes is quite easy to do with SFML.

You also don't need to ship a 64 bit executable on windows usually.
It's a nice to have and shows your code isn't dependent on arch which is also good and it has some special upsides but it also has some downsides and isn't actually needed due to backwards compatibility of x64 with x86.
Many games, even double or triple A on Steam, ship just 32 bit exes.
Back to C++ gamedev with SFML in May 2023

arnavb

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: Just another Snake clone
« Reply #2 on: August 07, 2017, 09:42:46 pm »
Hey @FRex Thanks for taking the time to look at my game! Yours is nice too!

Quote
Will you share the code too?
Sure, I will post it to GitHub probably today

Quote
It'd also be nice if the screen was resizeable, it's a bit hard to play comfortably on a 1080p screen and reacting to screen resizes is quite easy to do with SFML.

I mean, while I appreciate the feedback, I don't think this is really necessary for a Snake Game, but I will consider that for other games I make.

Quote
You also don't need to ship a 64 bit executable on windows usually.
It's a nice to have and shows your code isn't dependent on arch which is also good and it has some special upsides but it also has some downsides and isn't actually needed due to backwards compatibility of x64 with x86.
Many games, even double or triple A on Steam, ship just 32 bit exes.

Hmmm, interesting to hear. I was just reading an article about this (although I cannot remember or find the link  :P) Anyways, I was wondering what the upsides and downsides you mentioned are. The article said that if you have a choice, develop a 64 and 32 bit executable, but if you don't, just do 32 bit and don't worry about the rest. Since I have a choice here, should I follow your advice or this articles?

arnavb

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: Just another Snake clone
« Reply #3 on: August 07, 2017, 09:55:25 pm »
Here is the github repository with my code:

https://github.com/arnavb/SnakeGame/

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: Just another Snake clone
« Reply #4 on: August 07, 2017, 11:56:24 pm »
I'll check the code out tomorrow.

As for the 32 vs 64 debate, it's a can of worms in itself. :-\

The only reason I mentioned was in case you thought you must ship both kinds because exe must match system bitness or there is some sort of a problem. Shipping 32 bit is a safe bet to make sure it works on both 32 and 64 bit windowses.

On 32 bit the pointers take less space and 32 bit exes work both on 32 and 64 bit windows.

On 64 bit you have more memory address space and more registers which leads to better optimizations of compiler generated or hand written asm.

Downside to shipping both is a tiny bit more work and uploading (since the game zips include all the resources they have to be uploaded twice despite lack of changes since requiring downloading resources and exe itself separate would look silly and be tedious) and explaining to do (no 'this is the windows pack', the user has to choose although the only possible bad choice is 32 bit user taking 64 bit exe pack).

Downside to shipping just 64 bit is excluding a very tiny percentage of 32 bit users and issues with some unusual stuffs like LuaJIT memory limit due to internal use of 32 bit pointers and worse cache usage (highly theoretical/niche problem).

Downside to shipping just 32 bit is loss of potential performance and memory benefits (which are also highly theoretical/niche).

The debate is also highly pointless and continues to matter less. 64 bit is nice to have for performance, it's nice to have well written code that works on both, etc. but unless you are doing something really performance and/or memory intensive like compression, video decoding, web browsers, etc. it just doesn't matter.

The pointer size issue comes up in a few serious and niche places in various ways:
http://docs.oracle.com/javase/7/docs/technotes/guides/vm/performance-enhancements-7.html#compressedOop
https://en.wikipedia.org/wiki/X32_ABI
https://github.com/LuaJIT/LuaJIT/blob/master/src/lj_alloc.c#L255

Many games like Payday and Stalker series actually ship only 32 bit exes and it's okay-ish (although it'd be a nice to have for performance to have them as 64 bit, they are intensive 3D FPS games). TDM GCC also ships 32 bit compilers (they can compile to 32 and 64 bit but themselves they are 32 bit exes): http://tdm-gcc.tdragon.net/quirks

Another things is that everyone who has over 3.5 gigs of ram (very common nowdays) has 64 bit windows too...

Pascal compiler I use does 64 bit by default and I gave a few people the exe of my program and no one complained.. 64 bit has crazy high penetration already too.

And you get news like: http://www.tgdaily.com/mobility-features/32079-no-more-32-bit-operating-systems-%E2%80%93-microsoft

But you can still see 5% of 32 bit machines on http://store.steampowered.com/hwsurvey/ despite the fact that 32 bit is kinda rare and on the way out so it might be best to support it still for the third world and since it's usually people with older computers and less ram that play indies games and such anyway.

Some people are purists about it and will argue for matching system bitness just for the sake of it, even if it's a premature optimization (which should be avoided).

Knowing/learning how to avoid 32 -> 64 code porting pitfalls, not relying on types' and pointers' sizes, writing future proof, 64 bit proof code, etc. are all good and should be done though, even if you don't ship the resulting 64 bit exes.

On linux it's also a bit different because most people don't have 32 bit libraries installed to running a 32 bit exe on 64 bit linux is more of a hassle but then again - 32 bit systems are becoming less common nowdays, the only 32 bit linux I ever seen was my own old one.
« Last Edit: August 08, 2017, 12:24:47 am by FRex »
Back to C++ gamedev with SFML in May 2023

arnavb

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: Just another Snake clone
« Reply #5 on: August 08, 2017, 06:10:59 pm »
Ok, Thanks for the detailed answer!

So here is my final verdict:

Just make a x86 version of the game, unless I am making a memory heavy, performance needing game. Would this be a good rule to follow?

As for my code, feel free to review the overall content of my code and post code review here. Thanks!  :D

arnavb

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: Just another Snake clone
« Reply #6 on: August 09, 2017, 07:59:00 pm »
@FRex did you look at my code yet?

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: Just another Snake clone
« Reply #7 on: August 10, 2017, 03:48:10 pm »
Yes, shipping x86 will not leave anyone hanging and allow you to have "THE WINDOWS EXE" and be done with it until you need something from x64 in one of your games.

I did look at your code just now, not super throughly or anything though. It looks quite okay-ish, reasonably structured, there is few news but their results instantly go into a unique_ptr so it's okay (maybe it could move unique_ptr into there instead but that's stretching it and making the interface a bit less clear), etc.

Few things (no particular order and many are my taste, not absolutes):

You probably should put resources like fonts and images in separate folder called 'read' or 'data' or something, not in src where code is.

You use {} (the new list stlye initialization, etc.), it has upsides about safety and so on but it looks so weird and out of place when used so often and even sf::Vector2f(1.f, 1.f) is written with {} instead and you pad it with spaces too so it looks extra strange.

You don't need to use WinMain, etc. you can just use normal mail and link to sfml-main. You also use MessageBox which makes it not portable but it's just this one place.

The Direction class is a bit weird. I'd use an enum (or a strong C++11 enum if you prefer them) and then use free functions for reverse and vector from direction.

You don't need the vector2Cast function because sf::Vector2f can take other kinds of sf::Vector and does the same (without the exception throwing/check).

You put private members without a specifier in the class while it's a bit of a custom/readability aid to put public members first and private ones second, take a look at SFML.

You put a semicolon after inline method body which isn't needed, like that (the final semicolon isn't needed): inline void doStateChangeDelay() const override { sf::sleep(sf::milliseconds(750)); };

Naming base State class GameState is a bit. For a moment I thought I'm missing the cpp until I found out SnakeGame is the class that inherits from it.

If you want you can put u after a number to signify it's unsigned, like: const sf::Vector2u windowSize_{ 400u, 336u };

You might want to check the image is the size you expect before calling setIcon with it, this can crash your game even if the image is too small (unlikely) or cause visual glitches (more likely).

Speaking of that image, windowIcon.png is PNG image data, 2880 x 1800, 1-bit colormap, non-interlaced not a 16x16 icon of any kind. Why is that so?

That function from SnakeGameApp could be const: inline std::unique_ptr<GameState> getInitialState() { return std::make_unique<MainMenu>(windowSize_); };

You might look into making logic independent from graphics/framerate (for future projects, not here where it doesn't matter).

Why in GameState is nextState_ initialized to this and no just NULL, 0x0 or nullptr (whichever you prefer)?
« Last Edit: August 10, 2017, 04:10:15 pm by FRex »
Back to C++ gamedev with SFML in May 2023

arnavb

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: Just another Snake clone
« Reply #8 on: August 10, 2017, 05:31:23 pm »
First of all, thanks for the review!

Quote
there is few news but their results instantly go into a unique_ptr so it's okay (maybe it could move unique_ptr into there instead but that's stretching it and making the interface a bit less clear),

Sorry, I don't understand what that means, so could you be a little clearer?

Quote
You probably should put resources like fonts and images in separate folder called 'read' or 'data' or something, not in src where code is.

Yea, I probably would do that for larger games with loads of resources. Since this was small, I decided to keep it all in the same place.

Quote
You use {} (the new list stlye initialization, etc.), it has upsides about safety and so on but it looks so weird and out of place when used so often and even sf::Vector2f(1.f, 1.f) is written with {} instead and you pad it with spaces too so it looks extra strange.

The spaces are Visual Studio formatting, so I just keep it :P, As for the braces, its just my style, and I personally like it so it separates functions from classes and aggregates.

Quote
You don't need to use WinMain, etc. you can just use normal mail and link to sfml-main. You also use MessageBox which makes it not portable but it's just this one place.

Noted.

Quote
The Direction class is a bit weird. I'd use an enum (or a strong C++11 enum if you prefer them) and then use free functions for reverse and vector from direction.

That was what I initially did, but I wanted to reduce the amount of switch statements in my Snake class.

Quote
You don't need the vector2Cast function because sf::Vector2f can take other kinds of sf::Vector and does the same (without the exception throwing/check).

Just wanted to turn off warnings in VS. :P I get a niggling feeling seeing one.

Quote
You put private members without a specifier in the class while it's a bit of a custom/readability aid to put public members first and private ones second, take a look at SFML.

Thats something I might consider, although it is more of a style thing.

Quote
You put a semicolon after inline method body which isn't needed, like that (the final semicolon isn't needed): inline void doStateChangeDelay() const override { sf::sleep(sf::milliseconds(750)); };

Preference and consistency with members; unless it is wrong, it shouldn't matter

Quote
Naming base State class GameState is a bit. For a moment I thought I'm missing the cpp until I found out SnakeGame is the class that inherits from it.

Noted, probably will use new name like BaseState to clarity

Quote
If you want you can put u after a number to signify it's unsigned, like: const sf::Vector2u windowSize_{ 400u, 336u };

Huh, I did that in other parts of my code. How did I miss that?

Quote
You might want to check the image is the size you expect before calling setIcon with it, this can crash your game even if the image is too small (unlikely) or cause visual glitches (more likely).

Speaking of that image, windowIcon.png is PNG image data, 2880 x 1800, 1-bit colormap, non-interlaced not a 16x16 icon of any kind. Why is that so?

That was just a quick image that I expected to be cropped on its own. As for the checking, I didn't know that, so thanks!

Quote
That function from SnakeGameApp could be const: inline std::unique_ptr<GameState> getInitialState() { return std::make_unique<MainMenu>(windowSize_); };

Noted

Quote
You might look into making logic independent from graphics/framerate (for future projects, not here where it doesn't matter).

clarify?

Quote
Why in GameState is nextState_ initialized to this and no just NULL, 0x0 or nullptr (whichever you prefer)?

If I set it to nullptr, it would mean the game is over and the state shouldn't be changed in the next frame and the window should instead close. I made it this, so that it shows that the state stays the same.

Anyways, thanks for your review, and please answer my questions @FRex

FRex

  • Hero Member
  • *****
  • Posts: 1845
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: Just another Snake clone
« Reply #9 on: August 10, 2017, 07:03:45 pm »
Doing @ at people doesn't notify them in any way.

I'm also not an expert in gamedev and C++, just an advanced hobbyist.

Here is an example of what I meant with unique_ptr move instead of new: http://ideone.com/8LgAY0

There is a popular article about independent frame and update times: http://gafferongames.com/game-physics/fix-your-timestep/

But I see that in the game itself your framerate is not tied to updates and that you speed up updates as the game progresses so it's just as the final solution in the article, the fact you did that in the state and no in the app fooled me at first.

Another thing I noticed is the code to spawn food is identical and could be it's own small function (and from main app class I see you like small functions which is good and readable IMO). It'd be instantly more readable to see spawnFood() or respawnFood() than the do while (which is a rare looping construct actually, most people use for and while vast majority of time).

Quote
That was what I initially did, but I wanted to reduce the amount of switch statements in my Snake class.
This would not add switches to your Snake class. You could keep your Direction hpp and cpp and use an enum and have a functions like reverseDirection and directionToVector that converts that enum into a vector, just like now there are such member functions in that Direction class.

Quote
Preference and consistency with members; unless it is wrong, it shouldn't matter
It doesn't affect the compilation and you can also put two semicolons at end of each line of code but no one does that. You can also make inline member functions multi line or put them under the class to not have them mixed with members.

Quote
The spaces are Visual Studio formatting, so I just keep it
I see, you can customize it a bit although not as well as in NetBeans or such...

Quote
If I set it to nullptr, it would mean the game is over and the state shouldn't be changed in the next frame and the window should instead close. I made it this, so that it shows that the state stays the same.
I see. Some people object to such multi-purpose fields (next state except if it's this then keep same and when null end the game) and just do field per purpose like: next state ptr + bool if state should change + bool if game should end. It's usually pointers that are used as both flag (null or not) and value (if not null then object pointed to is used. It may make sense if you're designing some tight structures (like LuaJIT packing pointer, bool, number and a type field all into a 64 bit double using NaN tagging) but not in general coding where trend is to be very explicit and so on.
« Last Edit: August 10, 2017, 07:05:50 pm by FRex »
Back to C++ gamedev with SFML in May 2023

arnavb

  • Newbie
  • *
  • Posts: 18
    • View Profile
Re: Just another Snake clone
« Reply #10 on: August 11, 2017, 12:09:07 am »
Thanks again!

Quote
Doing @ at people doesn't notify them in any way.

Lol, I'm new to this forum and all, soooo

Ill be sure to read that article and I now understand what you mean with the
Code: [Select]
unique_ptr
I agree, I should probably do that with the food, I hate code repetition in general and care more to readability than to performance, (unless necessary of course).

I'll be sure to do what you said with the
Code: [Select]
Direction class.

Quote
It doesn't affect the compilation and you can also put two semicolons at end of each line of code but no one does that. You can also make inline member functions multi line or put them under the class to not have them mixed with members.

Point taken. Just because I can doesn't mean I should...

As for the
Code: [Select]
GameState thing, I see what you mean with the boolean flag. Sure, I will do that in the future.

Thanks!