• Create Account

## Game Engine using SDL 2

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

10 replies to this topic

### #1kovacsmarcell99  Members

Posted 22 April 2014 - 10:42 AM

I just made my game engine MEngine SDL public!
Please look at the source code and give some tips because I would like to use it in the Ludum Dare compo this weekend!

https://bitbucket.org/kovacsmarcell99/mengine-sdl

Feel free to make a Pull request

Edited by kovacsmarcell99, 26 April 2014 - 10:44 AM.

### #2Alundra  Members

Posted 22 April 2014 - 11:14 AM

Your math.h should have precomputed value :

const float DEG2RAD = 0.01745329251994329576923690768f;

inline float ToRadian( const float Degree )
{
return ( Degree * DEG2RAD );
}

inline float ToDegree( const float Radian )
{
}

You can clean your main.cpp, but it's personal comment, if you release, you have to clean, it's not a big code source so it's not a big effort.

You can optimize your vector2.h and vector3.h a lot using *= versus /= and avoiding return in CreateVector using reference as output.

I saw the same thing on different files who can be changed, output who has to be reference output and not returned.

This kind of optimization is important if the function is called each frame or often.

Now what I have to say about the all stuff is it's not a game engine for me but a framework actually.

But it can't be really used by other because you don't have any doc and it needs to be clean on different places.

Edited by Alundra, 22 April 2014 - 11:20 AM.

### #3BeerNutts  Members

Posted 22 April 2014 - 12:21 PM

Please, move the code out of the headers.  Every file that includes a header will include all the code as well.  That means you'll have multiple instances of the same code included over and over.

Just declare the classes and API's in the include files, and have the source code be available via a library, or at least separate .cpp source files.

My Gamedev Journal: 2D Game Making, the Easy Way

---(Old Blog, still has good info): 2dGameMaking
-----
"No one ever posts on that message board; it's too crowded." - Yoga Berra (sorta)

### #4Alundra  Members

Posted 22 April 2014 - 12:50 PM

It's true, I didn't noticed that the first view. It's very bad coding style.

When you will move all the code, if you have function who is just "get" or "set", let them inline.

It's a hint for the compiler but you know these function will be inlined so it's ok to let them in .h inlined.

### #5Irlan Robson  Members

Posted 23 April 2014 - 06:43 AM

If you use C++ go with SFML instead.

### #6Hodgman  Moderators

Posted 23 April 2014 - 06:55 AM

You can remove a lot of new/delete usage from your classes, e.g.

class Global
{
public:
Global(SDL_Window& window, SDL_Renderer& renderer, Logger& logger)
: input()
, screen(window, renderer)
, logger(logger)
, audio(logger)
, gfx(window, renderer, logger)
{
}

Input input;
Screen screen;
Logger& logger;
AudioManager audio;
GraphicsManager gfx;
};

This is exactly equivalent to your original code, but removes the need for you to remember to write a destructor that deletes everything (and the chance that you'll forget to add a matching delete for every new)... The member variables will be constructed automatically when a Global object is constructed, and will be destructed automatically when that object is destructed.

You're also not following the rule of three -- if your class has a destructor, then it must also have a copy constructor and an assignment operator (the above re-write avoids all this hassle by just not having an explicit destructor ).

Edited by Hodgman, 23 April 2014 - 07:00 AM.

### #7Alundra  Members

Posted 23 April 2014 - 09:46 AM

If you use C++ go with SFML instead.

If he used SFML, the result would have been the same because it's a question of coding style more than API used.

Another point is SFML is more a game framework for OpenGL, SDL can be just used for windowing on D3D/OGL.

But after that, it's just a comment because SDL or SFML can be used for Unix version only if needed.

You're also not following the rule of three

I always like when Hodgman speaks about rule of three, because it's not always knew and should be.

Edited by Alundra, 23 April 2014 - 09:46 AM.

### #8kovacsmarcell99  Members

Posted 26 April 2014 - 10:47 AM

Thanks everyone I almost finished all of them! I'm currently working on seperaing the code into .h and .cpp files, it's the last thing. I would like to hear further tips!

Edited by kovacsmarcell99, 26 April 2014 - 10:55 AM.

### #9kovacsmarcell99  Members

Posted 02 January 2015 - 08:48 AM

Hey everyone! I've done some work on the engine in the last half year currently I am adding the .cpp files. Could you guys give some further tips? (Hint: I've redone the logging pls check it. Is it good?)

### #10Artemiye  Members

Posted 02 January 2015 - 10:17 PM

Hey everyone! I've done some work on the engine in the last half year currently I am adding the .cpp files. Could you guys give some further tips? (Hint: I've redone the logging pls check it. Is it good?)

Sure, here's a few things I noticed:

	//TODO: This MAY cause problems!
bool isOpen = (bool)(in);

	Logger() {};
Logger(Logger const&){};
Logger& operator=(Logger const&){};


If your compiler supports C++11, consider using = default; or = delete;  See http://en.wikipedia.org/wiki/C%2B%2B11#Explicitly_defaulted_and_deleted_special_member_functions

	static Logger* Instance();
static void ResetInstance();


Consider making a templated Singleton class so you don't need to re-write this for every singleton you have.  Personally, I try to stay away from them but sometimes they're appropriate (ie. a Logger class is quite appropriate).

#define LOG(Message_) Logger::Instance()->LogLine(static_cast<std::ostringstream&>(std::ostringstream().flush() << Message_).str());


Using a preprocessor to wrap any debug/log statements is a great idea, as you can easily have the #define statement do nothing for certain builds.  However, consider utilizing the __FILE__ and __LINE__ macros to see where messages are popping up.  This will help in the long run.  (note that __LINE__ is an int).

inline std::string GetClockTime(void)


Consider making global functions as static functions in a class so you don't pollute the global namespace, and I don't seem to see any namespaces anywhere.  Also, I assume this is C++ since you're using C++ tools, so remove (void) as it does nothing in C++ and only clutters things.

if (logNumber > 500)


500 is a magic number to me.  Consider creating a constant that makes more sense to an external viewer.

	if (!instance)
instance = new Logger();


At first, I was a believer of the "no braces for a single statement if statement" but unfortunately, it can be error prone (an error may only come up 1 in 1000 of these situations, but even that small chance is worth the coding style change), so consider always wrapping statements in { } braces.

	instance = NULL;


Consider using nullptr. See http://en.wikipedia.org/wiki/C%2B%2B11#Null_pointer_constant

Logger* Logger::Instance()
{
if (!instance)
instance = new Logger();

return instance;
}


Consider returning a reference for a function if it's guaranteed that the item it returns will always exist.  Only use a raw pointer if you cannot guarantee that the returned item will exist.

I also implemented my log/debug functions as functions that take a std::string as a parameter, but I probably should have just overloaded the << operator instead, so you may want to try that out as well.

I was just looking in TextureManager as well, and for your sanity, use typedefs or using = SomeC++11Type or using = SomePreC++11Type.  I'm referring to this:


#ifdef CPP11_SUPPORT
std::unordered_map<std::string, SDL_Texture*> textures;

std::unordered_map<std::string, bool> errorShown;
#else
std::map<std::string, SDL_Texture*> textures;

std::map<std::string, bool> errorShown;
#endif


I hope this helps.

### #11kovacsmarcell99  Members

Posted 03 January 2015 - 02:25 AM

Hey everyone! I've done some work on the engine in the last half year currently I am adding the .cpp files. Could you guys give some further tips? (Hint: I've redone the logging pls check it. Is it good?)

Sure, here's a few things I noticed:

	//TODO: This MAY cause problems!
bool isOpen = (bool)(in);

	Logger() {};
Logger(Logger const&){};
Logger& operator=(Logger const&){};


If your compiler supports C++11, consider using = default; or = delete;  See http://en.wikipedia.org/wiki/C%2B%2B11#Explicitly_defaulted_and_deleted_special_member_functions

	static Logger* Instance();
static void ResetInstance();


Consider making a templated Singleton class so you don't need to re-write this for every singleton you have.  Personally, I try to stay away from them but sometimes they're appropriate (ie. a Logger class is quite appropriate).

#define LOG(Message_) Logger::Instance()->LogLine(static_cast<std::ostringstream&>(std::ostringstream().flush() << Message_).str());


Using a preprocessor to wrap any debug/log statements is a great idea, as you can easily have the #define statement do nothing for certain builds.  However, consider utilizing the __FILE__ and __LINE__ macros to see where messages are popping up.  This will help in the long run.  (note that __LINE__ is an int).

inline std::string GetClockTime(void)


Consider making global functions as static functions in a class so you don't pollute the global namespace, and I don't seem to see any namespaces anywhere.  Also, I assume this is C++ since you're using C++ tools, so remove (void) as it does nothing in C++ and only clutters things.

if (logNumber > 500)


500 is a magic number to me.  Consider creating a constant that makes more sense to an external viewer.

	if (!instance)
instance = new Logger();


At first, I was a believer of the "no braces for a single statement if statement" but unfortunately, it can be error prone (an error may only come up 1 in 1000 of these situations, but even that small chance is worth the coding style change), so consider always wrapping statements in { } braces.

	instance = NULL;


Consider using nullptr. See http://en.wikipedia.org/wiki/C%2B%2B11#Null_pointer_constant

Logger* Logger::Instance()
{
if (!instance)
instance = new Logger();

return instance;
}


Consider returning a reference for a function if it's guaranteed that the item it returns will always exist.  Only use a raw pointer if you cannot guarantee that the returned item will exist.

I also implemented my log/debug functions as functions that take a std::string as a parameter, but I probably should have just overloaded the << operator instead, so you may want to try that out as well.

I was just looking in TextureManager as well, and for your sanity, use typedefs or using = SomeC++11Type or using = SomePreC++11Type.  I'm referring to this:


#ifdef CPP11_SUPPORT
std::unordered_map<std::string, SDL_Texture*> textures;

std::unordered_map<std::string, bool> errorShown;
#else
std::map<std::string, SDL_Texture*> textures;

std::map<std::string, bool> errorShown;
#endif


I hope this helps.

Thank you very much!

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.