views:

470

answers:

7

Hi

I'm developing a game and I need to find a way of getting the value of a certain 'map block' in the game (in char format). I have a class DisplayableObject which takes care of all sprites, and a sub-class ThreeDCubePlayer which takes care of the player object. For ease of rendering/updating everything, all DisplayableObjects are stored in an array, with the 0th cell containing the player (which is of type ThreeDCubePlayer). ThreeDCubePlayer has a different constructor from DisplayableObject (it takes two additional arguments) and only ThreeDCubePlayer has the GetMap() functions that I need. So, here is what I have done so far:

ThreeDCubePlayer* cubePlayer = &((ThreeDCubePlayer &)m_ppDisplayableObjects[0]);

char mapEntry = GetMapEntry((int)*(cubePlayer->GetMapX()), (int)*(cubePlayer->GetMapY()));

This is the part of ThreeDCubeGame.cpp (the function which controls the map and keyboard input). The problem I've had is that both of these lines give an 'illegal indirection' error at compilation. I thought this error is when I try to dereference something that isn't a pointer, and I'm sure cubePlayer looks like a pointer...

Does anyone have an idea as to what I should do?

+1  A: 

Use one of the type safe casts, e.g. dynamic_cast instead of the C-style cast.

If m_ppDisplayableObjects is a DisplayableObject**, then it would look something like this:

ThreeDCubePlayer* cubePlayer = dynamic_cast<ThreeDCubePlayer*>(m_ppDisplayableObjects[0]);

if (cubePlayer != NULL)
{
    char mapEntry = GetMapEntry(cubePlayer->GetMapX(), cubePlayer->GetMapY());
}
else // Not a ThreeDCubePlayer* ...
richj
Thanks - this solved the illegal indirection error, however now I get this error:ThreeDCubeGame.obj : error LNK2001: unresolved external symbol "private: static char (* ThreeDCubeGame::m_acMapData)[10]" (?m_acMapData@ThreeDCubeGame@@0PAY09DA)Is this a problem with somewhere else in the code?
benwad
Yes, this is a problem somewhere else in the code. Your code has successfully compiled. This error is a linker error. It is saying that ThreeDCubeGame::m_acMapData has been declared (you have told the program about it, and perhaps even used it) but it is not yet defined (you haven't reserved any space for it).
richj
if (cubePlayer)... but yeah good point
queBurro
+1  A: 

Your cast is wrong, and I think the second line requires no cast at all (depends on how the methods are defined).

It should be:

ThreeDCubePlayer* cubePlayer = (ThreeDCubePlayer*)m_ppDisplayableObjects[0];

char mapEntry = GetMapEntry( cubePlayer->GetMapX(), cubePlayer->GetMapY() );

The cast in the first line should also be a C++ style cast, e.g.:

ThreeDCubePlayer* cubePlayer = static_cast<ThreeDCubePlayer*>(m_ppDisplayableObjects[0]);
frunsi
+1  A: 

For the first line, you can cast the DisplayableObject to the derived class ThreeDCubePlayer type using dynamic_cast.

ThreeDCubePlayer* cubePlayer = dynamic_cast<ThreeDCubePlayer*> (m_ppDisplayableObjects[0]);

For the second line, you're dereferencing whatever is returned by ThreeDCubePlayer::GetMapX(). If that function doesn't return a pointer (or some class with an overloaded * operator), you'll get a compilation error.

Charles Salvia
A dynamic_cast would work, but is not required here. static_cast is sufficient.
frunsi
boost::polymorphic_downcast<> is probably the best thing to use. Speed of static_cast, but asserts with dynamic_cast in debug mode.
Marcus Lindblom
@frunsi, yes a static_cast will work, but he's downcasting from a base to a derived class, so unless this is really performance critical code here, a dynamic cast is more appropriate.
Charles Salvia
@Charles Salvia: I think the code at that place already ensured that the object is of the required type. So, I'd use static_cast (of course, this requires error checking, but explicit and at other locations in code). The "if in doubt use dynamic_cast" advice IMHO is not good. C++ casts exist in different flavours, not only because of performance reasons ! @Marcus Lindblom: boost::polymorphic_downcast seems to be a good thing.
frunsi
A: 

You haven't given much code to go on, but I think your first line should be:

ThreeDCubePlayer* cubePlayer = (ThreeDCubePlayer *) m_ppDisplayableObjects[0]);

We'd need to see the declaration of GetMapX() to know about the second line.

Steve Fallows
+2  A: 

A couple of suggestions:

Don't use the C-style casts, use proper C++ casts instead. In your case, as you're casting down the inheritance hierarchy, you should be using dynamic_cast instead of the sledgehammer C-style cast. This will incur a small runtime overhead but it'll also make the whole thing type safe inasmuch as it isn't going to do something nasty behind your back simply because you're treating a chunk of $deity_knows_what as a ThreeDCubePlayer. Assuming that your m_ppDisplayableObjects array actually holds pointers, it'll look like this:

ThreeDCubePlayer *cubePlayer = dynamic_Cast<ThreeDCubePlayer *>(m_ppDisplayableObjects[0])
if (cubePlayer) {  // Important, if you don't check for 0 here you might dereference a null pointer
   ... cubePlayer->GetMapX() ...

Also, if you have to cast the result of the GetMapX then you have an impedance mismatch that you should sort out somewhere else; I'd recommend either adjusting the return type of GetMapX or the parameters passed to GetMapEntry. Usually, having to wildly cast about is a sign of a design issue - well-designed C++ code should not require a lot of casts and especially not a lot of C-style casts.

Timo Geusch
A: 

Be aware that in a game application, having a situation where you end up doing zillions of dynamic_casts per game frame can negatively impact performance. You'll usually find people doing other solutions to determine how to recast a object pointer stored in a container to the appropriate object type using static_cast or C style casting rather than relying on RTTI. Depending on the number of object types and other factors, there are various ways of doing this, but a simple method is just to have a enum class id and a get method for each of the enum types that returns null if the class id doesn't match the one being requested.

nctrost
A: 

ppDisplayableObjects is an array or base pointers isn't it? so try this?

const ThreeDCubePlayer* const cubePlayer = m_ppDisplayableObjects[0]; 
char mapEntry = GetMapEntry( cubePlayer->GetMapX(), cubePlayer->GetMapY() );

GetMapX etc. ought to return an (unsigned) int? and not a pointer to an int? (no negs? so unsigned?)

I'd like to second everyone else's comments on casting, they're a sign that your hierachy is not working quite right, but... but if you do have to cast then thinking about which C++ cast you'd need to use is a useful exercise, it also means when you want to revisit/tighten up your code all the casts are easier to search out and remove

ps - rack up your constness too where you can and add the arrays etc to some sort of owner class, maybe a singleton if you know you've only got the one

also IMHO... (sorry) write yourself a Coords class so that you can do things like GetMapEntry(const Coords& coords) instead of getting the x and the y values separately, this'll save you getting them swapped round the wrong way etc. :)

queBurro