Jump to content

  • Log In with Google      Sign In   
  • Create Account

Banner advertising on our site currently available from just $5!


1. Learn about the promo. 2. Sign up for GDNet+. 3. Set up your advert!


SDL game/engine?


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.

  • You cannot reply to this topic
23 replies to this topic

#21 Captain P   Members   -  Reputation: 1092

Like
0Likes
Like

Posted 15 April 2010 - 10:58 AM

Quote:
Original post by sandorlev
i've just commited. there are about 10 lines to delete/fix, maybe some not even commented, so i'm not sure it can be compiled.

Always make sure the repository is in a working state. You'll want to be able to do a checkout at any moment, on any machine, and have it up and running without hassle. Your repository is incomplete (main function? project files/make files?) and I can't find any notion about dependencies. I know you're using SDL, but what version? Ideally, you'll want to add the necessary files to the repository. Otherwise you may find yourself guessing as to what SDL version your 1 or 2 years old project used - because it doesn't work with the latest version.

I also noticed you're stuffing multiple classes into a single file. That's confusing: I wouldn't expect to find the Texture and TextureLoader classes in Screen.h/cpp. Just place them in separate files (and name them according to the class - this makes searching through a project much more predictable and easy). If you end up with lots of files (10 isn't much ;)), organize them in folders - and that would perhaps be an appropriate time to use namespaces: each folder (module) it's own namespace.


In most cases, passing std::strings by const reference is better. It prevents the string from being copied (because you're passing a reference) and the const disallows the function to alter the given string (only const member functions can be called on it - which in turn promise not to alter the string).

Making Texture's destructor virtual is only necessary if you intend to give Texture polymorphic behavior. That is, you're planning to derive classes from Texture, each with it's own specific behavior, but you want to treat them all as Textures. That means, you want some virtual member functions, so the correct version is called depending on the actual type of the texture. In such cases, you need a virtual destructor.

Your Texture class is hiding some useful information, like width, height and bpp. Provide some member functions for those. This also means you can throw away that ScreenData struct - it's only duplicating existing data.

I noticed Screen inherits from Texture. I understand that Screen is a surface that can be blitted to, but you're mixing it with Window functionality: a screen doesn't have a caption or a notion of what 'windowed' is - it's just a surface. A Window class that contains a Texture instance (named 'screen') would fit better here - no inheritance from Texture required.


Perhaps the most glaring problem is your memory management. There are various functions that return pointers to new'ed Texture instances, but I couldn't find a single 'delete' call. You'll want to be very strict with who owns what - generally speaking, the code that allocated resources should also clean them up.

I would also look carefully into your Texture class'es assignment operator (hint: you're missing the copy constructor). What happens if you've got two Texture instances (a and b) and you're assigning b to a, and you're then destroying b? A freed it's own surface and then set it's surface pointer to point to b's surface, but when b is destroyed, it frees it's surface. A still points to that surface. Auch. You performed a shallow copy while you needed to perform a deep copy.

But think about it: does it make sense for textures to be copied and assigned? Probably not, unless you want to copy a texture in order to modify a copy at run-time. If you don't need that, then just declare the assignment operator and copy constructor private. That essentially says: instances of this class can't be copied.


As for your problem, Texture's destructor not being called, it's hard to tell without having a working repository, but I guess it's because you're never cleaning up your textures. Which means you're leaking memory. Remember: anything you 'new' must be 'delete'd.


EDIT: If you can, try to get a hold of a copy of 'Effective C++' by Scott Meyers. I'd say it's a must for anyone who's working with C++.
Create-ivity - a game development blog Mouseover for more information.

Sponsor:

#22 sandorlev   Members   -  Reputation: 193

Like
0Likes
Like

Posted 16 April 2010 - 02:01 AM

Quote:
Original post by Captain P
But think about it: does it make sense for textures to be copied and assigned? Probably not, unless you want to copy a texture in order to modify a copy at run-time. If you don't need that, then just declare the assignment operator and copy constructor private. That essentially says: instances of this class can't be copied.

it's because i need to create new textures every time the text of textbox changes. or should i do it by deleting and then addig a new one?

Quote:
Making Texture's destructor virtual is only necessary if you intend to give Texture polymorphic behavior.

i just didn't want window to have a destructor, but i'm gonna change it as you adviced.

i got effective c++, and i've already started reading. it seems to be a great book, thanks for telling :)

#23 Captain P   Members   -  Reputation: 1092

Like
0Likes
Like

Posted 16 April 2010 - 05:10 AM

Quote:
Original post by sandorlev
it's because i need to create new textures every time the text of textbox changes. or should i do it by deleting and then addig a new one?

Delete the previous one and create a new one, yes. Why copy the old one if you're going to overwrite it anyway? You won't be using any of the old data.

I've just gotten into the habit of explicitly disallowing copying for non-POD types if it's not needed. You either spend time getting it right, or you take the easy way out by disabling it. If, however, you don't bother writing copy constructors and assignment operators, default versions will be created for you, and for non-POD types, that can lead to subtle problems (leaks, double deletes).

Quote:
i just didn't want window to have a destructor, but i'm gonna change it as you adviced.

If a class needs a destructor, it needs one, whether you want it or not. ;)
Create-ivity - a game development blog Mouseover for more information.

#24 sandorlev   Members   -  Reputation: 193

Like
0Likes
Like

Posted 16 April 2010 - 11:32 PM

Quote:
Original post by Captain P
I've just gotten into the habit of explicitly disallowing copying for non-POD types if it's not needed.

it sounds good. i've googled it but didn't find how to.

Quote:
I wouldn't give the texture loader a destroy-texture function though - the Texture destructor should be able to take care of that itself.

if i destroy it with the textureloader, it deletes the filename from the map, if i call delete, i cannot load it again (because it's still in the list of already loaded files).

what do you think, does it make any sense if i use my own structs instead of sdl's (like yolane::rectangle instead of sdl_rect, yolane::color instead of sdl_color), so when making a game with yolane, people wouldn't have to know anything about sdl's structs?

EDIT: if in the window class, i use a texture called screen, how can i set the video mode? texture's sdl_surface is private, so i cannot modify it anytime.
EDIT2: if i load a texture with textureloader, then 'delete' it, the program crashes as the textureloader trying to delete it again. what should i do with that?

[Edited by - sandorlev on April 18, 2010 2:32:07 AM]




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.



PARTNERS