• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
Storyyeller

How can I improve this code? (C++ enums)

22 posts in this topic

So in my game I have various secret items, each associated with a name and image. The problem is coming up with a way to automatically assign an index to each one (from 0 to n-1) so that they can be iterated over and used as indicies in a bitmap.

Enums solve the automatic index assignment problem, but they require type casting to iterate over or use as indices. So what should I do? Here's my current implementation. The problem is that I would have to manually change all the values when adding or removing a secret.


[source lang='cpp']typedef unsigned int Secret;

namespace Secrets
{
Secret begin();
Secret end();

Secret POTATO();
Secret NONPOISONOUSMUSHROOM();
Secret TM49();
Secret OBVIOUSSECRET();
Secret DELICIOUSCAKE();
Secret BIGDIPPER();

std::string Name(Secret s);
ImgHandle Image(Secret s);
}[/source]

[source lang='cpp']namespace Secrets
{
const Secret ePOTATO = 0;
const Secret eMUSHROOM = 1;
const Secret eTM49 = 2;
const Secret eOBVIOUS = 3;
const Secret eCAKE = 4;
const Secret eDIPPER = 5;
const Secret eNUMSECRETS = 6;

Secret begin() {return 0;}
Secret end() {return eNUMSECRETS;}

Secret POTATO() {return Secret(ePOTATO);}
Secret NONPOISONOUSMUSHROOM() {return Secret(eMUSHROOM);}
Secret TM49() {return Secret(eTM49);}
Secret OBVIOUSSECRET() {return Secret(eOBVIOUS);}
Secret DELICIOUSCAKE() {return Secret(eCAKE);}
Secret BIGDIPPER() {return Secret(eDIPPER);}

std::string Name(Secret s)
{
switch (s)
{
case ePOTATO: return "Potato Battery";
case eMUSHROOM: return "Nonpoisonous Mushroom";
case eTM49: return "TM49 (Tri Attack)";
case eOBVIOUS: return "Very Obvious Secret";
case eCAKE: return "Delicious Cake";
case eDIPPER: return "Big Dipper";
default: assert(false); return "INVALID SECRET";
}
}

ImgHandle Image(Secret s)
{
switch (s)
{
case ePOTATO: return ImgHandle(ImgDataStruct("Secrets/potato.png",0,99,0));
case eMUSHROOM: return ImgHandle(ImgDataStruct("Secrets/nonpoisonousmushroom.bmp",255,0,255));
case eTM49: return ImgHandle(ImgDataStruct("Secrets/tm49.bmp",0,255,0));
case eOBVIOUS: return ImgHandle(ImgDataStruct("Secrets/obvioussecret.bmp",0,255,0));
case eCAKE: return ImgHandle(ImgDataStruct("Secrets/deliciouscake.bmp",0,255,0));
case eDIPPER: return ImgHandle(ImgDataStruct("Secrets/bigdipper.bmp",0,255,0));
default: assert(false); return ImgHandle();
}
}
}[/source]
0

Share this post


Link to post
Share on other sites
I the compiler you are using supports the C++11 standard then you can use an enum and set the underlying type of that enum so no casting will be necessary.
0

Share this post


Link to post
Share on other sites
Try to store the item types in a dynamic array and load them from a file for the ability to load a mod from a folder. Try to represent their differences using numbers and boolean flags instead of hard coded behaviours. For example: Instead of naming all the items that can be used as a weapon, store a boolean property called "is weapon" so that an item type can be removed or duplicated without much effort. If the weapon have properties for the type of bullet used, refer to another list of ammunition types.

The same problems are common when designing databases without redundancy.
[url="http://en.wikipedia.org/wiki/Database_normalization"]http://en.wikipedia....e_normalization[/url]
1

Share this post


Link to post
Share on other sites
If you're indexing by enum then the std::map is the more appropriate solution.
0

Share this post


Link to post
Share on other sites
[quote name='yckx' timestamp='1317665640' post='4868639']
If you're indexing by enum then the std::map is the more appropriate solution.
[/quote]
This.
[source]
#include <iostream>
#include <map>
#include <string>


enum Secret
{
POTATO,
MUSHROOM,
TM49,
OBVIOUS,
CAKE,
DIPPER,
NUMSECRETS,
};


int
main(int argc, char* argv)
{
std::map<Secret, std::string> mapping = {
{ POTATO, "Potato Battery" },
{ MUSHROOM, "Nonpoisonous Mushroom" },
{ TM49, "TM49 (Tri Attack)" },
{ OBVIOUS, "Very Obvious Secret" },
{ DIPPER, "Big Dipper" },
};

for (auto p = mapping.begin(); p != mapping.end(); ++p)
{
std::cerr << p->first << ": " << p->second << "\n";
}
}
[/source]
Compiled and tested on Ubuntu 10.04 with g++ 4.5.2 (this is an older compiler does not support the current C++ standard, so no lambdas or [font="Courier New"]for (auto p: mapping)[/font] range iteration).

If you replace the [font="Courier New"]std::string[/font] as the [font="Courier New"]map[/font] value with a struct, you can get both the name and image.
0

Share this post


Link to post
Share on other sites
It's an associative container, which is what should be used when you're not indexing by integer. Otherwise I'd recommend a std::vector. An array should typically be avoided if possible. It's a C holdover; using the STL is the proper way to approach containers in C++.
0

Share this post


Link to post
Share on other sites
[quote name='yckx' timestamp='1317665640' post='4868639']
If you're indexing by enum then the std::map is the more appropriate solution.
[/quote]
It seems a bit odd to use a binary tree when an vector/array is basically a perfect hash for enums. Just don't encode meanings into the integer representation of your enums and there's no problem.

The C++ practice of giving enums special integer values is kind of weird. It almost feels like enums are a hack that paper over a hole in the language. Particularly when they're used as flags.
0

Share this post


Link to post
Share on other sites
I like fastcall22's solution, except I wouldn't store the index as part of the object. Replacing a simple array with an std::map is a pessimization that I can't understand.
0

Share this post


Link to post
Share on other sites
Yeah, that's what I ended up doing. There's no real reason to store a copy of the index.

[source lang='cpp']typedef unsigned int Secret;

namespace Secrets
{
//Order must be kept in sync with array in secret.cpp
enum SecretID : unsigned int {POTATO, NONPOISONOUSMUSHROOM, TM49, OBVIOUSSECRET, DELICIOUSCAKE, BIGDIPPER, NUMSECRETS};

inline Secret begin() {return 0u;}
inline Secret end() {return NUMSECRETS;}

std::string Name(Secret s);
ImgHandle Image(Secret s);
}[/source]
[source lang='cpp']
namespace Secrets
{
struct SecretData{
std::string name;
ImgDataStruct imgdata; //Don't convert to handle until runtime
};

const SecretData data[NUMSECRETS] = {
{"Potato Battery", ImgDataStruct("Secrets/potato.png",0,99,0)},
{"Nonpoisonous Mushroom", ImgDataStruct("Secrets/nonpoisonousmushroom.bmp",255,0,255)},
{"TM49 (Tri Attack)", ImgDataStruct("Secrets/tm49.bmp",0,255,0)},
{"Very Obvious Secret", ImgDataStruct("Secrets/obvioussecret.bmp",0,255,0)},
{"Delicious Cake", ImgDataStruct("Secrets/deliciouscake.bmp",0,255,0)},
{"Big Dipper", ImgDataStruct("Secrets/bigdipper.bmp",0,255,0)}
};

std::string Name(Secret s)
{
assert(s < NUMSECRETS);
return data[s].name;
}

ImgHandle Image(Secret s)
{
assert(s < NUMSECRETS);
return data[s].imgdata;
}
}[/source]
0

Share this post


Link to post
Share on other sites
[quote name='alvaro' timestamp='1317692769' post='4868786']
I like fastcall22's solution, except I wouldn't store the index as part of the object. Replacing a simple array with an std::map is a pessimization that I can't understand.
[/quote]
I think its mostly because any enum isn't guaranteed to be zero initialized (I'm sure there's a more technically correct term) or contiguous. While there isn't anything inherently wrong with fastcall's solution, the more general solution is something like a map.
0

Share this post


Link to post
Share on other sites
If enum values are really undefined, then there are bigger issues and using a different collection won't fix that.

The only case I can imagine where a map would be useful is if the enum values were discontinuous and had a large range, but that is specifically not the case here.
0

Share this post


Link to post
Share on other sites
I agree that a map is overkill here. Using enums to index an array is perfectly fine.

If not specified the first enumerator has value zero. For the rest, if not specified the value of the enumerator is one more than the previous enumerator.
0

Share this post


Link to post
Share on other sites
[quote name='Cornstalks' timestamp='1317693081' post='4868790']
I think its mostly because any enum isn't guaranteed to be zero initialized (I'm sure there's a more technically correct term) or contiguous. While there isn't anything inherently wrong with fastcall's solution, the more general solution is something like a map.
[/quote]
In C++, enums are guaranteed to start at zero and count up one by one from there.

Without that guarantee, a lot of normal enum usages would break down.

[code]enum PizzaToppings
{
Cheese, Pepperoni, Anchovies, ToppingsCount
};
[/code]

ToppingsCount will be 3.
0

Share this post


Link to post
Share on other sites
[quote name='Cornstalks' timestamp='1317693081' post='4868790']
[quote name='alvaro' timestamp='1317692769' post='4868786']
I like fastcall22's solution, except I wouldn't store the index as part of the object. Replacing a simple array with an std::map is a pessimization that I can't understand.
[/quote]
I think its mostly because any enum isn't guaranteed to be zero initialized (I'm sure there's a more technically correct term) or contiguous. While there isn't anything inherently wrong with fastcall's solution, the more general solution is something like a map.
[/quote]

In C++ an enum is guaranteed to start at 0 and continue with 1, 2, 3, etc.

From the ANSI C++ standard (7.2 - Enumeration declarations [dcl.enum]):

[quote]If the first enumerator has no initializer, the value of the corresponding constant is zero. An enumerator-definition without an initializer gives the enumerator the value obtained by increasing the value of the previous enumerator by one.[/quote]
0

Share this post


Link to post
Share on other sites
I understand that. That's why I said there was nothing wrong with fastcall's code. I said you aren't guaranteed with any enum, meaning there exists at least one enum where it does not start at zero (i.e. if he set mushroom to 1000). Perhaps I could have chosen clearer words, because I can see now how you may have thought I meant you aren't guaranteed that with any enum at all. I was specifically referring to cases where the first enumerator has a non-zero initializer.

This is safe and well defined in this case. But there may come a time when you're using some external library, and you cant always assume the index will start at zero, because the library's author may have specified a different initialization. I just wanted the OP to be aware of that and know that if he is going to use an enum to index an array, he needs to know exactly how the enum was defined, or else he may end up with a big mess. Plus I was just responding to alvaro's comment, saying that there are indeed times when a map is both preferred and necessary.

Like I said, it is for this reason (and the fact that the programmer may have chosen non contiguous values) that maps are considered the "more general" solution. It does not, however, invalidate fastcall's solution in this case.
1

Share this post


Link to post
Share on other sites
I'm the one defining the enum. It's right there in the code. Of course I know what values it can take.
1

Share this post


Link to post
Share on other sites
I know you do. I never said you didn't. But I don't know personally, and I don't know whether or not you would make this kind of assumption in the future, so I thought I would just add that and make it explicitly clear, just in case.

If you already do know everything I have stated, wonderful. Then I hope what I said will help someone who may browse these forums in the future and hopefully help them avoid making a poor assumption by helping them understand *why* its a poor assumption.

In this particular case, its safe, like I've said before. But its not safe in every possible case. You may already know that, but I have no way of knowing whether you do or not, nor do I know if any potential beginners browsing this thread know that or not. So I tried to add a small, clear comment as to why it isn't safe in every case.
1

Share this post


Link to post
Share on other sites
Cornstalks has done a good job explaining my reasoning, which I failed to add in my post because I was typing it on my phone. fastcall22's solution works, and that's fine. Run with it.

But not every enumeration is guaranteed to start from 0 or be contiguous. And just because this one is typical doesn't mean it will necessarily remain so for all time (although it may). There have been plenty of times when I made some apparently innocuous changes to code and end up breaking other bits of code here and there that assume the implementation of the bit of code that was originally changed. Hence my recommendation. I'm okay with not everyone agreeing with me ;)

Basic arrays are a part of the language, so I'm not going to say never use them. But I prefer an appropriate stl container wherever possible.
0

Share this post


Link to post
Share on other sites
Data driven design, people. Why hard-coded these? I recommend a hybrid of fastcall22's and Dawoodoz's approaches.
1

Share this post


Link to post
Share on other sites
It seems to me like a data driven approach works best when you have a well defined engine component and logic/data component that are planned out ahead of time and a fixed interface between the two. Whereas here, I've got a sprawling multigenre project with no real planning. Actually the entire game is hardcoded, though recently I've experimented with writing scripts to generate the code associated with level data.

One thing I could do is use forward declaration of enums to reduce recompilation, but I'm still using gcc 4.4.1 as I'm a windows noob and don't really know how to mess around with installing other versions of mingw.
0

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0