[SOLVED] Vector Deleting Error

Hey guys, I'm trying to delete an element of a vector with an iterator using the erase function. Here's the basics of my way of trying to delete it.

In my startup code I'm creating the vector and iterator like this. Yes it does have to be like this as it's for an assessment.
1
2
3
4
5
6
//Create a vector that holds 20 Knight Objects from the structure
	std::vector<Knight> *myKnightsOne = new std::vector<Knight>(5, Knight());
	std::vector<Knight> *myKnightsTwo = new std::vector<Knight>(5, Knight());
	//Create an iterator that can travel through the vector to pull each Knight Object out
	std::vector<Knight>::iterator *myKnightsOneIterator = new std::vector<Knight>::iterator;
	std::vector<Knight>::iterator *myKnightsTwoIterator = new std::vector<Knight>::iterator;


In the startup above when i call this function - along with the player movement functions. This code activates itself when 2 of the vector elements touch.
Now the fight between them is normal - they hit each other until one of them has health thats less than 0. Only problem is when that happens I get an error about insertion errors.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
void Fight(std::vector<Knight> *pVecOne, std::vector<Knight> *pVecTwo, std::vector<Knight>::iterator *pItOne, std::vector<Knight>::iterator *pItTwo)
{
	for(*pItOne = pVecOne->begin(); *pItOne != pVecOne->end(); ++*pItOne)
	{
		for(*pItTwo = pVecTwo->begin(); *pItTwo != pVecTwo->end(); ++*pItTwo)
		{
			if(( (*pItOne)->x == (*pItTwo)->x ) && ( (*pItOne)->y == (*pItTwo)->y ))
			{
				while( (*pItOne)->health >= 1 && (*pItTwo)->health >= 1 )
				{
					(*pItOne)->speed = rand() % 10;
					(*pItTwo)->speed = rand() % 10;

					if( (*pItOne)->speed > (*pItTwo)->speed )
					{
						std::cout << "ArmyOne Hits ArmyTwo" << std::endl;
						if((*pItTwo)->armour > 0)
						{
							(*pItTwo)->armour -= (*pItOne)->damage;
						}
						else						
						{
							(*pItTwo)->health -= (*pItOne)->damage;
						}
					}
					else if( (*pItOne)->speed == (*pItTwo)->speed )
					{
						std::cout << "Players Both Miss Each Other" << std::endl;
					}
					else
					{
						std::cout << "ArmyTwo Hits ArmyOne" << std::endl;
						if((*pItOne)->armour > 0)
						{
							(*pItOne)->armour -= (*pItTwo)->damage;
						}
						else						
						{
							(*pItOne)->health -= (*pItTwo)->damage;
						}
					}
				}
			}

			//Test Delete Vector Element------------------------------------------------------
			if((*pItOne)->health <= 0)
			{
				pVecOne->erase((*pItOne));
			}

			if((*pItTwo)->health <= 0)
			{
				pVecTwo->erase((*pItTwo));
			}
		}
	}
}


The code just above this line is where I'm having the problem trying to delete the element from the vector. I've tried the two IF statements in different spots inside this function. But they all end up with errors.

I've tried also using
pVecOne->erase(*pItOne);
But that gives me pretty much the same problem

So my question is how do I remove the element and where should i put this?
Last edited on
delete *this ?
I didn't think that would work but I Gave it a shot
error C2673: 'Fight' : global functions do not have 'this' pointers
So no luck in that one sorry.

I edited the function to this. But now the program just ends normally when two of the elements touch. Just says "Press any key to continue".

This is how i changed it.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
void Fight(std::vector<Knight> *pVecOne, std::vector<Knight> *pVecTwo, std::vector<Knight>::iterator *pItOne, std::vector<Knight>::iterator *pItTwo)
{
	for(*pItOne = pVecOne->begin(); *pItOne != pVecOne->end(); ++*pItOne)
	{
		for(*pItTwo = pVecTwo->begin(); *pItTwo != pVecTwo->end(); ++*pItTwo)
		{
			if(( (*pItOne)->x == (*pItTwo)->x ) && ( (*pItOne)->y == (*pItTwo)->y ))
			{
				while( (*pItOne)->health >= 1 && (*pItTwo)->health >= 1 )
				{
					(*pItOne)->speed = rand() % 10;
					(*pItTwo)->speed = rand() % 10;

					if( (*pItOne)->speed > (*pItTwo)->speed )
					{
						std::cout << "ArmyOne Hits ArmyTwo" << std::endl;
						if((*pItTwo)->armour > 0)
						{
							(*pItTwo)->armour -= (*pItOne)->damage;
						}
						else						
						{
							(*pItTwo)->health -= (*pItOne)->damage;
						}
					}
					else if( (*pItOne)->speed == (*pItTwo)->speed )
					{
						std::cout << "Players Both Miss Each Other" << std::endl;
					}
					else
					{
						std::cout << "ArmyTwo Hits ArmyOne" << std::endl;
						if((*pItOne)->armour > 0)
						{
							(*pItOne)->armour -= (*pItTwo)->damage;
						}
						else						
						{
							(*pItOne)->health -= (*pItTwo)->damage;
						}
					}
				}//End while(hpOne > 0 && hpTwo > 0)
			}//End if(x == x and y == y)

			if((*pItTwo)->health <= 0)
			{
				pVecTwo->erase((*pItTwo));
				break;
			}
		}//End for(pVecTwo = pItTwo)

		if((*pItOne)->health <= 0)
		{
			pVecOne->erase((*pItOne));
			break;
		}
	}//End for(pVecOne = pItOne)
}//End Function 
Last edited on
from your code its seems you know more than me just throwing out a suggestion lol
Ok now I'm confused - this new code I used sometimes works - it'll display the quick fight and delete the element. But sometimes it won't. It'll just give me the press any key - yet for some random ones the fight will happen, the element will delete, and it won't show up after that?
Erasing an element from a vector invalidates all iterators.

This is why it crashes.
So basically I can't do this? Ok so say if I was to still use vectors is there a way to make it possible at all? Or should i rewrite the code and use a list or something? - Linked list, Binary Tree?
You could use a std::list, and then you have to do 3 things:

1) Remove the increment of the iterator in both for() loops;
2) In the cases where you erase an iterator, you must first
get an iterator to the next element, then erase the one
you want to erase, and finally re-assign pItOne or pItTwo
with the temporary iterator;
3) In the cases where you don't erase an iterator, you have
to increment the iterator (since you did 1, nothing will
increment the iterator otherwise).

Couldn't you still do what you said with a Vector though?
I've just worked out if I'm using this code without the erasing an element in the vector.. After a couple of fights or sometimes on the first fight - It'll just shut down anyway and tell me to "Press any key to continue"

So something is wrong with this code in general then?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
void Fight(std::vector<Knight> *pVecOne, std::vector<Knight> *pVecTwo, std::vector<Knight>::iterator *pItOne, std::vector<Knight>::iterator *pItTwo)
{
	for(*pItOne = pVecOne->begin(); *pItOne != pVecOne->end(); ++*pItOne)
	{
		for(*pItTwo = pVecTwo->begin(); *pItTwo != pVecTwo->end(); ++*pItTwo)
		{
			if(( (*pItOne)->x == (*pItTwo)->x ) && ( (*pItOne)->y == (*pItTwo)->y ))
			{
				while( (*pItOne)->health >= 1 && (*pItTwo)->health >= 1 )
				{
					(*pItOne)->speed = rand() % 10;
					(*pItTwo)->speed = rand() % 10;

					if( (*pItOne)->speed > (*pItTwo)->speed )
					{
						std::cout << "ArmyOne Hits ArmyTwo" << std::endl;
						if((*pItTwo)->armour > 0)
						{
							(*pItTwo)->armour -= (*pItOne)->damage;
						}
						else						
						{
							(*pItTwo)->health -= (*pItOne)->damage;
						}
					}
					else if( (*pItOne)->speed == (*pItTwo)->speed )
					{
						std::cout << "Players Both Miss Each Other" << std::endl;
					}
					else
					{
						std::cout << "ArmyTwo Hits ArmyOne" << std::endl;
						if((*pItOne)->armour > 0)
						{
							(*pItOne)->armour -= (*pItTwo)->damage;
						}
						else						
						{
							(*pItOne)->health -= (*pItTwo)->damage;
						}
					}
				}//End while(hpOne > 0 && hpTwo > 0)
			}//End if(x == x and y == y)
		}//End for(pVecTwo = pItTwo)
	}//End for(pVecOne = pItOne)
}//End Function 
Well if anyone knows if it's possible to do it like this exactly how to lay it out - if its possible for someone to give me a small demo of how it works. Please let me know. I'm heading out for a while and will be back soon. Cheers guys!
Ok, well I'm wondering why you are passing pointers to iterators...there is no reason to IMO...but since you said they are requiring you to, I guess you don't have a choice. As for the random exiting error...have you tried stepping through it with a debugger? I don't really see why it would randomly be quitting though...what is the movement/fight calling code?
Didn't change much but thought I'd update it's solved with this code and works beautifully now.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
void Fight(std::vector<Knight> *pVecOne, std::vector<Knight> *pVecTwo, std::vector<Knight>::iterator *pItOne, std::vector<Knight>::iterator *pItTwo)
{
	for(*pItOne = pVecOne->begin(); *pItOne != pVecOne->end(); ++*pItOne)
	{
		for(*pItTwo = pVecTwo->begin(); *pItTwo != pVecTwo->end(); ++*pItTwo)
		{
			if(( (*pItOne)->x == (*pItTwo)->x ) && ( (*pItOne)->y == (*pItTwo)->y ))
			{
				while( (*pItOne)->health >= 1 && (*pItTwo)->health >= 1 )
				{
					(*pItOne)->speed = rand() % 10;
					(*pItTwo)->speed = rand() % 10;

					if( (*pItOne)->speed > (*pItTwo)->speed )
					{
						//std::cout << "ArmyOne Hits ArmyTwo" << std::endl;
						if((*pItTwo)->armour > 0)
						{
							(*pItTwo)->armour -= (*pItOne)->damage;
						}
						else						
						{
							(*pItTwo)->health -= (*pItOne)->damage;
						}
					}
					else if( (*pItOne)->speed == (*pItTwo)->speed )
					{
						//std::cout << "Players Both Miss Each Other" << std::endl;
					}
					else
					{
						//std::cout << "ArmyTwo Hits ArmyOne" << std::endl;
						if((*pItOne)->armour > 0)
						{
							(*pItOne)->armour -= (*pItTwo)->damage;
						}
						else						
						{
							(*pItOne)->health -= (*pItTwo)->damage;
						}
					}
				}//End while(hpOne > 0 && hpTwo > 0)
			}//End if(x == x and y == y)

			if((*pItTwo)->health <= 0)
			{
				pVecTwo->erase((*pItTwo));
				break;
			}
		}//End for(pVecTwo = pItTwo)

		if((*pItOne)->health <= 0)
		{
			pVecOne->erase((*pItOne));
			break;
		}
	}//End for(pVecOne = pItOne)
}//End Function  
Topic archived. No new replies allowed.