Game Development Community

[T3D1.1B1 and earlier] - possible issue with ArrayObject deletion - RESOLVED

by Guy Allard · in Torque 3D Professional · 05/19/2010 (8:58 am) · 5 replies

The onRemove function for the ArrayObject class looks like this:
void ArrayObject::onRemove()
{
   mArray.empty();
   Parent::onRemove();
}

it looks as if calling mArray.empty() is intended to clear the contents of mArray, which is a Vector.

However, Vector::empty() does not clear out the vector, it only returns a bool indicating whether or not the vector is empty.

change
mArray.empty();
to
empty();

and it will clean up.

#1
05/19/2010 (9:16 am)
Gah, it appears that this class is straight from this thread:
http://www.torquepowered.com/community/resources/view/4711

but without any of the bug fixes since it's creation in 2003!

So the other issues mentioned in that thread, such as using a U32 to hold a value of -1 to indicate whether an index has been found in getIndexFromValue or getIndexFromKey are also missing.

#2
05/19/2010 (9:31 am)
I use ArrayObjects all over -- Is there any chance these fixes will make it for B2? It should be a pretty trivial update.
#3
05/26/2010 (10:41 am)
I honestly can't believe that this has not be updated by GG into the Head code, here are the changes as I have gathered from the previous thread and some I saw as well for T3D 1.1 B1

void ArrayObject::onRemove()
{
	// Bug Fix -> (Actually empty the list on removal)
	empty();
	Parent::onRemove();
}

// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
S32 ArrayObject::getIndexFromValue( const String &value ) const
{
	// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
	S32 foundIndex = -1;
        /// Rest of function unchanged
	...
}

// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
S32 ArrayObject::getIndexFromKey( const String &key ) const
{
	// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
	S32 foundIndex = -1;
        /// Rest of function unchanged
	...
}

// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
S32 ArrayObject::getIndexFromKeyValue( const String &key, const String &value ) const
{
	// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
	S32 foundIndex = -1;
        /// Rest of function unchanged
	...
}

// Bug Fix -> ( Allow for S32 since GetIndex functions can return -1 )
const String& ArrayObject::getKeyFromIndex( S32 index ) const
{
	// Bug Fix -> ( Replaced > with >= and removed the -1, it seemed redundant math)
	if ( index >= mArray.size() || index < 0 || mArray.size() == 0 )
		return String::EmptyString;

	return mArray[index].key;
}

// Bug Fix -> ( Allow for S32 since GetIndex functions can return -1 )
const String& ArrayObject::getValueFromIndex( S32 index ) const
{
	// Bug Fix -> ( Replaced > with >= and the -1 it seemed like redundant math)
	if( index >= mArray.size() || index < 0 || mArray.size() == 0 )
		return String::EmptyString;

	return mArray[index].value;
}

void ArrayObject::insert( const String &key, const String &value, U32 index )
{
	index = mClampF( index, 0, mArray.size() - 1 );

	// Bug Fix -> (Changed because size could become negative and crash if left like it was)
	U32 size = mArray.size();
        size = mClampF( size, 0, mArray.size() - 1 );

	for( U32 i = size; i >= index; i-- )
		moveIndex( i, i + 1 );
	// <- FIX

	Element temp;
	temp.value = value;
	temp.key = key;
	mArray[index] = temp;
}

void ArrayObject::moveIndex(U32 prev, U32 index)
{
	if(index >= mArray.size())
		push_back(mArray[prev].key, mArray[prev].value);
	else
		mArray[index] = mArray[prev];
	// Bug Fix -> (Changed from index to prev otherwise it wipes what you are copying)
	mArray[prev].value = String::EmptyString;
	mArray[prev].key = String::EmptyString;
	// <- FIX
}

U32 ArrayObject::moveLast()
{
	// Bug Fix -> (Changed to check for empty array otherwise it would return -1 for the last index when empty)
	if (mArray.empty())
		mCurrentIndex = 0;
	else
		mCurrentIndex = mArray.size() - 1;
	// <- FIX

	return mCurrentIndex;
}

// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
S32 ArrayObject::moveNext()
{
	if ( mCurrentIndex >= mArray.size() - 1 )
		return -1;

	mCurrentIndex++;

	return mCurrentIndex;
}

// Bug Fix -> (Changed from U32 to S32 to account for -1 returns)
S32 ArrayObject::movePrev()
{
	if ( mCurrentIndex <= 0 )
		return -1;

	mCurrentIndex--;

	return mCurrentIndex;
}

ConsoleMethod( ArrayObject, getValue, const char*, 3, 3, "(int index)"
			  "Get the value of the array element at the submitted index" )
{
	// Bug Fix -> (StringTableEntry's are write once, read many no need to use a buffer to limit our return, changed from key to value name for variable)
	StringTableEntry value = object->getValueFromIndex( dAtoi( argv[2] ) );
	return value;
	// <- FIX
}

ConsoleMethod( ArrayObject, getKey, const char*, 3, 3, "(int index)"
			  "Get the key of the array element at the submitted index" )
{
	// Bug Fix -> (StringTableEntry's are write once, read many no need to use a buffer to limit our return)
	StringTableEntry key = object->getKeyFromIndex( dAtoi( argv[2] ) );
	return key;
	// <- FIX
}
#4
05/28/2010 (12:47 pm)
bug logged Key: TQA-158
#5
09/06/2010 (11:10 pm)
Fixed in 1.1 Beta 3.