   How can I improve my code?
  http://www.mediafire.com/?wmjzdilzj2zIt's simplistic and small.Really, I'd like ideas of things to look into and some overall insight.Now that I've finished class, I have plenty of time over the summer to read something. Up until now I've really only dabbled in C and for the most part, I've just went with the flow and picked up what I needed as I went.I tried to quickly bring over my C version of this to a more C++ish manor but I think I'm falling into the so called C with classes problem that I hear about. Looking at the DX / GL docs it was pretty easy to get everything working, but what can I be doing differently / better?   
  It's not bad =DHere's some critique:* I like your use of namespace. Allows easier interop with other libraries.* You need a propper timer, this is just plain bad:for(;;Sleep(20)) //50 fps* Instead of making FOV a property of the graphics object, I'd make it inside a seperate projection object. Usually in a 3D library the "projection object" would just be a 4x4 matrix.In your case though, you could just make an object that holds x_dist/y_dist, and pass this object as an argument to the project function.* Your types header isn't necessarily right. I'd wrap it in something like:#if defined(MYCOMPILER) && defined(MYPLATFORM)...#else#error "This header needs to be ported"#endifAnd possibly use some STATIC_ASSERT's to ensure for example, that sizeof(u16)==2* Your math header looks useful, but inline asm is dubious... I'd like to see a unit test that does some benchmarks on these methods against more "simple" implementations to prove the complexity is worth it. Same goes for the SSE vector/matrix stuff.These kinds of "optimal" functions should also be #ifdef'ed and basic implementations provided for other compilers/platforms.e.g.inline void sincos(f32 x, f32* s, f32* c){#ifdef MYPLATFORM__asm...#elsesincos_basic(x, s, c);#endif}* You could write an input class instead of using GetAsyncKeyState directly.* You're accumulating modifications into the world matrix every frame. This is generally a bad idea as numerical errors build up. It's better to reconstruct the matrix each frame (starting from identity).On a similar note, your translate/rotate functions don't take elapsed time into account, so the "game" will run at different speeds on different PC's (also because your 50fps frame-lock isn't reliable).* Instead of:inline void Vector::multiply(Vector* v, Matrix* m)I would have written:inline void Vector::multiply(const Vector& v, const Matrix& m)In other words, look into using references over pointers, if you are going to use pointers the at least assert that they're not 0, and use const-correctness!* Use constructors/destructors.Instead of:video::gdigfx;gfx.create(0, 512, 256);You should havevideo::gdigfx(0, 512, 256);Simmilarly, the code for gdi::destroy should be moved into ~gdi.* If it doesn't make sense for a user to copy a class, make it non-copyable.e.g. if your users shouldn't be able to do this:video::gdi gfx;video::gdi gfx2 = gfx;Then don't let them.e.g.class gdi{public:gdi(HWND hw, s32 w, s32 h);~gdi();...private:gdi();//not implementedgdi( const gdi& );//not implementedgdi& operator=( const gdi& );//not implemented... [Edited by - Hodgman on May 14, 2010 1:53:08 AM];  I'm aware the Sleep(20) is gross. A proper timer class is coming up, there's a couple things I'm not sure of in this regardless though. Ideally I'd like the program not just sitting there, spinning away CPU time while its waiting for the next frame.I didn't really like the way I handled the FOV function either, but I couldn't think of a more clever way at the time.The fsincos thing I thought was pretty straightforward, as was the rounding function. I'm not too worried about them. I'm sure that the SSE variants of the multiplication is an improvement since I profiled them a while back. They were originally just a fun experiment, but I think they actually ended up simpler than the originals they replaced.edit: thanks for the extra info after your edit.I'll mull over it all in the morning.   ;  Providing a non-default constructor already prevents the compiler from automatically generating the default constructor, thus you only need the other two private & unimplemented methods.   ;  You should read Fix Your Timestep. For a quick example of a (Python) main loop with variable framerate:    while 1: newTs = pygame.time.get_ticks()   dt = newTs - curTs   curTs = newTs   timeAccum += dt   if timeAccum > physicsUpdateRate:  # Only do at most one physics update between drawings, even if more  # time has passed.  physicsNum += 1  game.gameObjectManager.update()  timeAccum -= int(timeAccum / physicsUpdateRate) * physicsUpdateRate   draw(timeAccum / physicsUpdateRate)   game.frameNum += 1   framesSincePrevSec += 1   if int(curTs / 1000) != curSec:  game.curFPS = framesSincePrevSec  logger.debug("FPS: ",framesSincePrevSec)  curSec = int(curTs / 1000)  framesSincePrevSec = 0The argument to the draw function is simply the amount of time that has passed since the last physics update, as a percentage of the amount of time between physics updates. I use it as a weighting to determine where to draw sprites for smooth position updates. For example, if I have a sprite that moved from (1, 0> to (2, 0) in the last two physics updates, and it's been 10ms since the last physics update, and I try to do 50 physics updates per second (20ms per physics update), then I draw the sprite at (1.5, 0).   