Jump to content

  • Log In with Google      Sign In   
  • Create Account

Variable consistently returns bad values [FIXED]


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
19 replies to this topic

#1 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 07 January 2014 - 12:16 PM

I have a sort of infrastructure thing going on for running my game, and everything works just about fine, except when I return a COORD type variable to a different object, it consistently returns -12851, regardless of the value of the actual COORD values. Here's the relevant code; the problem happens in the Collide function. The Player object is a derived class of the Entity virtual base class:

|

void SceneManager::Init(__int32 stateID, short info)
{
    // initialize the level

    Entities.push_back(new Player(10, 10));
    Entities.push_back(new Block(25, 10));
    for (short i = 0; i < Entities.size(); i++)
        Entities[i]->Init();
}

void SceneManager::Tick()
{
    for (unsigned short i = 0; i < Entities.size(); i++)
    {
        Entities[i]->Act(hOut);
        Entities[i]->Draw(hOut);

        for (unsigned short j = 0; j < Entities.size(); j++)
        {
            if (j == i)
                continue;
            else
                Entities[i]->Collide(Entities[j]);
        }
    }
}
Collision Player::Collide(Entity* other)
{
    Collision info;
    COORD pos = other->GetPos();
    
    if (pos.X == Position.X && pos.Y == Position.Y)
        info.Contact = true;
    else
        info.Contact = false;
    
    info.Intersection.X = 0;
    info.Intersection.Y = 0;
    info.Intersection.Z = 0;
    info.OtherID = other->ID;
    info.Velocity.X = 0;
    info.Velocity.Y = 0;
    info.Velocity.Z = 0;

    if (other->ID == eIDBlock && info.Contact)
        Velocity.X *= -1;

    return info;
}
class Entity
{
    static unsigned short Count;

public:
    Entity();
    virtual void Init()  = 0;
    virtual void Act (const HANDLE) = 0;
    virtual void Draw(const HANDLE) = 0;
    virtual Collision Collide(Entity*) = 0;
    virtual bool Alive() = 0;
    const COORD GetPos()  { return Position; };
    const bool  GetDraw() { return Redraw; };
    const __int32 ID;

protected:
    virtual void Move() = 0;
    virtual void Die()  = 0;

    COORD* Beacon;
    COORD  Position;
    Vector Velocity;
    Vector Dimensions;
    bool   Redraw;
    char   Flags;
    char   Frames[4];
    short CurFrame, EndFrame;
    short FrameDelay, FrameTimer;
    unsigned short Timer;
};

|

Please let me know if you see anything else I did wrong, or if you need more code to work with. This is an ASCII game, so there aren't really any graphical considerations to take into account.


Edited by Akashi, 07 January 2014 - 07:06 PM.


Sponsor:

#2 Mnemotic   Members   -  Reputation: 340

Like
0Likes
Like

Posted 07 January 2014 - 12:33 PM

If this is C++, then `Entity` destructor must be declared virtual. Have you tried stepping through this with a debugger? Visual Studio has a great integrated debugger, or so I hear, and Nemiver is a good visual debugger on Linux.



#3 Melkon   Members   -  Reputation: 505

Like
0Likes
Like

Posted 07 January 2014 - 12:39 PM

Hello!

 

Well, i don't know the answer to your question, however i try to give you some tip:

- If you return a copy of a value, it shouldn't be const, in your getters you always return just a copy. Use the const keyword if you return for example a reference.

However the function itself is const, because you can call it in a const object.

 

- You shouldn't write "signed" before anything, if you write nothing it mean it's signed. It's not a big mistake, but pointless to write it and will be harder to read your code.

 

So, instead of:

const signed short GetX() { return Position.X; };

You should write

short GetX() const{ return Position.X;};

- At some place you use lowercap for variables, some place upper cap. You should choose what you like more, if you use one style at a place of code, you should do it everywhere, if you want to write a readable code.

 

Try to debug it. Unfortunately i don't see what's the problem, and i can't debug it without the whole code.

 

Good luck!


Edited by Melkon, 07 January 2014 - 12:45 PM.


#4 Mnemotic   Members   -  Reputation: 340

Like
0Likes
Like

Posted 07 January 2014 - 12:52 PM

At some place you use lowercap for variables, some place upper cap. You should choose what you like more, if you use one style at a place of code, you should do it everywhere, if you want to write a readable code.

So agree with this!

 

Sane naming conventions are very important. Especially if others will have to read your code -- as is happening here! Good defaults are to reserve TitleCase for type names, camelCase for functions (member or otherwise) and variables, and prefix class data members with m_, e.g. m_position.



#5 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 07 January 2014 - 12:59 PM

@Mnemotic Yeah, I've stepped through it. The X and Y values are just what I set them to be; X of 20, Y of 10, but when it's returned, it consistently gives me -12851 for both the X and Y variables.

 

@Melkon I was trying to test to see if it was some weird signed/unsigned mismatch, because my friend suggested it to me, as well as to see if returning individual values helped-- obviously it didn't. I don't normally write signed/unsigned in front of everything. I'll edit that part out. The const was to specify that the getter shouldn't change anything, and that it's just to return a value. Whether or not it's there doesn't change the result.

 

@both My convention is to have member variables capitalized, and things like local variables to have camelCase. I don't think I'm inconsistent in it...

 

Thank you for your replies. smile.png I really hope to find out what's going on in here soon.


Edited by Akashi, 07 January 2014 - 01:06 PM.


#6 Mnemotic   Members   -  Reputation: 340

Like
0Likes
Like

Posted 07 January 2014 - 01:09 PM

Okay. Can we see the definition of COORD type? Have you turned up pedantry to 11 on your compiler? And what about the virtual destructor for `Entity`?

 


@both My convention is to have member variables capitalized, and things like local variables to have camelCase. I don't think I'm inconsistent in it...

Flame wars have been started over less... laugh.png


Edited by Mnemotic, 07 January 2014 - 01:11 PM.


#7 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 07 January 2014 - 01:16 PM

The COORD type is part of Windows.h. I'm using a lot of things from it, such as SetConsoleCursorPosition. This seems to be its defintion:

typedef struct _COORD {
    SHORT X;
    SHORT Y;
} COORD, *PCOORD;

I haven't written a virtual destructor for Entity. Is that essential? I don't create any new objects inside them, so I figured the default one would kick in, or am I forgetting something? If so, what should I put as a virtual destructor?

 

Also, what's the problem with my convention, from your point of view?


Edited by Akashi, 07 January 2014 - 01:16 PM.


#8 Melkon   Members   -  Reputation: 505

Like
0Likes
Like

Posted 07 January 2014 - 01:17 PM

 The const was to specify that the getter shouldn't change anything, and that it's just to return a value. Whether or not it's there doesn't change the result.

Well, in your example when someone take a look on your code, he will see that there is a getter, that return a const copy value, but it actually can modify the object. In my example you see in the interface that the getter is unable to modify the object (if you try it you get compile error), and the function return a copied value, just as yours. However a copied value shouldn't be const.


Edited by Melkon, 07 January 2014 - 01:18 PM.


#9 Pink Horror   Members   -  Reputation: 1203

Like
0Likes
Like

Posted 07 January 2014 - 01:31 PM


@Mnemotic Yeah, I've stepped through it. The X and Y values are just what I set them to be; X of 20, Y of 10, but when it's returned, it consistently gives me -12851 for both the X and Y variables.
 

 

Is it only when you use COORD? What happens if you make a GetX and GetY instead of using a GetPos?

Also, what's the value of the "other" pointer? What is the "Position" of the this object?



#10 Nypyren   Crossbones+   -  Reputation: 4306

Like
4Likes
Like

Posted 07 January 2014 - 01:31 PM

(short)-12851 is 0xCDCD.  That is a well-known bit pattern that indicates heap memory that you have allocated but not initialized (i.e. uninitialized variable on the heap).


Edited by Nypyren, 07 January 2014 - 01:34 PM.


#11 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 07 January 2014 - 01:57 PM

@Melkon I get you now. I guess the syntax just slipped my mind.

 

@Pink Horror Using GetX/GetY gives me the same result.

 

@Nypyren I figured it was some weird memory thing, because it gives me the same value regardless of what the value actually is.

 

I guess I forgot to mention, the "other" object is a Block object, which is practically identical to the Player object, except it has a different sprite and a velocity of 0. The problem is that it IS initialized, the COORD variable DOES have actual values, and if I output the coordinates from the Block object, it gives me the correct result. The only time it gives me issues is when I pass the Position (20, 10) to somewhere outside the Block object.


Edited by Akashi, 07 January 2014 - 01:59 PM.


#12 ApochPiQ   Moderators   -  Reputation: 15741

Like
0Likes
Like

Posted 07 January 2014 - 02:03 PM

Show the constructors for Block and Player.



#13 Akashi   Members   -  Reputation: 268

Like
0Likes
Like

Posted 07 January 2014 - 02:36 PM

Block::Block(short x, short y) :
    ID (eIDBlock)
{
    Position.X = x;
    Position.Y = y;
    Dimensions.X = 16;
    Dimensions.Y = 16;
    Velocity.X = 1;
    Velocity.Y = 0;
    Velocity.Z = 0;
}
Player::Player(short x, short y):
    ID (eIDPlayer)
{
    Position.X = x;
    Position.Y = y;
    Dimensions.X = 16;
    Dimensions.Y = 16;
    Dimensions.Z = 0;
}


#14 Melkon   Members   -  Reputation: 505

Like
1Likes
Like

Posted 07 January 2014 - 02:47 PM

Check IN the getter if they have a valid value before the return, or not.



#15 Nypyren   Crossbones+   -  Reputation: 4306

Like
1Likes
Like

Posted 07 January 2014 - 02:47 PM

I would put a data breakpoint on the COORD (well, the associated one in heap memory, not the temporary/stack variables) that reports bad values to find out exactly when it's being changed.  It's possible you're stomping over memory in some completely unrelated code.


Edited by Nypyren, 07 January 2014 - 02:50 PM.


#16 phil_t   Crossbones+   -  Reputation: 3922

Like
4Likes
Like

Posted 07 January 2014 - 02:48 PM

Can we see the declaration for Block? And what is ID? Does it inherit from Entity? What's your complete inheritance chain?

 

Only thing I can think of is that maybe you have duplicate declarations of Position (i.e. declared in Block and again in Entity, and you're only setting the value in Block).



#17 Nypyren   Crossbones+   -  Reputation: 4306

Like
1Likes
Like

Posted 07 January 2014 - 02:52 PM

Agreed with phil_t - if you have an extra Position accidentally defined in your derived classes, really "fun" things can happen.


Edited by Nypyren, 07 January 2014 - 02:52 PM.


#18 boogyman19946   Members   -  Reputation: 1063

Like
1Likes
Like

Posted 07 January 2014 - 03:14 PM

I don't know how to get rid of this code box :D

 

 

I haven't written a virtual destructor for Entity. Is that essential? I don't create any new objects inside them, so I figured the default one would kick in, or am I forgetting something? If so, what should I put as a virtual destructor?

 

 

The consequences of non-virtual destructors in polymorphic classes have more clever effects than that. When you call a destructor of a derived class, the base class destructors get called automatically whether they are virtual or not, but when you cast a derived class to it's base class (as is often the case) there is no guarantee that the derived destructor will be called. The results of this are undefined behavior, although generally you'll end up destroying only the base part of the object.


"If highly skilled generalists are rare, though, then highly skilled innovators are priceless." - ApochPiQ

My personal links :)
- Khan Academy - For all your math needs
- Java API Documentation - For all your Java info needs :D
- C++ Standard Library Reference - For some of your C++ needs ^.^

#19 Akashi   Members   -  Reputation: 268

Like
1Likes
Like

Posted 07 January 2014 - 07:05 PM

@Melkon/Nypyren Yeah, I tried that.

 

@phil_t Okay, so I looked it over, and it turns out that I did redeclare Position, Velocity, and Dimensions in my derived objects. I took them out and now it works perfectly.

 

@boogyman So what exactly would I put for a virtual destructor? For anything, really? They don't have any assets I necessarily need to delete, I think.

 

Once again, thank you all for your help.



#20 Mnemotic   Members   -  Reputation: 340

Like
2Likes
Like

Posted 07 January 2014 - 07:33 PM


@boogyman So what exactly would I put for a virtual destructor? For anything, really? They don't have any assets I necessarily need to delete, I think.
It still needs to be marked as virtual. So write one that does exactly nothing.
virtual ~Entity() { }





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