• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
Steno

Code Review Comments

17 posts in this topic

We have a process where every commit gets code reviewed and graded.  I've had some good ones come my way and these were all rated as 'F' in the American grading scale.  These are all real, so I hope someone gets a chuckle out of this.  Keep in mind that we are a purely C++ shop.

 

   void SetX(int iNewX) { iX = iNewX; }

 

Comment:  int iNewX needs to be const.

 

float fMinDistance = 1.0e10f;  // Some arbitrary large distance

 

Comment:  This value is too large for a float.  Use a double.

 

char szFilename[STR_LENGTH];

 

Comment:  szFilename needs to be szFileName!!!!!  (note: !!!!! is not exaggerated)

 

 

glPushMatrix();
...
glPopMatrix();

 

Comment:  I don't know anything about OpenGL, but even I can see this is stupid!

 

// Foo is a POD type
struct Foo {
  Foo() : x(0), y(0) {}
  ...

 

Comment:  POD types can't be structs.

Comment:  What does this even mean?

 

 

OK, I have paraphrased some of the comments, but they are real.  Feel free to troll this thread for your own code review horrors.

Edited by Steno
0

Share this post


Link to post
Share on other sites

I'm not sure which fails worse. The code or the comments......

 

As far as code goes, in case your referring to the char* szFilename part, I fixed that.

0

Share this post


Link to post
Share on other sites

Well I usually define floating-point infinity as 1e20 in prototype code if the actual value is too difficult to reach (e.g. no INFINITY macro, and you have to type a complicated templated expression that you usually get wrong the first time.. thank you type abstraction!). 1e10 is a bit low though.. depending on uses.

0

Share this post


Link to post
Share on other sites

Comment: szFilename needs to be szFileName!!!!! (note: !!!!! is not exaggerated)

Filename can be a single world, both variants are widely used.

Comment: I don't know anything about OpenGL, but even I can see this is stupid!

Based on those two lines?

// Foo is a POD type
struct Foo {
  Foo() : x(0), y(0) {}
  ...
Comment: POD types can't be structs.
Comment: What does this even mean?

Foo is not a POD type, but not for the reasons you mentioned.

0

Share this post


Link to post
Share on other sites

Foo is not a POD type, but not for the reasons you mentioned.

Huh? It has neither a user-defined copy constructor nor destructor.

A default constructor does not prevent a class/struct from being a POD type.
0

Share this post


Link to post
Share on other sites

A default constructor does not prevent a class/struct from being a POD type.

Frankly I'm not sure anymore. Quoting from wikipedia: 'Moreover, a POD class must be an aggregate, meaning it has no user-declared constructors', I'm not sure if declaring a defualt constructor falls under user-declared constructors.

0

Share this post


Link to post
Share on other sites

Frankly I'm not sure anymore. Quoting from wikipedia: 'Moreover, a POD class must be an aggregate, meaning it has no user-declared constructors', I'm not sure if declaring a defualt constructor falls under user-declared constructors.

Hmm. Now I'm confused too. Did this behaviour change in C++11?
struct MyPOD {
	int x, y;

	MyPOD() : x(0), y(0) {}
};

union MyUnion {
	int a;
	MyPOD pod;
};

int main() {
}
Clang with -std=c++11 compiles this just fine, but without -std=c++11 it gives me an error.
0

Share this post


Link to post
Share on other sites

WTF? It just ate the important parts of my post... editing to try and add it back in...

What I tried to write:

Not an expert, but I was just writing a response about this...
 
Consider the following:

#include <type_traits>
#include <iostream>
 
struct Foo {
  Foo() : x(0), y(0) {}
  int x;
  int y;
};
 
struct Bar {
  Bar() = default;
  int x;
  int y;
};
 
int main()
{
    if (std::is_pod<Foo>::value) std::cout << "Foo is a POD" << std::endl;
    else                         std::cout << "Foo is *not* a POD" << std::endl;
    
    if (std::is_pod<Bar>::value) std::cout << "Bar is a POD" << std::endl;
    else                         std::cout << "Bar is *not* a POD" << std::endl;
}

This outputs:

Foo is *not* a POD
Bar is a POD
 

 

Let's look at what's going on...

 

Section 12.1, paragraph 5
A default constructor for a class X is a constructor of class X that can be called without an argument. If
there is no user-declared constructor for class X, a constructor having no parameters is implicitly declared
as defaulted (8.4). An implicitly-declared default constructor is an inline public member of its class.

...

A default constructor is trivial if it is neither user-provided nor deleted and ...

(note the last line, so Foo's constructor is not trivial because it is user-provided)

So let's look at what a POD is:

Section 9, paragraph 10
A POD struct is a class that is both a trivial class and a standard-layout class, and has no non-static
data members of type non-POD struct, non-POD union (or array of such types).

 

So now we ask, is Foo a trivial class?

Section 9, paragraph 6
A trivial class is a class that has a trivial default constructor (12.1) and is trivially copyable.

Nope, looks like Foo is not a trivial class, because it does not have a trivial default constructor, therefore it cannot be a POD.

Note that this is for C++11. I'd have to read through C++03 more thoroughly to be able to comment on C++03.

Edited by Cornstalks
2

Share this post


Link to post
Share on other sites
The change in C++11 is to unions.

From the wikipedia page: "Unions can now contain objects that have a non-trivial constructor." and "If so, the implicit default constructor of the union is deleted, forcing a manual definition."

So, its ok to have non-POD classes in unions now.
Though, it does say you have to add a default constructor to the union if you do... so still something unexplained going on with the union... Edited by Olof Hedman
0

Share this post


Link to post
Share on other sites

Though, it does say you have to add a default constructor to the union if you do... so still something unexplained going on with the union...

Ok, this was bugging me, but I think it makes sense.

You can have as many user-defined constructors as you like, but you have to have a default constructor, so that the compiler can construct an instance when it needs to.
0

Share this post


Link to post
Share on other sites

Wow. Maybe you should change your codebase over to D. Because if 3 C++ experts (or advanced users) are all confused about what is standard, then debug hell must have more than 9 circles.

0

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0