RPGDXThe center of Indie-RPG gaming
Not logged in. [log in] [register]
 
 
Post new topic Reply to topic Goto page 1, 2  Next 
View previous topic - View next topic  
Author Message
DeveloperX
202192397


Joined: 04 May 2003
Posts: 1626
Location: Decatur, IL, USA

PostPosted: Tue Jul 14, 2009 2:39 am    Post subject: Please Critique - TBALib Hello World [quote]

Okay so I am working on a little game engine library thing for my upcoming book, and I wanted to get some feedback on the design before I take this any further in this direction.

Code:

// main.cpp
// Project: TBALib Hello World - Simple Game Class Usage Example
// Author: Richard Marks <ccpsceo@gmail.com>

#include <tbalib/simple.h>

////////////////////////////////////////////////////////////////////////////////

TBALIB_GAME_CLASS_BEGIN_GAME(MyGame)

// place your method declarations here

void do_frame();

TBALIB_GAME_CLASS_END;

////////////////////////////////////////////////////////////////////////////////

bool TBALIB_METHOD(MyGame, init)
{
   console_ = TBALIB::VirtualConsole::create(80, 25);

   if (!console_)
   {
      this->report_error("Virtual Console could not be created!");
      return false;
   }

   return true;
}

////////////////////////////////////////////////////////////////////////////////

void TBALIB_METHOD(MyGame, start)
{
   bool gameover = false;
   while(!gameover)
   {
      if (TBALIB::Key::Escape == get_key_press())
      {
         gameover = true;
      }

      this->do_frame();
   }
}

////////////////////////////////////////////////////////////////////////////////

void TBALIB_METHOD(MyGame, do_frame)
{
   console_->clear_screen();
   console_->print("Hello World!");
   console_->yield();
   console_->display();
}



The idea is for things to not need comments to be understood.
If you can read the code and get the jest of what is going on, then I have done my job. :D
Please critique.
_________________
Principal Software Architect
Rambling Indie Games, LLC

See my professional portfolio
Back to top  
Rainer Deyke
Demon Hunter


Joined: 05 Jun 2002
Posts: 672

PostPosted: Tue Jul 14, 2009 3:27 am    Post subject: [quote]

Are all those macros really necessary?
Back to top  
DeveloperX
202192397


Joined: 04 May 2003
Posts: 1626
Location: Decatur, IL, USA

PostPosted: Tue Jul 14, 2009 7:56 am    Post subject: [quote]

Rainer Deyke wrote:
Are all those macros really necessary?

They are just there to hide the class code that might confuse a newbie.

Its not necessary, and they are only used if the simple.h is included, othersise you include the normal.h in which you would need to write the class code that the macros are hiding.

I'll post the normal.h version with class code a bit later.
_________________
Principal Software Architect
Rambling Indie Games, LLC

See my professional portfolio
Back to top  
Mattias Gustavsson
Mage


Joined: 10 Nov 2007
Posts: 457
Location: Royal Leamington Spa, UK

PostPosted: Tue Jul 14, 2009 8:11 am    Post subject: [quote]

DeveloperX wrote:
The idea is for things to not need comments to be understood.

You always need comments. It's good to try and keep the code as simple as possible, so you don't need to comment weird implementation details, but comments are still needed to state intent.

DeveloperX wrote:
Rainer Deyke wrote:
Are all those macros really necessary?

They are just there to hide the class code that might confuse a newbie.


Macros are a problem, in that they can do a lot of things, which are impossible to know just from looking at them. You need to either learn how they are meant to be used (and trust that documentation) or dig into their implementation to find out for yourself. I think that by using macros in this way, you actually make it harder and more confusing for a newbie.

I couldn't say what's going on in that code - I could guess, but I wouldn't work with it based on that guesswork - I'd still have to dig in and find out.

If you want to spare newbies the complexity of classes and object oriented programming, why not have the lib make use of functions only?
_________________
www.mattiasgustavsson.com - My blog
www.rivtind.com - My Fantasy world and isometric RPG engine
www.pixieuniversity.com - Software 2D Game Engine
Back to top  
Rainer Deyke
Demon Hunter


Joined: 05 Jun 2002
Posts: 672

PostPosted: Tue Jul 14, 2009 8:36 am    Post subject: Re: Please Critique - TBALib Hello World [quote]

Classes are a beginner feature. Templates are an intermediate feature. Macros are an advanced feature. Hiding class code behind macros is... backwards, to say the least.

There are times when you truly need macros, because you have some complicated abstraction that cannot be expressed as a function or template (or because you need include guards). When this is not the cases, macros are best avoided.

Also:
DeveloperX wrote:
Code:

   while(!gameover)
   {
      if (TBALIB::Key::Escape == get_key_press())
      {
         gameover = true;
      }

      this->do_frame();
   }


This looks wrong. You are (apparently) processing exactly one (or at most one) keystroke per frame. If the user hits a lot of keys at the same time, the program will have to render multiple frames as it tries to catch up.
Back to top  
DeveloperX
202192397


Joined: 04 May 2003
Posts: 1626
Location: Decatur, IL, USA

PostPosted: Tue Jul 14, 2009 9:26 am    Post subject: [quote]

Hmm okay, I'll lose the macros.
I will post the new code later. Its almost 5am and I should be asleep already...damn..I could have sworn that it was midnight 20 minutes ago...yeesh how time flies when you are having fun.

Good night all, and thanks for the constructive criticism. :D

--oh, and I did do some shitty design on the input code I noticed.
I'll have something better tomorrow. (Well..I have it now, but I'm too tired to go dig the source files out now)

..night..*collapses*
_________________
Principal Software Architect
Rambling Indie Games, LLC

See my professional portfolio
Back to top  
DeveloperX
202192397


Joined: 04 May 2003
Posts: 1626
Location: Decatur, IL, USA

PostPosted: Wed Jul 15, 2009 8:13 pm    Post subject: [quote]

Okay here is what the macros expanded to.
I also fixed the input code.
Code:

// main.cpp
// Project: TBALib Hello World - Simple Game Class Usage Example
// Author: Richard Marks <ccpsceo@gmail.com>

#include <tbalib/tbalib.h>

////////////////////////////////////////////////////////////////////////////////

// TBALIB_GAME_CLASS_BEGIN_GAME(MyGame)
// start of macro expansion
class MyGame : public TBALIB::Game
{
public:
   MyGame(int argc, char* argv);
   virtual ~MyGame();
   virtual bool init();
   virtual void start();
private:
// end of macro expansion
   
   // place your method declarations here
   void do_frame();

// TBALIB_GAME_CLASS_END;
// start of macro expansion
};
int main(int argc, char* argv[])
{
   MyGame* game = new MyGame(argc, argv);
   if (game)
   {
      if (game->init())
      {
         game->start();
         delete game;
      }
      else
      {
         fprintf(TBALIB::logfile, "Unable to initialize the game!\n");
         exit(1);
      }
   }
   return 0;
}
MyGame::MyGame(int argc, char* argv[]) : TBALIB::Game(argc, argv)
{
}
MyGame::~MyGame()
{
   this->shutdown();
}
// end of macro expansion

////////////////////////////////////////////////////////////////////////////////

// bool TBALIB_METHOD(MyGame, init)
bool MyGame::init();
{
   console_ = TBALIB::VirtualConsole::create(80, 25);

   if (!console_)
   {
      this->report_error("Virtual Console could not be created!");
      return false;
   }

   return true;
}

////////////////////////////////////////////////////////////////////////////////

// void TBALIB_METHOD(MyGame, start)
void MyGame::start()
{
   bool gameover = false;
   while(!gameover)
   {
      if (this->key_pressed(TBALIB::Key::Escape))
      {
         gameover = true;
      }

      this->do_frame();
   }
}

////////////////////////////////////////////////////////////////////////////////

// void TBALIB_METHOD(MyGame, do_frame)
void MyGame::do_frame()
{
   console_->clear_screen();
   console_->print("Hello World!");
   console_->yield();
   console_->display();
}


Please tell me your thoughts and suggestions on this. :)
And yes, the macros are no-more.
I only left them in commented-out to show what they expanded to.
_________________
Principal Software Architect
Rambling Indie Games, LLC

See my professional portfolio
Back to top  
Rainer Deyke
Demon Hunter


Joined: 05 Jun 2002
Posts: 672

PostPosted: Wed Jul 15, 2009 9:15 pm    Post subject: [quote]

DeveloperX wrote:
Code:

int main(int argc, char* argv[])
{
   MyGame* game = new MyGame(argc, argv);
   if (game)
   {
      if (game->init())
      {
         game->start();
         delete game;
      }
      else
      {
         fprintf(TBALIB::logfile, "Unable to initialize the game!\n");
         exit(1);
      }
   }
   return 0;
}


Several problems with this:
  • new never returns NULL, so the if (game) bit is redundant. (new throws an exception on failure. If you want it to return NULL, use new(std::nothrow) instead.)
  • You are using a raw pointer instead of a smart pointer. That's bad form, and somewhat dangerous. (The game object never gets deleted if initialization fails, or if any function throws an exception.) Client code should never explicitly call delete.
  • You don't actually need a pointer here at all. Just declare MyGame game(argc, argv);.
  • fprintf is called std::fprintf in C++.
  • exit is called std::exit in C++, and unnecessary when called from main. Just return 1; instead.
  • Two-phase initialization (constructor followed by init function) is a questionable idiom, but it has some legitimate uses, so it might be OK here.
  • You are not handling exceptions at all.
Back to top  
DeveloperX
202192397


Joined: 04 May 2003
Posts: 1626
Location: Decatur, IL, USA

PostPosted: Thu Jul 16, 2009 1:04 am    Post subject: [quote]

Rainer Deyke wrote:

Several problems with this:
  • new never returns NULL, so the if (game) bit is redundant. (new throws an exception on failure. If you want it to return NULL, use new(std::nothrow) instead.)
  • You are using a raw pointer instead of a smart pointer. That's bad form, and somewhat dangerous. (The game object never gets deleted if initialization fails, or if any function throws an exception.) Client code should never explicitly call delete.
  • You don't actually need a pointer here at all. Just declare MyGame game(argc, argv);.
  • fprintf is called std::fprintf in C++.
  • exit is called std::exit in C++, and unnecessary when called from main. Just return 1; instead.
  • Two-phase initialization (constructor followed by init function) is a questionable idiom, but it has some legitimate uses, so it might be OK here.
  • You are not handling exceptions at all.


1. I develop on Linux, I don't use the std namespace, and I've never heard of nothrow.
2. I always use normal pointers. smart pointers are not something I care about. Its easy enough to declare a pointer, allocate it and deallocate it when I'm done. You're right, I placed the call to delete in the wrong place. Its supposed to be after the if(init) block. I overlooked it.
3. I could have chosen to not use a pointer, but its just my personal preference to use them.
4. I've never used std:: in front of fprintf.
5. Never used std:: in front of exit, and I have always used exit(1) instead of just a return, so again its personal style and habit.
6. The constructor builds a copy of the program's command line so that you can easily make use of the arguments in your program at any time. The init method is for you to initialize your game's data, such as loading audio or visual resources and allocating pointers to objects and stuff.
7. There isn't any need for exception handling here... O_o

But thank you. :)
_________________
Principal Software Architect
Rambling Indie Games, LLC

See my professional portfolio
Back to top  
Rainer Deyke
Demon Hunter


Joined: 05 Jun 2002
Posts: 672

PostPosted: Thu Jul 16, 2009 2:50 am    Post subject: [quote]

Sometimes a personal preference is just a personal preference, and sometimes there are solid technical reasons for preferring one style over another.

DeveloperX wrote:
1. I develop on Linux,


Not relevant. I develop on Windows, OS X, and Linux.

Quote:
I don't use the std namespace,


In C++, all standard library functions, even those inherited from C, go into namespace std. The old C headers that use global namespace still work, but are deprecated. See: The C++ Standard Library.

Quote:
and I've never heard of nothrow.


See: <new>.

Quote:
2. I always use normal pointers. smart pointers are not something I care about. Its easy enough to declare a pointer, allocate it and deallocate it when I'm done. You're right, I placed the call to delete in the wrong place. Its supposed to be after the if(init) block. I overlooked it.


If you managed to mess up such a simple example program, then it might not be easy enough (for you). But more importantly, it's not easy enough for beginners. Beginners should not even see a raw delete statement until they are comfortable with smart pointers. To introducing raw pointers to beginners is to teach a bad habit that may take years to unlearn.

Quote:
3. I could have chosen to not use a pointer, but its just my personal preference to use them.


Excessive use of dynamic memory allocation is another bad habit that takes time to unlearn. Using a stack object instead of a pointer leads to tighter code and better performance.

Quote:
6. The constructor builds a copy of the program's command line so that you can easily make use of the arguments in your program at any time. The init method is for you to initialize your game's data, such as loading audio or visual resources and allocating pointers to objects and stuff.


I know how two-phase construction works. It's generally a bad idea because it can leave an object in an undefined state. What happens when you forget to call init? What happens when you accidentally call init twice?

Quote:
7. There isn't any need for exception handling here... O_o


There is if you call functions that can throw exceptions, either directly or anywhere in your library. Such as, for example, operator new.
Back to top  
RedSlash
Mage


Joined: 12 May 2005
Posts: 331

PostPosted: Thu Jul 16, 2009 4:19 am    Post subject: [quote]

Nothrow:
Code:

int *x = new (std::nothrow) int;
if(x) { // now we can check for if x == NULL
....
}


Quote:
7. There isn't any need for exception handling here... O_o

I don't want to be picky picky here, but exception safe programming is considered good coding form and should be practiced even if you don't use exceptions. It is possible make your program exception-safe without using any try..catch statements through techniques like RAII, no-throw swaps, etc..
Back to top  
Rainer Deyke
Demon Hunter


Joined: 05 Jun 2002
Posts: 672

PostPosted: Thu Jul 16, 2009 5:13 am    Post subject: [quote]

RedSlash wrote:

I don't want to be picky picky here, but exception safe programming is considered good coding form and should be practiced even if you don't use exceptions. It is possible make your program exception-safe without using any try..catch statements through techniques like RAII, no-throw swaps, etc..


This works for most functions, but not for main, which should catch and report any exceptions that propagate that far. Otherwise your program may suddenly shut down without any clue as to what went wrong.
Back to top  
DeveloperX
202192397


Joined: 04 May 2003
Posts: 1626
Location: Decatur, IL, USA

PostPosted: Thu Jul 16, 2009 6:42 am    Post subject: [quote]

*sigh*
I asked for it, and boy did I get it. :P

I guess I'm just not cut out for this.

I've never seen exception code in a single book that I own, nor any articles I've read.

I've also never seen anything that ever said anything about smart pointers. I hadn't heard of them until Redslash talked about them when I was developing my ged101 engine.

I also have never heard anything about using a dynamically allocated object being slower than a statically allocated object. :\

Maybe I'm blind, but I didn't see anything about using std:: for things like fprintf or exit at that site..

Dear god that new reference has some retarded examples..
If that is indeed how its used, why is it that no one else uses it like that? ..Rather not a single book or article that I've ever seen uses it like that.

I typically double check all my code before I run it through the build process. I didn't check the code I posted, because I typed it out and pasted it here blindly.
I didn't intend to teach C or C++ in my book, I intended the reader to already have a working knowledge of the language already.
My book's target audience is programmers that wish to be game programmers that don't really know where to start.

I usually use code like this in the init() method:
Code:

static firstcall = true;
if (firstcall)
{
 // init stuff here
 firstcall = false;
}


that way the init code only executes once no matter how many times the method is called.

And the constructor will init all the member variables of the parent class, and any member variables for the child class need to be addressed by the programmer.

Anyway...
I guess I got the feedback that I asked for.
Its clear that while I've been programming in C++ for over 10 years that things have changed behind my back, or that I never was introduced to the language properly.
*sigh*

Maybe I'll not write the book after all.

Well thanks for the feedback.
_________________
Principal Software Architect
Rambling Indie Games, LLC

See my professional portfolio
Back to top  
Mattias Gustavsson
Mage


Joined: 10 Nov 2007
Posts: 457
Location: Royal Leamington Spa, UK

PostPosted: Thu Jul 16, 2009 10:16 am    Post subject: [quote]

DeveloperX wrote:
I've never seen exception code in a single book that I own, nor any articles I've read.

I've also never seen anything that ever said anything about smart pointers. I hadn't heard of them until Redslash talked about them when I was developing my ged101 engine.

I also have never heard anything about using a dynamically allocated object being slower than a statically allocated object. :\


DeveloperX wrote:
Rather not a single book or article that I've ever seen uses it like that.


The thing about books is, there's lots of books for beginner level, a rare few for intermediete, but pretty much no books for the really advanced stuff. Also, most books simplify things and ignore a lot of the common "best practices" used in real life.

There's only so much that can be learned from books - a lot of stuff is only learned when you start working in a team (not online, but in physical proximity) on a bigger project.

A lot of the things pointed out in this thread would be ignored when writing a book, in favor of making the core issues easier to follow - but it's still important stuff for when you make a real-world product, as it makes the code more robust or easier to manage.


DeveloperX wrote:
Its clear that while I've been programming in C++ for over 10 years that things have changed behind my back, or that I never was introduced to the language properly.

I think that is a common thing for programmers who haven't worked professionally with programming (though I don't know if that includes you, I just mean in general). I know that I learned more during the first 1-2 years of working than I learned all the years before from programming on my own and at uni.


DeveloperX wrote:
Maybe I'll not write the book after all.


Don't let the feedback discourage you - use it to make your book better instead. Also, if you tell us a bit more about your intentions (target audience, what you plan to cover, what technologies to talk about and why you're writing the book in the first place), I'm sure we can all offer more advice and feedback - some of which you will agree with and make use of, some which you won't agree with and can just ignore.

and good luck.
_________________
www.mattiasgustavsson.com - My blog
www.rivtind.com - My Fantasy world and isometric RPG engine
www.pixieuniversity.com - Software 2D Game Engine
Back to top  
DeveloperX
202192397


Joined: 04 May 2003
Posts: 1626
Location: Decatur, IL, USA

PostPosted: Thu Jul 16, 2009 12:52 pm    Post subject: [quote]

I think that despite the lack of best practices and everything, I am going to just stick with what I know.
Its not possible for me to write about something I don't know, and I don't want to spend X days learning about Y topic so that I can satisfy the expectations of a select few. As has been stated before, I've never seen any other book use it, so why should I? I think it will just further confuse the reader. Hell it confuses me, and I've been programming for 20 years!

If the reader knows about the std:: stuff, and wants to use it, then great. But I'm not.

TBALib is to be a small library that provides a cross-platform virtual terminal/ console window for easing the development of text based adventure games.
I am going to use SDL for the cross-platform aspect of the library.

The book is to be about the development of a complete text based adventure game (think along the lines of Zork if you don't know what I mean by text based adventure) from start to finish. I was going to provide the TBALib to the reader, and develop the game using the library.

As said before, my target audience consists of programmers that wish to get into game programming. I know that there isn't a large market for text games, though this is intended to be sort of a full introduction to game programming (in that by the end of the book, the reader will have developed a complete playable text based adventure game ..well if they are following along with me as they read it)

I wanted to write the book to prove to myself that I can.
I've attempted to start a game programming book many times, each time aiming too fucking high to get anything done.

I know that I can pull off the content of this book.
And hey if this goes well, then I'll be more enthusiastic about working on the other books that I've started and haven't been able to finish for various reasons.

I gotta start somewhere, and starting small seems like the best idea.

I appreciate the feedback from everyone. :)
_________________
Principal Software Architect
Rambling Indie Games, LLC

See my professional portfolio
Back to top  
Post new topic Reply to topic Page 1 of 2 All times are GMT
Goto page 1, 2  Next 



Display posts from previous:   
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum