Trying to wipe a certain type of Mobile (+crash)

[sahisahi]

Have a item that when player doubleclick it removes guards from a region i made this

 public static void No_Guard()
         {
          
           ArrayList list = new ArrayList();
           
            foreach ( Mobile m in World.Mobiles.Values )
           
        {
                if ( m is BaseCreature )
                {
                    BaseCreature bc = (BaseCreature)m;
                   
                    if ( bc is CDE )
                        list.Add( bc );
                }
            foreach (CDE x in list)
              {
           
                x.Delete();
                 }
            }
         }

And added

No_Guard();

to the ondoubleclick method.


Server crash when i doubleclick the item:

Exception:
System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
at System.Collections.Generic.Dictionary`2.ValueCollection.Enumerator.MoveNext()
at Server.Items.Nogd.No_Guard()
at Server.Items.Nogd.OnDoubleClick(Mobile from)
at Server.Mobile.Use(Item item)
at Server.Engines.XmlSpawner2.XmlAttach.UseReq(NetState state, PacketReader pvSrc)
at Server.Network.MessagePump.HandleReceive(NetState ns)
at Server.Network.MessagePump.Slice()
at Server.Core.Main(String[] args)


Thanks!

 

[Talow]

var del = World.Mobiles.Values.OfType<CDE>();
foreach(var entry in del)
    entry.Delete();

This uses Linq to quickly filter out anything that's not of the type CDE(CDE is assumed to be a class here, based on the class Mobile/BaseCreature). Then you can use the temp list to iterate and delete.

 

[sahisahi]

Thank you Talow i totally forgot about linq


im getting a crash:



Exception:
System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
at System.Collections.Generic.Dictionary`2.ValueCollection.Enumerator.MoveNext()
at System.Linq.Enumerable.<OfTypeIterator>d__a3`1.MoveNext()
at Server.Items.Nogd.No_Guard()
at Server.Items.Nogd.OnDoubleClick(Mobile from)
at Server.Mobile.Use(Item item)
at Server.Engines.XmlSpawner2.XmlAttach.UseReq(NetState state, PacketReader pvSrc)
at Server.Network.MessagePump.HandleReceive(NetState ns)
at Server.Network.MessagePump.Slice()
at Server.Core.Main(String[] args)

EDIT!---------------------------


Fixed the crash changing the code u gave me to:

 var del = new List<Mobile>(World.Mobiles.Values.Where(x=>x is CDE));
foreach(var entry in del)
 entry.Delete();

is it correct?

Thank you

 

[Talow]

Same idea, new cat skinning device. Honestly if I was not drunk last night when posting I would have used where myself

 

[Voxpire]

The Where() didn't fix it, he added the results to a separate List<T>.
If it wasn't in a separate collection, it would have still crashed.

var list = World.Mobiles.Values.OfType<CDE>( ).ToList( ); // Linq has ToList( )

foreach( var o in list )
{
	o.Delete( );
}

// Free memory
list.Clear( );
list.TrimExcess( );

 

[Talow]

Wow hung over I'm wrong too yeah Voxpire's got it correct for you

 

[Crome969]

// Free memory
list.Clear( );
list.TrimExcess( );

The solution is perfect but i wonder why you free the memory here by yourself and not leaves it to the gc?

 

[Voxpire]

The solution is perfect but i wonder why you free the memory here by yourself and not leaves it to the gc?

Clearing the list removes the references to the items it contains - one less thing for GC to worry about, otherwise it would have to wait for the list itself to be disposed before the references to its contents are released.

Calling TrimExcess then sets the capacity of the list to 0 (since it's empty), which discards the internal T[] storage array, meaning again, one less thing for GC to wait for.

That just leaves the list object itself, once its reference is no longer used (by the end of that method), it will be eligible for GC and primed, since it will be empty and have its internal resources released.

I think the GC gives us a false sense of security and persuades us not to clean up after ourselves because it thinks it can do a better job, but the truth is, it's not as great as it thinks it is and needs a nudge in the right direction every now and then.

 

[Crome969]

Clearing the list removes the references to the items it contains - one less thing for GC to worry about, otherwise it would have to wait for the list itself to be disposed before the references to its contents are released.

Calling TrimExcess then sets the capacity of the list to 0 (since it's empty), which discards the internal T[] storage array, meaning again, one less thing for GC to wait for.

That just leaves the list object itself, once its reference is no longer used (by the end of that method), it will be eligible for GC and primed, since it will be empty and have its internal resources released.

I think the GC gives us a false sense of security and persuades us not to clean up after ourselves because it thinks it can do a better job, but the truth is, it's not as great as it thinks it is and needs a nudge in the right direction every now and then.

Thx for your statement! My thoughts were similar about that, because iam used to clean up my own mess thanks to delphi but thought it might be interesting for a few people who not knows what gc is or why you went this way.