Remoce units from a group or selection [solved]

[Torrasque]

Ok, it try to remove from the selection all the units that have or don't have a specified id. (for exemple, keep only the peewee)

I try this, but it always crash when it try to remove a unit(idBP is the id of the unit)

Code: Select all

set<CUnit*>::iterator ui;
if(selectedUnits.selectedGroup!=-1){
			for(ui=grouphandler->groups[selectedUnits.selectedGroup]->units.begin();ui!=grouphandler->groups[selectedUnits.selectedGroup]->units.end();++ui){
				if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT])
				{
					selectedUnits.RemoveUnit((*ui));
				}
				if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT])
				{
					selectedUnits.RemoveUnit((*ui));
				}

			}
		} else {
			for(ui=selectedUnits.selectedUnits.begin();ui!=selectedUnits.selectedUnits.end();++ui){
				if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT])
				{
					selectedUnits.RemoveUnit((*ui));
				}
				if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT])
				{
					selectedUnits.RemoveUnit((*ui));
				}

			}
		}

If you have some clue, it can save me some time :)

[jcnossen]

Code: Select all

void CSelectedUnits::RemoveUnit(CUnit* unit)
{
	selectedUnits.erase(unit);
	DeleteDeathDependence(unit);
	selectionChanged=true;
	possibleCommandsChanged=true;
	selectedGroup=-1;
	PUSH_CODE_MODE;
	ENTER_MIXED;
	unit->commandAI->selected=false;
	POP_CODE_MODE;
}

Notice the selectedGroup=-1 and the fact that you're using ui!=grouphandler->groups[selectedUnits.selectedGroup]->units.end()

[Torrasque]

Hum, even if I have 10 units, it will crash at the second one. So I don't think it's a boundarie problem.

Hum, perhaps I do not understand because I do not have really understand how those iterators work...

Do really (*ui) equals the adress of the unit?

[jcnossen]

when it does a RemoveUnit call at some point, this will set selectedUnits.selectedGroup to -1.
So after that, it's checks the condition to continue the loop which is:

Code: Select all

ui!=grouphandler->groups[selectedUnits.selectedGroup]->units.end()

which will be:

Code: Select all

ui!=grouphandler->groups[-1]->units.end()

this condition is probably always true, since there is just some random data at grouphandler->groups[-1]->units.end(), so the loop goes on forever and starts reading and writing invalid data.


this might fix it:

Code: Select all

set<CUnit*>::iterator ui; 
int selGroup = selectedUnits.selectedGroup;
if(selGroup!=-1){ 
         for(ui=grouphandler->groups[selGroup]->units.begin();ui!=grouphandler->groups[selGroup]->units.end();++ui){ 
            if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT]) 
            { 
               selectedUnits.RemoveUnit((*ui)); 
            } 
            if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT]) 
            { 
               selectedUnits.RemoveUnit((*ui)); 
            } 

         } 
      } else { 
         for(ui=selectedUnits.selectedUnits.begin();ui!=selectedUnits.selectedUnits.end();++ui){ 
            if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT]) 
            { 
               selectedUnits.RemoveUnit((*ui)); 
            } 
            if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT]) 
            { 
               selectedUnits.RemoveUnit((*ui)); 
            } 

         } 
      }

iterators in general can be implemented in any way, only when you do *ui, you call the overloaded operator *() which returns the address calculated from some internal structure of the iterator.

cool thing is that you can use any type for an iterator as long as it supports the iterator interface. this means you can even do:

Code: Select all

int array [10] = { some numbers  };

int *p = std::find (array, array + 10, 3);

p now points to the array element equal to 3

[SJ]

I can see several problems in this.
First if you have selected a group all commands all commands are given to the group and trying to remove the unit from selectedunits will do nothing since its still in the group.

Second the stuff that makes it crash is most likely that you remove ui from the set while not updating ui. This means that ui get undefined afterward.

Why do you need to do this , I thought this sort of stuff was already taken care of in mousehandler ?

[Torrasque]

I'm making a windows that show you wich units are selected and how many.

Like :

1 * commander
32 * peewee
3 * vampire
10 * solar plant

And when I click on a name (it's buildpics in reallity) it will take only all these units. If you shift click on a name, it will remove them from the selection.

I have searched for helping functions or looking how selectionKeyHandler work, but I found nothing.

Even if I use only this :

Code: Select all

for(ui=selectedUnits.selectedUnits.begin();ui!=selectedUnits.selectedUnits.end();++ui){
            if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT])
            {
               selectedUnits.RemoveUnit((*ui));
            }
            if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT])
            {
               selectedUnits.RemoveUnit((*ui));
            }

         } 

Don't work, so the problem is not only the "selectedGroup=-1"

Thanks for your answer all.

[SJ]

As I said the thing that makes it crash is that you make ui undefined. Easiest way to go around it is something like

Code: Select all

	set<CUnit*>::iterator oldui;
	for(ui=selectedUnits.selectedUnits.begin();ui!=selectedUnits.selectedUnits.end();){
		if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT]){
			oldui=ui++;
			selectedUnits.RemoveUnit((*oldui));
		} else if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT]) {
			oldui=ui++;
			selectedUnits.RemoveUnit((*oldui));
		} else {
			++ui;
		}
	}

[/code]

[Torrasque]

Ok it work ! Thanks you very much :)

(if someone is intrested, I post the working code)

Code: Select all

 set<CUnit*>::iterator ui;
 set<CUnit*>::iterator oldui;
 int selGroup = selectedUnits.selectedGroup;
 if(selGroup!=-1){			for(ui=grouphandler->groups[selGroup]->units.begin();ui!=grouphandler->groups[selGroup]->units.end();){
				if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT])
				{
					oldui=ui++;
					selectedUnits.RemoveUnit((*oldui));
				} else
				if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT])
				{
					oldui=ui++;
					selectedUnits.RemoveUnit((*oldui));
				} else {
					++ui;
				}
			}
		} else {
			for(ui=selectedUnits.selectedUnits.begin();ui!=selectedUnits.selectedUnits.end();){
				if ((*ui)->unitDef->id != idBP && !keys[VK_SHIFT])
				{
					oldui=ui++;
					selectedUnits.RemoveUnit((*oldui));
				} else
				if ((*ui)->unitDef->id == idBP && keys[VK_SHIFT])
				{
					oldui=ui++;
					selectedUnits.RemoveUnit((*oldui));
				} else {
					++ui;
				}
			}
		}

[Dragon45]

That's actually a pretty useful feature; Rome Total War had something similar and it helped in choosing groups. Heck, even StarCraft had this, IIRC.