Jump to content

  • Log In with Google      Sign In   
  • Create Account

C++ array always has the same values for each element


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

#1 gchris6810   Members   -  Reputation: 207

Like
0Likes
Like

Posted 06 October 2013 - 12:36 PM

Hi,

 

i'm sure there is an easy answer to this but I am finding it particularly difficult. I want to store mesh data in a static array of C++ classes but when I allocate to each element in the array it sets all the elements in the array with the same value. I initialize the array as follows:

mMeshes = new AnimableMesh[mNumMeshes];

.........

for( int i = 0; i < mNumMeshes; i++ )
{
    mMeshes[i].SetMaterial(materialName); // this sets the material but all elements are set to this value
    .....
}

Normally I would use the C malloc function which works fine but it isn't ideal for class initialization.

 

EDIT: I declare the class array as an instance variable in the class definition.

 

Thanks.


Edited by gchris6810, 06 October 2013 - 12:45 PM.


Sponsor:

#2 SiCrane   Moderators   -  Reputation: 9598

Like
6Likes
Like

Posted 06 October 2013 - 12:42 PM

Well if you initialize all the elements the same way wouldn't it make sense that they all have the same value? What do you want it to do differently?



#3 gchris6810   Members   -  Reputation: 207

Like
-3Likes
Like

Posted 06 October 2013 - 12:44 PM

No, sorry, that was just example code. in the actual code it uses a variable read in from the file. I have validated that each element read from the file is unique.



#4 SiCrane   Moderators   -  Reputation: 9598

Like
6Likes
Like

Posted 06 October 2013 - 12:47 PM

Then show the actual code that's giving you problems.



#5 gchris6810   Members   -  Reputation: 207

Like
0Likes
Like

Posted 06 October 2013 - 12:49 PM

OK here it is, but I though it would be easier if you had a more concise representation.

mMeshes = new AnimableMesh[mNumMeshes];

		file->Seek( mJointOfs, File::IO_SEEK_SET );

		mBindPose = new AnimableMesh::Joint[mNumJoints];

		/* Load the armature's bind pose position and orientation */

		for( int i = 0; i < mNumJoints; i++ )
		{
			file->ReadString( mBindPose[i].name, 64 );

			mBindPose[i].parent = file->ReadDword( );
			
			ReadVector3( mBindPose[i].pos, file );
	
			ReadQuaternion( mBindPose[i].orient, file );
			mBindPose[i].orient.ComputeW( );
		}
	
		/* Load the mesh data */
	
		file->Seek( mMeshesOfs, File::IO_SEEK_SET );
	
		for( int meshIndex = 0; meshIndex < mNumMeshes; meshIndex++ )
		{
			char material[64];
			file->ReadString( material, 64 );
			mMeshes[meshIndex].SetMaterialName( material );
		        /* Allocate all the mesh data */
			mMeshes[meshIndex].AllocVertices( file->ReadDword( ) );
			mMeshes[meshIndex].AllocIndices( file->ReadDword( ) );
			mMeshes[meshIndex].AllocWeights( file->ReadDword( ) );
		
			for( int i = 0; i < mMeshes[meshIndex].GetNumVertices( ); i++ )
			{
				AnimableMesh::Vertex *vertex = mMeshes[meshIndex].GetVertexPtr( i );
				Math::DrawVert *drawVert = mMeshes[meshIndex].GetDrawVertPtr( i );
		
				int vertexNumber = file->ReadDword( );
	
				ReadVector2( drawVert->uv, file );
				vertex->start = file->ReadDword( );
				vertex->count = file->ReadDword( );
			}
		
			for( int i = 0; i < mMeshes[meshIndex].GetNumIndices( ); i++ )
			{
				int triCount = file->ReadDword( );
		
				short *index1 = mMeshes[meshIndex].GetIndexPtr( i*3 );
				short *index2 = mMeshes[meshIndex].GetIndexPtr( i*3+1 );
				short *index3 = mMeshes[meshIndex].GetIndexPtr( i*3+2 );
		
				index1 = (short*)file->ReadWord( );
				index2 = (short*)file->ReadWord( );
				index3 = (short*)file->ReadWord( );
			}
	
			for( int i = 0; i < mMeshes[meshIndex].GetNumWeights( ); i++ )
			{
				int weightCount = file->ReadDword( );
		
				AnimableMesh::Weight* weight = mMeshes[meshIndex].GetWeightPtr( i );
				
				weight->joint = file->ReadDword( );
				weight->weight = file->ReadFloat( );
			
				ReadVector3( weight->pos, file );
			}

			PrepareMeshes( mBindPose );
			
			mMeshes[meshIndex].CreateBuffers( );
		}


#6 Adam_42   Crossbones+   -  Reputation: 2507

Like
5Likes
Like

Posted 06 October 2013 - 04:03 PM

My guess is that the bug is inside SetMaterialName(). It's probably something like this:

class AnimableMesh
{
private:
   const char *m_Name;
public:
   // Does not copy name, just copies pointer to where it's stored (which in this case is the stack).
   void SetMaterialName(const char *name) { m_Name = name; }
}

There's a few ways to fix it, the simplest is:

#include <string>

class AnimableMesh
{
private:
   std::string m_Name;
public:
   // This will actually make a copy of the string
   void SetMaterialName(const char *name) { m_Name = name; }
}

You could also allocate memory and copy the string manually. Don't forget to update the destructor, assignment operator and copy constructor if you do that.

 

Edit: I should also probably mention that using malloc() in C++ code is generally a bad idea. operator new does the same thing but more safely.  Firstly malloc() doesn't return pointers of the correct type so you end up with a reinterpret_cast<>() whenever you use it. In addition it won't properly initialize non-POD classes and structs, so if you change a class or struct you may also need to change everywhere you allocate memory for them.

 

The only thing to watch out for is that new[] must be matched with delete[] and new with delete.


Edited by Adam_42, 06 October 2013 - 04:14 PM.


#7 gchris6810   Members   -  Reputation: 207

Like
0Likes
Like

Posted 07 October 2013 - 09:11 AM

Thanks. It worksbiggrin.png I just used strcpy.  For future reference what was it that made all the objects in the array set to the same value?



#8 Adam_42   Crossbones+   -  Reputation: 2507

Like
3Likes
Like

Posted 07 October 2013 - 04:08 PM

The pointers were all pointing at the same memory location. It will have been set to the value of the last thing you processed.



#9 kunos   Crossbones+   -  Reputation: 2207

Like
1Likes
Like

Posted 08 October 2013 - 12:37 AM

Thanks. It worksbiggrin.png I just used strcpy.  For future reference what was it that made all the objects in the array set to the same value?

 

the lesson to bring home here is: if you are working with C++, USE C++ and forget about C.

The code you have posted is bound to fail in subtle ways (as you have discovered). Unless you have very good reasons not to, ALWAYS use std::string instead of char* and ALWAYS use std::vector instead of old style arrays... the code will end up being easier to read for you, for people trying to help you, for people you work with and it will also be safer.


Edited by kunos, 08 October 2013 - 12:38 AM.

Stefano Casillo
Lead Programmer
TWITTER: @KunosStefano
AssettoCorsa - netKar PRO - Kunos Simulazioni




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