A Better Way

Started by
21 comments, last by EmrldDrgn 15 years, 9 months ago
I'm starting to think that the design of my current project is fundamentally flawed. My basic idea went like this: Game contains Level contains Map contains a 2D array (std::vectors, don't worry) of Tile *s which point to various subclasses of abstract base class Tile. They all know how to draw themselves, update themselves, and output themselves. Some contain animations, while some contain static images, while others have added new primitive data members for various purposes. It sounded good on paper. However, I've been having all sorts of problems trying to save the map to file. My idea was that I'd loop over the Map array and find all the different types of tiles, assign them an index, then output their stats (path to image, walk value, etc.) and the index at the top of a file. The bottom would then be a rectangle of integers. Here's a sample file: Tile 0 "data/grass.bmp" 3 Tile 1 "data/lava.bmp" 7 000000000001000 000000011100000 111000000000000 Get the idea? But I couldn't do the loop over Tile *'s thing because I had no way of testing for == (see a recent post of mine). I eventually solved that problem, but I was required to use dynamic_cast<> (only once per derived class, but still). That set some alarm bells ringing, but I was prepared to dismiss the slightly hackish nature of that solution. Until I tried to write this:
file << MapArray[2][4];
This clearly will not work without additional dynamic_cast<>ing. Thus, I conclude that I'm approaching something from very much the wrong angle. I am therefore requesting a high-level evaluation of my plans, as outlined above. Any source code anybody wants to see, I can post, but I'm not really interested in nitty-gritty implementation details so much as a better over-arching design. On the flip side, if you don't see anything wrong with what I'm doing, please tell me that too. My ego could use it :). Thanks in advance, to anyone with the time to help.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~I program in C++, on MSVC++ '05 Express, and on Windows. Most of my programs are also for windows.
Advertisement
Let me try to understand:

You create an std::vector of an std::vector that holds all the tiles for your level. You want to write your level to a file, presumably so you can load it again. You don't like that, to find out what type of Tile it is, you have to use a dynamic_cast<>(), which you don't like.

Well, there are two options I know of:

Have a data member in the base class that all inheriting classes define to something specific to them--an enum or int is suggested, but chars could work too--then use that to find out what kind of tile it is.

Or, you could do what Bjarne Stroustrup suggests in "The C++ Programming Language", use dynamic_casts. He supposedly created C++, so I guess it's not a bad suggestion, and since you were already doing it, you obviously know how! Although, to be honest, I've never had the need to test it out.

Hope I got what the problem was/helped.
I do believe I see a number of problems with your approach but they may just be personal preference.

The first is that you are using a plain text file for your data file. Either using XML or a binary data format would be better (XML gives the advantage of being easier to parse and more formatted, while binary results in less wasted space and a "slightly" harder to hack file).

Your using single values to save your tile information which I feel is somewhat bad because what happens in the future when you want to add layers? It just seems like a somewhat limited design from that perspective.

Also I noted that in some of the code you posted you wrote:

MapArray[2][4];

In my experience hard coding values like this is rarely ever a good idea. Either use constants or drive the data externally but don't use magic numbers in your code as this makes things VERY hard to change later!

Your game should contain levels represented by a vector of level classes (only if random access is needed, otherwise you could get away with using a list). It seems though that your level could contain the 2d array directly without the need for a map class. It really doesn't make a lot of sense to use classes for the sake of using classes. For the levels I can see using a class but having a separate map class (which I presume would be a container for an array with some extra methods) seems a bit of a waste to me.

I am also a bit weary about your sub classing of the tile base class. Is it really necessary to create a separate class for each tile? Can you get away with a more generic approach? Think about why you would need say a grass class, a dirt class, a sky class, ect... Instead wouldn't it be easier to just store values in your tile class such as movement cost, ect?

Also using separate images for each tile seems like a waste, I would consider looking into a tilesheet because the overhead is less...

This is just my opinion so take my feedback with a grain of salt and do what YOU think is best for your project ;) .

What happens when you need more than 10 different tile types? Your rectangle of integers suddenly needs to be base 11, base 12, hex.. beyond?

From a parsing and extensibility point of view you'd probably be better off storing level size variables in the file, then just a linear array of identifiers (that can be ints of any size, or strings or anything else) and rectangularise them in code.

I claim royalties from anyone using my new word "rectangularise" ;)

Visit http://www.mugsgames.com

Stroids, a retro style mini-game for Windows PC. http://barryskellern.itch.io/stroids

Mugs Games on Twitter: [twitter]MugsGames[/twitter] and Facebook: www.facebook.com/mugsgames

Me on Twitter [twitter]BarrySkellern[/twitter]

Quote:Original post by PKLoki
I claim royalties from anyone using my new word "rectangularise" ;)


I claim royalties for the Americanized version: rectangularize.
While this thread hasn't of yet solved my original probelm (I did that this morning in the shower, actually), it's certainly chock-full of food for thought.

PKLoki -
I would use char's to store the tile indexes, actually. I doubt very much I'll have more than 255 tile types on one map (or 36 readable characters, for that matter). I'm not really sure what you mean by "level size variables"... just the X and Y lengths for the map?

shadowisadog -
The plain text file is a relic of two things - one, my previous experience in nix programming, where text files are the norm, and two, the fact that I don't as of yet have a working level editor, and text files are a lot easier to fake for testing purposes. I appreciate the problem of layers, and I haven't yet decided how to handle it. I could do a sort of superposition of tile indexes, but I'd need to find a way to handle that, or I could just have more than one rectangle of values. If you have a better solution, I'd love to hear it. The hard-coded values were just an example - I'm really using loops, so it'd be Map[x][y] (well really Map(x, y), if you want to be specific). The separation between Map and Level is a consequence of wanting reusable maps for different levels, as well as trying to build a map editor and a game simultaneously. Level will contain scripts for win conditions and such, as well as maybe the characters - I haven't decided yet where that responsibility falls. Finally, on the subject of subclassing tiles - I couldn't think of a better way to do everything I needed to do. That's about all there is to it.

Splinter of Chaos -
Basically I was trained to beware casts, that they were a sign (not always, and not the only sign, but a sign) of bad design. And I tried to give each type an enum stating its type, but that really doesn't help, because to access the data members added to the derived class, I still need to dynamic_cast<> to a derived class pointer. Plus, the deriveds are only very general - for instance, I can't tell a HealingTile (water) from a HealingTile(grass), but I can tell a HealingTile from a HurtingTile.

Thanks everyone for your input... I really appreciate it.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~I program in C++, on MSVC++ '05 Express, and on Windows. Most of my programs are also for windows.
Why exactly are there subclasses of Tile?

Even if there are subclasses of Tile, why is it problematic to build a Factory for Tiles, which is based off of some serialization token? No casting there, though you'll end up with a big switch or associative container to handle the various serialization tokens. Still, cleaner in the long run.
Quote:Original post by Telastyn
Why exactly are there subclasses of Tile?

Even if there are subclasses of Tile, why is it problematic to build a Factory for Tiles, which is based off of some serialization token? No casting there, though you'll end up with a big switch or associative container to handle the various serialization tokens. Still, cleaner in the long run.


Um... because there are? My game is planning to have things like tiles that heal you when you sit on them (think fortresses from Fire Emblem, if you played that game), or tiles with, say, lava that will burn you if you stop on them. I am not aware of a 'better' or 'more correct' way to do this than subclasses, although I'm sure there are many other ways.

What's a "serialization token"? I googled it but I only got links to articles on multi-threaded OS programming. How does a Factory for Tiles solve my problem? That sounds like it could be an excellent solution - if you could maybe give me a little more detail?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~I program in C++, on MSVC++ '05 Express, and on Windows. Most of my programs are also for windows.
Quote:Original post by shadowisadog
Your game should contain levels represented by a vector of level classes (only if random access is needed, otherwise you could get away with using a list).
Only use a (linked) list if you need insertions/deletions in the middle and/or the beginning - other than that there's not much reason to use one.
As a rule of thumb make your default container either a vector or deque.

Quote:It seems though that your level could contain the 2d array directly without the need for a map class. It really doesn't make a lot of sense to use classes for the sake of using classes. ...
The level class composes all aspects of a level: name, difficulty, map, spawn point(s), initial bad guys, portals, and so on.
You could bung the map implementation, and indeed everything, into the level class if you wanted to, but by separating the map from the level you encapsulate implementation of a map. Whatever the implementation, the level class needn't care; the same map could even assigned to multiple levels.

At the OP:
Quote:Original post by EmrldDrgn
[tiles] all know how to draw themselves, update themselves, and output themselves.
The single responsibility principle dictates that a class or function should have one and only one responsibility. What you have there are 3 responsibilities that need to be split into 3 modular units.

You could have:

responsibility: domain representation
> A tile class to store a tile description.
> A tile_map2d class to store a 2d grid tiles.

responsibility: graphical representation
> A sprite class to store the visual description of a tile.
> A sprite_set class to associate tile descriptions with their visual. description.
> A tile_renderer class to handle the actual rendering of tiles.

responsibility: updates
> An update function/class (functor) to handle the updating of the map.

responsibility: persistence
> A save_map function to save a map.
> A load_map function to load a map.


Now then, a tile is the domain specific representation of a tile, it might have attributes for a walk cost, a health cost, a name ("tree", "hole", "house", etc), a time variable, anything else you need - but it doesn't know anything about graphics, sounds, events, or any other subsystem - just tile stuff here.

The tile_map2d is fairly obvious, it holds a collection of tiles. It'll need suitable member functions to iterate over the tiles (begin and end), as well as width and height functions. Add any more functions as and when you find the need.

A sprite represents the graphical aspect of a tile, like it's texture, or a collection of textures for different frames. Not that much to it really.

The sprite_set is essentially a wrapped up std::map (even though I called it a set), it might even just be a typedef for one really. Its purpose is to associate tiles (or some property of a tile, like its name and time state) with sprites that the tile_renderer can use for rendering.
An instance of this class is probably to be owned along side the level(s) and then passed to the renderer every frame, that way each level can have its own tile_set, or they can all use the same tile set, but with just one renderer.

The tile_renderer simply draws the contents of a tile_map2d. Its job is to iterate over the tiles in the map, then for each tile it grabs the associated sprite from the sprite_set and draws it to the screen at the relevant position.
A more sophisticated implementation of this class might also hold onto a render_batch instance whose job is to sort the sprites (after they've been found from the tiles but) before rendering; so as to group together sprites with the same states (textures, shaders, whatever) and thus minimise the expensive state switching.

The update function is as simple or as complex as you want: It just iterates through the tiles and does whatever you consider to be updating. Whether that's simply incrementing the time variables for those tiles you want to animate or changing tile properties (for example, growing more grass, or making them more harmful to the player).

The save_map and load_map functions are also rather simple, they take an std::ostream and std::istream respectively. They use those streams to read/write the map in whatever format they want. Something like indexing all the unqiue tiles first and then writing out a block of tile indices would work fine (somewhat like you suggested in your original post).
No need to dynamic_cast anything, particularly since tiles are unlikely to have subclasses now - and even if they did then using virtual functions and double dispatch you can get them to pick the right function to serialise with.


Now for other loaders and initialisation:

You'll probably be wanted to store the tile_set contents in a file (or the same file?), so you will need a load_tile_set function that will return an instance that is populated with sprites.

A similar thing goes for a load_level function to load all the attributes pertaining to levels, this includes calling load_map, and returning a fully populated level instance.

Lastly, you will need to have a high-level loading function which will load up the tile_set(s) using load_tile_set and load up all the levels using load_level. Such a function would be called during initialisation, this is also when the renderer would be created.


So I hope that was all helpful to you - the trick is to fully separate your concerns and then everything just falls into place. [smile]
I think I have all of those things, except sprite_set, and possibly tile_renderer.

tile = Tile, obviously
tile_map2d = Map
sprite = Image (essentially a picture)
update = I have several update functions, called down the line basically
I'm still working on save_map and load_map, but I'm getting there. That's actually the context in which this thread was started.

For tile_renderer... why not just have a Map.Draw() method, or similar? Why does it need to be a separate class? What benefit is derived from this, to balance the added complexity?

I'm afraid I don't understand sprite_set at all... What I'm currently doing is, each Tile has an Image. What's the difference? Why use a "sprite_set"?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~I program in C++, on MSVC++ '05 Express, and on Windows. Most of my programs are also for windows.

This topic is closed to new replies.

Advertisement