Allegro.cc - Online Community

Allegro.cc Forums » Allegro Development » Virtual function does not work... (or i missed something(iprobably did))

Credits go to Edgar Reynaldo for helping out!
This thread is locked; no one can reply to it. rss feed Print
Virtual function does not work... (or i missed something(iprobably did))
mumu1000
Member #16,842
April 2018

Well... this issue is so weird i don't know how to synthesize it...
I'm just gonna post the code here, and tell you what i've figured out so far.
Feel free to test it and tell me what output you get ! (Only Allegro lib and std used)

This is going to be some sort of space game, yet its more a test for me to learn than an actual game ;)

main.cpp

#SelectExpand
1#include <iostream> 2#include <string> 3#include <vector> 4#include <sstream> 5#include <chrono> 6#include <thread> 7#include "allegro5\allegro.h" 8#include "allegro5\allegro_color.h" 9#include "allegro5/allegro_primitives.h" 10#include "Chunk.h" 11#include <random> 12#include "Planet.h" 13#include "AstronomicalObject.h" 14 15void testDraw(AstronomicalObject *ast) 16{ 17 ast->draw(); 18 if ( typeid(*ast) == typeid(Planet) ) 19 { 20 std::cout<<"PLANET"; 21 }else 22 { 23 std::cout<<"imanobject"; 24 } 25} 26int main(int argc, char **argv) 27{ 28 auto now = std::chrono::system_clock::now(); 29 auto now_ms = std::chrono::time_point_cast<std::chrono::milliseconds>(now); 30 auto value = now_ms.time_since_epoch(); 31 unsigned int duration = value.count(); 32 33 srand(duration); 34 if (al_init() && al_init_primitives_addon()) 35 { 36 std::cout<<"It Works"; 37 }else 38 { 39 std::cout << "Allegro Not Loaded"; 40 return 1; 41 } 42 al_set_new_display_flags(ALLEGRO_FRAMELESS); 43 ALLEGRO_DISPLAY * mainDisplay = al_create_display(1800,1000); 44 Chunk testChunk; 45 Planet * testPlanet = new Planet(3,6); 46 testPlanet ->draw(); 47 al_flip_display(); 48 std::this_thread::sleep_for(std::chrono::milliseconds(1000)); 49 for (int u = 0;u<5;u++) 50 { 51 testChunk = Chunk(true); 52 std::cout<<testChunk.getAstroObjListSize(); 53 al_clear_to_color(al_map_rgb(20,20,20)); 54 for (unsigned int i = 0; i<(testChunk.getAstroObjListSize());i++) 55 { 56 testDraw(testChunk.getAstroObjBylistIndex(i)); 57 } 58 al_flip_display(); 59 std::this_thread::sleep_for(std::chrono::milliseconds(1000)); 60 61 } 62 63 //al_set_target_bitmap(nullptr); //does some weird access violation 64 //al_destroy_display(mainDisplay); 65 al_shutdown_primitives_addon(); 66 return 0; 67}

Chunk.h

#SelectExpand
1#ifndef CHUNK_H 2#define CHUNK_H 3#include "AstronomicalObject.h" 4#include "Planet.h" 5#include <vector> 6 7 8class Chunk 9{ 10 public: 11 Chunk(); 12 Chunk(bool ranGen); 13 virtual ~Chunk(); 14 void addAstroObject(AstronomicalObject *toBeAdded); 15 AstronomicalObject* getAstroObjBylistIndex(unsigned int x){if(x<m_listeContenu.size()){return m_listeContenu[x];}else{return nullptr;}}; 16 unsigned int getAstroObjListSize(){return m_listeContenu.size();}; 17 protected: 18 std::vector<AstronomicalObject*> m_listeContenu; 19 private: 20}; 21 22#endif // CHUNK_H

Chunk.cpp

#SelectExpand
1#include "Chunk.h" 2#include <math.h> 3#include "iostream" 4 5Chunk::Chunk() 6{ 7 8} 9Chunk::Chunk(bool ranGen) 10{ 11 if(ranGen) 12 { 13 unsigned int numbOfObj = rand()%30; 14 std::cout<<" number:"<< numbOfObj <<". "; 15 for (unsigned int i = 0; i<numbOfObj;i++) 16 { 17 addAstroObject(new AstronomicalObject(rand()%320,rand()%320)); //multiple tries to make it work 18 addAstroObject(new Planet(rand()%320,rand()%320)); 19 m_listeContenu.push_back(new Planet(rand()%320,rand()%320)); 20 } 21 } 22} 23 24Chunk::~Chunk() 25{ 26 for (unsigned int i = 0;i<m_listeContenu.size();i++) 27 { 28 delete m_listeContenu[i]; 29 } 30} 31void Chunk::addAstroObject(AstronomicalObject * toBeAdded) 32{ 33 m_listeContenu.push_back(toBeAdded); 34}

AstronomicalObject.h

#SelectExpand
1#ifndef ASTRONOMICALOBJECT_H 2#define ASTRONOMICALOBJECT_H 3#include "allegro5/color.h" 4#include "allegro5/allegro_primitives.h" 5#include <iostream> 6 7 8class AstronomicalObject 9{ 10 public: 11 12 AstronomicalObject(); 13 AstronomicalObject(float xPos, float yPos); 14 15 virtual ~AstronomicalObject(); 16 float const getXPos(){return m_xPos;}; 17 float const getYPos(){return m_yPos;}; 18 void setXPos(float xPos){m_xPos = xPos;}; 19 void setYPos(float yPos){m_yPos = yPos;}; 20 virtual void draw() const; 21 22 protected: 23 float m_xPos; 24 float m_yPos; 25 bool m_isAPlanet; 26 private: 27}; 28 29#endif // ASTRONOMICALOBJECT_H

AstronomicalObject.cpp

#SelectExpand
1#include "AstronomicalObject.h" 2 3AstronomicalObject::AstronomicalObject() 4{ 5 m_xPos = 0; 6 m_yPos = 0; 7 m_isAPlanet = false; 8} 9 10AstronomicalObject::AstronomicalObject(float xPos, float yPos) 11{ 12 m_xPos = xPos; 13 m_yPos = yPos; 14 m_isAPlanet = false; 15} 16 17AstronomicalObject::~AstronomicalObject() 18{ 19} 20 21void AstronomicalObject::draw() const 22{ 23 if(!m_isAPlanet) 24 { 25 al_draw_circle(10+(m_xPos*3),10+(m_yPos*3),8,al_map_rgb(200,200,200),3); 26 } 27 else 28 { 29 al_draw_circle(10+(m_xPos*3),10+(m_yPos*3),8,al_map_rgb(20,200,20),3); 30 } 31}

Planet.h

#SelectExpand
1#ifndef PLANET_H 2#define PLANET_H 3 4#include <iostream> 5#include "AstronomicalObject.h" 6#include "allegro5/color.h" 7#include "allegro5/allegro_primitives.h" 8 9 10class Planet : public AstronomicalObject 11{ 12 public: 13 Planet(); 14 Planet(float xPos, float yPos); 15 virtual ~Planet(); 16 virtual void draw() const; 17 18 protected: 19 bool m_isWaterPresent; 20 21 private: 22}; 23 24#endif // PLANET_H

Planet.cpp

#SelectExpand
1#include "Planet.h" 2Planet::Planet() : AstronomicalObject() 3{ 4 m_isWaterPresent = true; 5 m_isAPlanet = true; 6} 7Planet::Planet(float xPos, float yPos) 8{ 9 m_xPos = xPos; 10 m_yPos = yPos; 11 m_isAPlanet = true; 12 m_isWaterPresent = false; 13} 14 15Planet::~Planet() 16{ 17 18} 19void Planet::draw() const 20{ 21 std::cout<<"I am A Planet"; 22 al_draw_filled_circle(10+(m_xPos*3),10+(m_yPos*3),8,al_map_rgb(20,200,20)); 23}

Outcome:
i get a plain green circle on my display, as expected, then i get 5 screens with some green and grey hollow circles... the thing is i should get grey hollow circles and PLAIN green circles... Something is going wrong.

There we are, these are all the sources i've got.
What it seems is the virtual draw function from AstronomicalObject overriden by Planet does not do what its supposed to do (no runtime link resolution).
No matter whether the pointer points to a Planet or an AstronomicalObject, it will call the AstroObj version of the draw function.

I've spent more than 5 hours trying to debug and understand what goes wrong, but... it just makes no sense ! It i supposed to work, that's what virtual functions are made for !

I really hope you will discover where the issue come from, cuz i'm dispairing ! :'(

Thx for you help ! <3

PS: As you may have noticed, i'm quite new to programming, so maybe the issue is dumb easily fixable, and i'm just too bad to find out how to.

Edgar Reynaldo
Member #8,592
May 2007
avatar

If you want people to try your code, provide a zip file with the sources in it.

mumu1000
Member #16,842
April 2018

Of course !

EDIT: Sorry the link didnt work ! You have it attached on my next post.

There you go ! (Im working with code::blocks, and i'm using MinGW64 Compiler)

Edgar Reynaldo
Member #8,592
May 2007
avatar

Your link doesn't work.
{"name":"611634","src":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/a\/6\/a626c7009a4e4585ac18f6a2a43739da.png","w":1100,"h":552,"tn":"\/\/djungxnpq2nug.cloudfront.net\/image\/cache\/a\/6\/a626c7009a4e4585ac18f6a2a43739da"}611634

Just attach the zip file of your source.

mumu1000
Member #16,842
April 2018

Nani ??

Well, there you go then :p

Edgar Reynaldo
Member #8,592
May 2007
avatar

Every time you assign testChunk to a new Chunk(true) object on the stack you reference destroyed memory.

        testChunk = Chunk(true);

This calls Chunk::operator=(const Chunk& c);. The default assignment operator merely copies the data, so your vector<AstronomicalObject*> gets copied. This is a shallow copy. Now you have two copies of the same astronomical objects. Then the temporary Chuck(true) object goes out of scope, deleting every single object that was in it. Now your testChunk object holds pointers to deleted memory. That is why it segfaults.

2. You're using 'const' wrong. It goes at the end of the function declaration before the semi-colon.

class X {
public :
   void function() const;/// I promise I won't change anything
};

mumu1000
Member #16,842
April 2018

Oh
My
God

I still dont understand why it didnt work (i will definitely think about what you said, and try to understand) but using a pointer to a Chunk object instead of the Chunk object itself solved the sh*t !

My main.cpp is now

#SelectExpand
1 Chunk* testChunk; 2 Planet * testPlanet = new Planet(3,6); 3 testPlanet ->draw(); 4 al_flip_display(); 5 std::this_thread::sleep_for(std::chrono::milliseconds(1000)); 6 for (int u = 0;u<5;u++) 7 { 8 testChunk = new Chunk(true); 9 std::cout<<testChunk->getAstroObjListSize(); 10 al_clear_to_color(al_map_rgb(20,20,20)); 11 for (unsigned int i = 0; i<(testChunk->getAstroObjListSize());i++) 12 { 13 testDraw(testChunk->getAstroObjBylistIndex(i)); 14 } 15 al_flip_display(); 16 std::this_thread::sleep_for(std::chrono::milliseconds(1000)); 17 delete testChunk; 18 19 }

and the virtual function does its job !!

Now your testChunk object holds pointers to deleted memory. That is why it segfaults.

The problem is not a crash (it doesnt crash, no segfault here), the problem was the wrong version of the draw() function was called.

Edgar Reynaldo said:

your vector<AstronomicalObject*> gets copied. This is a shallow copy. Now you have two copies of the same astronomical objects.

The AstronomicalObjects dont get copied, since vector<AstronomicalObject*> holds pointers, not actual objects. So when it copies, it copies the adress stored in the pointers too. The actual AstronomicalObject is still being referenced to by these copied pointers : no memory leaks here. Where am i wrong ?

When you do testChunk = Chunk(true) doesnt it implicitly calls delete on the previous content of testChunk ? ???
I mean if you have something like std::string myString = "Thank You Very Much For Your Help";
then you do myString = "REALLY"; it doesnt leak memory right ?

Edgar Reynaldo said:

You're using 'const' wrong. It goes at the end of the function declaration before the semi-colon.

My bad ! Thx ! ;D

Thank you so much anyway ! You saved my day once again ! :D

Edgar Reynaldo
Member #8,592
May 2007
avatar

Mumu1000 said:

Now your testChunk object holds pointers to deleted memory. That is why it segfaults.

The problem is not a crash (it doesnt crash, no segfault here), the problem was the wrong version of the draw() function was called.

I ran it through gdb, and it segfault'ed. Same code. That's what you get when you dereference bad memory. You get undefined behavior.

mumu1000 said:

your vector<AstronomicalObject*> gets copied. This is a shallow copy. Now you have two copies of the same astronomical objects.

The AstronomicalObjects dont get copied, since vector<AstronomicalObject*> holds pointers, not actual objects. So when it copies, it copies the adress stored in the pointers too. The actual AstronomicalObject is still being referenced to by these copied pointers : no memory leaks here. Where am i wrong ?

Yes, the pointers get copied. But when your temporary object goes out of scope, those pointers get deleted. This means any copies of those pointers now hold references to deleted memory, which will make it segfault.

Mumu1000 said:

but using a pointer to a Chunk object instead of the Chunk object itself solved the sh*t !

That's because you stopped copying your vector. You need to learn about scope and the stack and the heap. Learn about the difference between a shallow copy and a deep copy.

Look at this code :

#SelectExpand
1 2 3 4#include <cstdio> 5 6typedef unsigned int ID; 7const ID BADID = ~0; 8 9ID GetNewId() { 10 static ID id = 0; 11 return id++; 12} 13 14 15 16class Base { 17protected : 18 ID id; 19public : 20 Base() : 21 id(GetNewId()) 22 {} 23 virtual ~Base() {} 24 virtual Base* Clone() { 25 return new Base(*this); 26 } 27 ID GetId() const {return id;} 28}; 29 30 31 32class Derived : public Base { 33public : 34 virtual Base* Clone() { 35 return new Derived(*this); 36 } 37}; 38 39 40 41class BadOwner { 42 Base* b; 43 44public : 45 46 BadOwner() : 47 b(0) 48 {} 49 50 BadOwner(Base* base) : 51 b(base) 52 {} 53 54 BadOwner(const BadOwner& bo) : 55 b(0) 56 { 57 Free(); 58 b = bo.b;/// Shallow copy 59 } 60 61 ~BadOwner() {Free();} 62 63 void Free() { 64 if (b) { 65 printf("BadOwner : Deleting base %p\n" , b); 66 delete b; 67 b = 0; 68 } 69 } 70}; 71 72class GoodOwner { 73 Base* b; 74 75public : 76 77 GoodOwner() : 78 b(0) 79 {} 80 81 GoodOwner(Base* base) : 82 b(base) 83 {} 84 85 GoodOwner(const GoodOwner& go) : 86 b(0) 87 { 88 Free(); 89 if (go.b) { 90 b = go.b->Clone();/// Deep copy 91 } 92 } 93 94 ~GoodOwner() {Free();} 95 96 void Free() { 97 if (b) { 98 printf("GoodOwner : Deleting base %p\n" , b); 99 delete b; 100 b = 0; 101 } 102 } 103}; 104 105 106int main(int argc , char** argv) { 107 108 (void)argc; 109 (void)argv; 110 111 { 112 BadOwner b1(new Derived()); 113 BadOwner b2 = b1; 114 115 GoodOwner g1(new Derived()); 116 GoodOwner g2 = g1; 117 }/// Crashes because b2 deletes b1.b after b1.b is already deleted when they go out of scope 118 119 return 0; 120}

If you run it you can see the output and that it crashes because it destroys the same memory twice. That is what you were doing when you copied your vector using a shallow copy.

GoodOwner : Deleting base 00ca1620
GoodOwner : Deleting base 00ca1600
BadOwner  : Deleting base 00ca2f30
BadOwner  : Deleting base 00ca2f30

mumu1000
Member #16,842
April 2018

Thank you very much ! I'll work on it. :)

Edit:

Yes, the pointers get copied. But when your temporary object goes out of scope, those pointers get deleted. This means any copies of those pointers now hold references to deleted memory, which will make it segfault.

Oh, so what you mean is when this temporary object goes out of scope it implicitly calls the destructor function of every object pointed by the pointers held in this temporary object ? (i'm trying to reformulate to be sure i have well understood what you said ^^) Is that correct ? (i really must look again at basic class definition and operator method overriding courts xD)

Edit Edit:

I'm stupid...
It just calls the destructor of the Chunk class, which i told to delete everything.........
Sorry, i've been kinda slow on this one xD

Edgar Reynaldo
Member #8,592
May 2007
avatar

mumu1000 said:

Oh, so what you mean is when this temporary object goes out of scope it implicitly calls the destructor function of every object pointed by the pointers held in this temporary object ? (i'm trying to reformulate to be sure i have well understood what you said ^^) Is that correct ? (i really must look again at basic class definition and operator method overriding courts xD)

To answer the specific point in bold, no. And yes, because you told it to. When a pointer goes out of scope, nothing happens. When an object goes out of scope, it's destructor is called. Which in this case, was Chunk::~Chunk, which deleted every pointer in the object vector.

Go to: