CopyConstructor

You aren''t copying the arrays properly; use memcpy, not the assignment operator. What your code currently does is copy over the pointer, not the array.

---

How appropriate. You fight like a cow.

There's quite a few new's in there, got all the appropriate deletes?
Edit: Yup, Sneftel is right.

[edited by - Wildfire on August 11, 2003 9:42:20 AM]

The problem is your allocations and assignations lines, like :

m_pMeshes=new Mesh[m_numMeshes];
m_pMeshes=par.m_pMeshes;

I''m sure that in your destructor you call: delete [] m_pMeshes,
and so you''ll delete multiple time the same memory area ...

What you really want to do in your constructor is probably this :

m_pMeshes=new Mesh[m_numMeshes];
for(int m=0; m < m_numMeshes ;++m)
{
m_pMeshes[m] = par.m_pMeshes[m]
}

Do the same thing for Materials, Triangles and vertices

I have try this befor

memcpy(m_pVertices, par.m_pVertices, sizeof(Vertex));

but this dosent works too :-(

My Destructor:
.
.
m_numMeshes = 0;
if ( m_pMeshes != NULL )
{
delete[] m_pMeshes;
m_pMeshes = NULL;
}
.
.
.

quote:

---

memcpy(m_pVertices, par.m_pVertices, sizeof(Vertex));

---

shouldn't that be

memcpy(m_pVertices, par.m_pVertices, par.m_numVertices *sizeof(Vertex));

You want to copy all of the vertices, not just a single one?

edit: Checking the pointer for null, and setting the pointer/counter to null/0 in the destructor is pretty much unneeded. As soon the the object is destroyed no one has access to those anymore.

[edited by - Wildfire on August 11, 2003 9:56:14 AM]

Ok,
the Problem is here

glBegin( GL_TRIANGLES );
{
for ( int j = 0; j < m_pMeshes.m_numTriangles; j++ )
{
int triangleIndex = m_pMeshes[i].m_pTriangleIndices[j];

const Triangle* pTri = &m_pTriangles[triangleIndex];

for ( int k = 0; k < 3; k++ )
{
--------->int index = pTri->m_vertexIndices[k];<-------------
glNormal3fv( pTri->m_vertexNormals[k] );
glTexCoord2f( pTri->m_s[k], pTri->m_t[k] );
glVertex3fv( m_pVertices[index].m_location );
}
}
}
glEnd();

with the Copyconstructor
''index'' have a wrong Value

:-((

did you first assign the memory before calling memcopy? I agree with the rest of them that your problem is here. In the destructor, it doesn''t matter calling delete on NULL it will not crash your program, but calling delete on memory that has already been freed will crash.
If you don''t like memcopy, use the for loop as suggested by Sphax.

Wildfire,
yes your right, but this dosent work

m_pMeshes=new Mesh[m_numMeshes];
memcpy(m_pMeshes, par.m_pMeshes, par._numMeshes*sizeof(Mesh));

desertcube,

that''s exactly is my Problem, the Destructor try delete Memory that dosent exist.
But i dont no how avoid that.
I belief the Copyconstructor works wrong, thats the Problem.

define "doesn''t work". What line does it crash on?

---

How appropriate. You fight like a cow.

Ok,
here my Code

Copyconstructor

CMSModel(const CMSModel ∥)
{
m_position=par.m_position;

m_timedelta=par.m_timedelta;

m_velocity=par.m_velocity;

m_speed=par.m_speed;

m_numMeshes=par.m_numMeshes;

m_pMeshes=new Mesh[m_numMeshes];
memcpy(m_pMeshes, par.m_pMeshes, par.m_numMeshes*sizeof(Mesh));

m_numMaterials=par.m_numMaterials;


m_pMaterials=new Material[m_numMaterials];
memcpy(m_pMaterials, par.m_pMaterials, par.m_numMaterials*sizeof(Material));

m_numTriangles=par.m_numTriangles;

m_pTriangles=new Triangle[m_numTriangles];
memcpy(m_pTriangles, par.m_pTriangles, par.m_numTriangles*sizeof(Triangle));

m_numVertices=par.m_numVertices;

m_pVertices= new Vertex[m_numVertices];
memcpy(m_pVertices, par.m_pVertices, par.m_numVertices*sizeof(Vertex));

Texture=par.Texture;
}


My assignment operator:



CMSModel& operator=(const CMSModel ∥)
{
if(this==∥)
return *this;

m_position=par.m_position;

m_timedelta=par.m_timedelta;

m_velocity=par.m_velocity;

m_speed=par.m_speed;

m_numMeshes=par.m_numMeshes;

delete[] m_pMeshes;
m_pMeshes=new Mesh[m_numMeshes];
memcpy(m_pMeshes, par.m_pMeshes, par.m_numMeshes*sizeof(Mesh));

m_numMaterials=par.m_numMaterials;

delete[] m_pMaterials;
m_pMaterials=new Material[m_numMaterials];
memcpy(m_pMaterials, par.m_pMaterials, par.m_numMaterials*sizeof(Material));

m_numTriangles=par.m_numTriangles;

delete[] m_pTriangles;
m_pTriangles=new Triangle[m_numTriangles];
memcpy(m_pTriangles, par.m_pTriangles, par.m_numTriangles*sizeof(Triangle));

m_numVertices=par.m_numVertices;

delete[] m_pVertices;
m_pVertices= new Vertex[m_numVertices];
memcpy(m_pVertices, par.m_pVertices, par.m_numVertices*sizeof(Vertex));

Texture=par.Texture;

return *this;
}

the Destructor (works well without Copyconstructor ;-) )

CMSModel::~CMSModel()
{
int i;
for ( i = 0; i < m_numMeshes; i++ )
delete[] m_pMeshes.m_pTriangleIndices;


for ( i = 0; i < m_numMaterials; i++ )
delete[] m_pMaterials[i].m_pTextureFilename;


if ( m_pMeshes != NULL )
{
delete[] m_pMeshes;
}

if ( m_pMaterials != NULL )
{
delete[] m_pMaterials;
}

if ( m_pTriangles != NULL )
{
delete[] m_pTriangles;
}

if ( m_pVertices != NULL )
{
delete[] m_pVertices;
}
}

The Program crashed when its call the Destructor for the Copy Object

quote:

---

Original post by wolf_10
The Program crashed when its call the Destructor for the Copy Object

---

Yes, we know that. On what line in the destructor?

---

How appropriate. You fight like a cow.

What you are doing in your copy constructor is creating whats called a shallow copy and is exactly what the default copy constructor does. The problem with this is when you start using pointers. Say you have a pointer that points to A. In your copy constructor, you simply tell the new class to point to A aswell, what you need to do is tell it to point to B, but make sure B holds the same data as A.
So, here's a short example: (note, i haven't had time to test it)

class MyClass
{
public:
	MyClass():itsPointer(0) {} //Constructor set pointers to null

	MyClass(const MyClass &); //Copy constructor

	~MyClass() {delete itsPointer;} //Destructor


	int GetData() const {return *itsPointer;}
	void SetData(int newData) {*itsPointer = newData;}
private:
	int * itsPointer;
};

MyClass::MyClass(const MyClass & rhs)
{
	itsPointer = new int;
	if (!itsPointer) //Couldn't get any memory

		return; //Error


	*itsPointer = *rhs.itsPointer; //Set itsPointer to the VALUE of rhs's itsPointer

}

In your example though you have an array, so you'd need to go through each element in a for loop.
Hope this helps

[edited by - desertcube on August 11, 2003 11:09:15 AM]

sorry ;-)

crashed here

if ( m_pMeshes != NULL )
{
delete[] m_pMeshes;
}

hold on, after veiwing you recent post, why are you doing this

for ( i = 0; i < m_numMeshes; i++ )
delete[] m_pMeshes.m_pTriangleIndices;

Ah, okay. The problem here is that you have arrays of arrays. These sub-arrays must have space allocated for their copies with new and be moved with a nested for-loop, as before.

---

How appropriate. You fight like a cow.

sorry,
i''am away for few houres
i try it later.

thanks a lot to all for help