Not logged inGosu Forums
Forum Help Search Register Login
Up Topic Gosu / Gosu Showcase / Simple card memory game demo
- - By OldBoy Date 2016-07-30 09:46
This little game is the biggest "project" I've ever done, so before I start doing anything further, I'd like to hear critique.
Anything I'm doing wrong, from file/class naming to coding style practices and github related stuff.

Also, I want to make .zip file and mail it to a friend who (probably) doesn't want to install ruby.
Which files I need to copy from ruby installation folder to my game root folder to make it portable?
Parent - - By RunnerPack Date 2016-07-31 16:02
I only took a quick look at it, but the only thing that jumped out at me is the use of "magic numbers", which will make it harder to change aspects of the game later. It's better to make everything a constant, or load stuff from configuration files.

It's pretty fun for as simple as it is. Keep up the good work. Be sure to browse and/or search though the forums; there's a lot of good information here.

Re: the distribution thing, you should check out Releasy. There's a link on the Gosu homepage.
Parent - - By OldBoy Date 2016-07-31 19:46
Thanks! My current priority is to improve my ruby knowledge, there will be time to polish game(s) later. At first I made game with only one class and no states, but I couldn't get the timer work no matter what I tried. So I refactored code to three classes/states, it took me almost a week to figure that out ... Today I tried for learning purpose (and also I thought game would look more professional) to make additional class Card, and put all card logic inside, but I messed up everything and finished with deleting the project. I am much more comfortable working with arrays and hashes than with classes and OOP, my only fear is that people gonna laugh when they see my code ...
Parent - By RunnerPack Date 2016-08-01 01:04
Focusing on learning Ruby (and OOP) first is a good idea. I know I could stand to be more fluent in it. I don't think anyone's going to make fun of your code, though, at least around here. This is one of the most helpful, sharing, and friendly communities you'll find online. Any criticism you get will be constructive, and usually only when you ask for it (as you already have).

As for using arrays instead of classes, that can sometimes be a better way to go to get the speed you need, especially for a game. You should focus on using the best tool for the job and ignore the coding elitists and know-it-all pedants (which, like I said, we don't really have here).
Parent - - By jlnr (dev) Date 2016-08-01 19:28

> Re: the distribution thing, you should check out Releasy. There's a link on the Gosu homepage.

I would like to add: if you get stuck using Releasy, you can also try using Ocra first (Releasy wraps the Ocra tool, among others). Ocra is conceptually simple - all it does is to stuff your game into a self-extracting EXE file that will run your game out of a temporary folder. If everything works with Ocra but not with Releasy, you know it's an issue with your Releasy configuration.
Parent - By lol_o2 Date 2016-08-02 13:34
One thing to note about Ocra is that it fails to pack some gems, for example Ashton or Chipmunk, so there's problem if you want to extend your game. Although, there's a trick to make them working.
Parent - - By jlnr (dev) Date 2016-08-01 19:21 Edited 2016-08-01 19:29
I haven't run the game, only scrolled over it on GitHub, but this is what I noticed:

It is not necessary to spell out the ".rb" when using require or require_relative:

Because there's usually only one Window, you don't need to use @@class_variables here:
Likewise, $current_room doesn't need to be a global variable, I think.

You don't need the {} around hash argument lists:
Just, :name => "foo") is enough.

When you call Window#close, it's usually best to return from the method (instead of forwarding the button press, in this case):

Loading each image individually leads to a lot of copy & paste here:
Probably better to create these images from an array of names:
%w(camel-head donkey frog ...).each_with_index do |name, index|
  @images[index + 1] ="images_animals/#{filename}.png", :retro => true)

This can be simplified to if @cards.compact.empty?:
I think the design of this method is unconventional too, it both answers a question (is the game over?) as well as setting the best_time. I'd rename it to game_ended? and move the assignment to best_time into button_up, into the if game_ended... block.

Most of it is nitpicking, but I think that's what you asked for - hope that helps :)

You could also try the Rubocop tool for (very superficial) feedback on the way you format and structure your code. I don't agree with its default settings at all, but sometimes it'll point out something useful.

Edit: One more thing, you might want to look into Bundler and commit the Gemfile and Gemfile.lock. That way, it is clear (without reading the README) which version of Gosu you used to develop your game. This makes it a lot easier to other people to debug issues such as running the game with a version of Gosu that's too old (say, 0.7.50). This applies to all sorts of Ruby projects.
Parent - By OldBoy Date 2016-08-01 20:55

> Most of it is nitpicking, but I think that's what you asked for - hope that helps :)

yes, this is great stuff, thanks alot :). Hearing other people's opinion is good way to learn something. I totally forgot I could use question mark for method names, "game_ended?" is more clear, also Array#compact is nice shortcut, I should have known that Ruby has built in methods for almost everything :). Other stuff like ".rb" in require and hash curly braces, etc are mostly for my own conveniance, since I do get lost easily when looking in large chunks of text/code (I assume I'm kinda reading impaired or something like that).

As for Bundler and Rubocop, that'll have to wait, I'm still figuring out git and github. I tried Bundler a while ago but I couldn't make it work the way I expected.
Parent - By lol_o2 Date 2016-08-02 13:58
As for images, it's best to make them load automatically, especially if there are lots of them. Here's a basic method (put it outside any class):

def img(name)
  $_images ||= {}
  $_images[name] ||=

You can adds more arguments to the method if you want an option to load tileable or retro image.

You can then draw images like: img("images_animals/#{filename}.png").draw(x, y, z)

Also, if you keep all images in one directory, you can automatically scan it and pre-load all images on start. I use such methods for every resource and it makes everything much easier.
Up Topic Gosu / Gosu Showcase / Simple card memory game demo

Powered by mwForum 2.29.7 © 1999-2015 Markus Wichitill