Hi all, i am starting a new project, a bomberman fan game and i am gathering resources and testing codes...so i decided to make my code this way..

is this way bad praticing or is a good code??

in player and enemies classes i make this...

void cPlayer::logic()
{
  cAnimation::logic();
  // here more code of the logic of player
}

void cPlayer::render()
{
  cAnimation::render();
}

then i create a class cManager that will hold a list of all cEntity objects,
so i simplify the loop with this...

I just want to know if my code style is good, sugestions and so on

It's good that you're abstracting functionality into separate classes (e.g., animations). It's bad that your entities inherit animation. A player isn't really an animation. A player has an animation. You would be better off to use composition instead of inheritance:

Another thing that you're doing is giving each of your entities and animations, etc., too much responsibility (e.g., their own logic and render methods). A lot of people do it like this and it does work. I prefer not to do it that way because IMHO it ends up violating good design practices and requires a lot of redundant code. It requires your different objects to know how to draw themselves. In practice, they usually don't need to know this. Often you draw every different type of entity the same exact way or ways. Ideally, you would figure out an interface (or interfaces) to draw with and write a single manager object to actually draw everything.

I have already demonstrated this for you before. See here.

Similarly, it requires all of your objects to know about the world around them to interact with it. Ideally, it would be the world or the game that does the thinking, not the Player, and the Boss, and the Bridge, etc.

I agree with bamccaig that your entities should own an animation, not be an animation.

However, I don't agree that entities and animations shouldn't have their own logic and render methods. All animations share the same basic logic and rendering. I see how it might help if you really need flexibility in how you render an animation, but unless you need shaders/effects/something other than plain bitmap drawing, then I don't think an external object should render an animation. I definitely disagree that entities and animations should not have their own logic methods.

I agree with Edgar on all accounts.

There is every reason for the Player, Boss and Bridge to do their own thinking, if it is thinking specific to them. If I want to find Player-specific logic, I expect to find it in the Player class. Certainly not somewhere in the World.

Which is fine. Give them an interface (or set of interfaces) to the World which is sensible (e.g. it might be bounded by location).

Globally, it's a rather-well-used pattern, familiar to many people. Yet everyone will have different advice, depending on one's idea of how generic the game engine should be, or if it should be simply limited to what the game will need : For example, if you never plan to use particles or vector objects, I got the impression that all your cEntities subclasses will derivate from cAnimation. If they do, then it makes sense to design one class instead of two, it's less code to write (and to understand later).

For cAnimation, as I understand it you've tried to isolate every piece of data that can be needed to draw a character/bonus/explosion/whatever. The evident common code is the render(), and increase_frame() is useful if it does what I think it does (ie: be called by any specific entities' logic() when they request to advance the animation by one step.). But then I wonder what's the intent of cAnimation:update() or cAnimation:logic(). logic() looks especially dubious, because every specific cEntity (CPlayer, etc.) is forced to implement a method of this name, which will always override cAnimation's one.

thanx to all for explanation...
so, after reading this i got new ideas...

class Entity
{
  public:
    virtual void logic();
    virtual void render();
};

so i think is this way good, right??

and one more thing to know your opinions...about tiles...
should i create a Level class that have a matrix tiles[][] to compare collision
or should i create a Level class that have a lot of tiles objects (class Tile, with information of x, y, size, is solid or not)??

You have x and y fields and two places now. I think you should do what Audric suggested: dump Entity and rename Animation to Entity. Both logic and rendering code need much of the same information, you shouldn't duplicate it.

You could do it like this:

struct Tile {
    int x, y;
    int size;
    bool solid;
};

Tile tiles[][];

A struct is a good match for tiles, since they are just dumb data.

Tiles: I'd recommend to separate two things:
- Available tiles: An array or list or vector where you enumerate the available tiles.
- Maps: each is (or contains) a 2D array or list<list> or vector<vector> of indices that refer to the distinct tiles above.

ie:

Entity could hold x and y, and Animation::Render could take x and y as input parameters.

tiles = new vector<Tile *>;
tiles[0]=new Tile(floor_bitmap, true); // second argument means "walkable"
tiles[1]=new Tile(wall_bitmap, false);
tiles[2]=new Tile(boulder_bitmap, false);

Don't forget to resize or use push_back, and dereference the pointer :

tiles = new vector<Tile*>;
tiles->reserve(3,NULL);
(*tiles)[0] = //...

Firstly, your animation class doesn't seem to be an animation at all. It has a single bitmap. That is just a sprite. You do seem to have some of the workings for an animation (e.g., a frame) so perhaps you are just keeping the implementation simple for now and will extend it into an animation later (e.g., std::vector<ALLEGRO_BITMAP *> sprites)?

Secondly, I think being visible is more an attribute of an entity within the game, not the animation itself. You care if the entity is visible, not if its animation is visible. At least, that seems logical to me. I guess it might depend on the game, but I think that is true for any generic entity vs. animation. If they aren't generic classes then you might want to give them more specific names.

Audric's solution is good: tiles are usually reused in a level to make a bigger level from fewer tiles. To save memory you might want to load your tiles into memory once into a manager/container and then reference them somehow from your levels. In memory, your level can store a matrix of numeric indexes, string keys (probably a bad idea for performance reasons), or even pointers to these tiles. That way you don't waste too much memory, but you can still reference tiles relatively efficiently.

For example, you could use a single char to identify your tiles. Assuming you use mnemonics to assign characters to your tiles, this could make it relatively easy to create levels with them. Load them up into a map and look them up once at level load-time.

(Untested... There is some very questionable code regarding const/static initialization, but you get the idea...)

Well that all turned out pretty messy, but hopefully not too messy to follow.

The idea is that your levels can be like this:

# media/levels/the-alter
A=media/tiles/alter.bmp
g=media/tiles/grass.bmp
T=media/tiles/tree.bmp
-=media/tiles/wall_horz.bmp
|=media/tiles/wall_vert.bmp

|------------|
|gggggggggggg|
|ggTgg---gggg|
|ggggg|Aggggg|
|ggggg---ggTg|
|ggggTggggggg|
|gggggggggggg|
|------------|

<over-engineer carried-away="true" /></tangent></digress>

That's already exactly what happens..

Better yet, use an automatic variable instead, if possible.

<checks eyesight...>
The latest incarnation of Entity doesn't hold x or y.
animation::render does take two ints...

1pt bambam, 1pt reynaldo

first thanx to all for the atention....
responding the questions....

is just simple for this post. I use a bitmap with all animation sprite with specific size and clip them acordiling..

e.g. al_draw_bitmap_region(sprite, frame * w, stance * h, x, y, w, h, 0);

hmmm, dont know.. but i agree with you in this point

for tiles i will try to find someway first, then after ask!!

Use one on the stack - a local.

I was talking to bamccaig - you're fine. It just makes sense for entity to store the position, not animation. That's all I was saying.

Not necessarily the stack. A class member can still be automatic, but the class instance itself can be on the heap.

*Something* like that anyway. Automatic variables are much easier to use and understand, which is why I suggested using them whenever possible.