Infinite loop problem

ID:263407
 
Dec 12 2006, 6:45 am
Code:
            GM_Kill_All_Monsters()
                set category = "GM"
                var/mon = input("What type?","Kill Monster")as null|anything in monsters
                if(!mon)return
                world<<'hit.wav'
                for(mon in world)
                    if(!mon)break
                    if(mon == src)continue
                    flick("hit",mon)
                    usr<<"<font color=red>[mon] has been defeated by [src]</font>"
                    del mon 
                var/T
                for(T in typesof(/monster)-list(/monster,/monster/Boss))
                    monsters += new T


Problem description:
This turns into an infinite loop, what am I doing wrong?
Dec 12 2006, 9:50 am
It's not an infinite loop, just a huge one. You're looping through every item in the world--not just the kind of monster you're looking for.

The problem is, you start out by treating mon as the type of monster to eliminate. Well and good. But then you use that same var as the loop var in for(). So as soon as it enters the for() loop, whatever was in mon before is wiped out, and instead mon becomes the current item in the loop. It's equivalent to this:

var/a = 2
for(a=1, a<10, ++a)
  world << a


Clearly, a is no longer 2 the moment the loop begins.

What you appear to want to do is loop by type path. There's no way to do that without a predefined type, so you'll have to use istype() or such to check. Therefore:

for(var/mob/M)
  if(istype(M, mon))
    ...
    del(M)


That's basically what you needed to do.

Lummox JR
Dec 12 2006, 9:52 am (Edited on Dec 12 2006, 10:16 am)
In response to Lummox JR
edit: It's not working, the monsters just delete themselves from the list.
GM_Kill_All_Monsters()
                set category = "GM"
                var/mon = input("What type?","Kill Monster")as null|anything in monsters
                if(!mon)return
                world<<'hit.wav'
                for(var/obj/M)
                    if(istype(M, mon))
                        flick("hit",mon)
                        usr<<"<font color=red>[mon] has been defeated by [src]</font>"
                        del mon
                var/T
                for(T in typesof(/monster)-list(/monster,/monster/Boss))
                    monsters += new T


edit: the monsters are actually a datum with an parent_type of obj, would this matter when looping through for?
Dec 12 2006, 12:16 pm
In response to Xx Dark Wizard xX
You'll note I defined M as a mob, not an obj. In your case it should be var/monster/M, since they're all of the /monster type. You're also using the mon var instead of M inside the loop, which will get you absolutely nowhere.

Lummox JR
Dec 12 2006, 12:45 pm
In response to Lummox JR
 
            GM_Kill_All_Monsters()
                set category = "GM"
                var/mon = input("What type?","Kill Monster")as null|anything in monsters
                if(!mon)return
                world<<'hit.wav'
                for(var/monster/M)
                    if(isobj(M))
                        sleep(1)
                        flick("hit",M)
                        usr<<"<font color=red>[M] has been defeated by [src]</font>"
                        del M
                var/T
                for(T in typesof(/monster)-list(/monster,/monster/Boss))
                    monsters += new T

Ok I got it working, one problem I get the message for all monsters, and when I tried to use an istype(M,mon) and a continue, nothing happens. How can I get this to work right?
Dec 12 2006, 12:51 pm
In response to Xx Dark Wizard xX
not sure but wouldn't it be istype(M,/monster)?
Dec 12 2006, 12:52 pm
In response to KirbyAllStar
monsters parent_type = /obj, so it doesent matter, and I prefer isobj.
Dec 13 2006, 12:40 am (Edited on Dec 15 2006, 5:52 am)
In response to Xx Dark Wizard xX
Xx Dark Wizard xX wrote:
monsters parent_type = /obj, so it doesent matter, and I prefer isobj.

Of course it matters, it would return true even if that was just an /obj.
But in the for() loop, you already use the /monster type as a filter, and as you said if thats derived from obj, (and isobj() behaves (it probably even uses) istype()), that will always be true anyway, since you're filtering to loop threw ONLY threw /monster types, and since they're always a child of /obj, checking with isobj() is useless.
So, as said above, you're looping threw all /monster types, and just checking if theyre objs which will always be true, so it loops for all monsters. As Lummox said, use istype on the 'mon' var; however, the system you're using is pretty weird. Looks like the monsters list holds object references, not types; so you'll have to declare the 'mon' var as an object - probably as an [/obj]/monster, and then use it's type var: 
            GM_Kill_All_Monsters()
                set category = "GM"
//declare the 'mon' var as type of /monster
                var/monster/mon = input("What type?","Kill Monster")as null|anything in monsters
                if(!mon)return
                world<<'hit.wav'
                for(var/monster/M)
                    if(!istype(M,mon.type)) continue //checks if it's the same type path as mon's type and if it isn't, skip it
                    sleep(1)
                    flick("hit",M)
                    usr<<"<font color=red>[M] has been defeated by [src]</font>"
                    del M
                var/T
                for(T in typesof(/monster)-list(/monster,/monster/Boss))
                    monsters += new T


If I were you, I'd find a better way to do this; you're keeping a list of created objects when you could do better. You could set the monsters list to an associative list in world/New(); have the index of the item be the NAME ONLY of the monster, and the associated value be the type path: 
var/list/monsters = new
world/New()
    . = ..()
    for(var/type in typesof(/monster) - list(/monster,/monster/Boss))
        var/monster/M = new type() //create a new monster, declare it as one (declaring as just an atom would be enough though)
        monsters[M.name] = type //add it's name to the list, associated with the type
        del M //delete it as we only needed it to get the name


mob/verb/Bla()
    var/choice = input("Choose a monster") as null|anything in monsters
    if(!choice) return
    //wanted type = monsters[choice]
    for(var/monster/M in world)
        if(!istype(M,monsters[choice])) continue
Dec 13 2006, 4:39 am (Edited on Dec 13 2006, 4:45 am)
In response to Kaioken
Hmm.. I can't seem to get it to work, I was studying your example and modified mine, and I got this, I created 8 monsters, used the verb it killed 4, used the verb again it killed 2, then again and it killed the rest. Also it adds them to the end of the list.
            GM_Kill_All_Monsters()
                set category = "GM"
                var/monster/mon = input("What type?","Kill Monster")as null|anything in monsters
                if(!mon)return
                world<<'hit.wav'
                for(var/monster/M in world)
                    if(!istype(M,mon)) continue
                    flick("hit",M)
                    usr<<"<font color=red>[M] has been defeated by [src]</font>"
                    del M
                var/T
                for(T in typesof(/monster)-list(/monster,/monster/Boss))
                    monsters += new T
Dec 14 2006, 7:39 am
In response to Xx Dark Wizard xX
You barely changed anything. Carefully re-read my previous post; it answers your problem...
Dec 14 2006, 7:59 am
In response to Kaioken
I tried your way, and it doesent add monsters to the list.
Dec 14 2006, 8:09 am
In response to Xx Dark Wizard xX
Look, READ my posts, don't skim them. In that prev. post I mentioned 2 methods. One to use your current, weird, system of a list of created object instances. You followed the instructions for this PARTLY; you did declare the 'mon' var as of type /monster, therefore a datum descendant (having the 'type' var), but in the loop you still used the 'mon' value, which is a reference to the monster object and therefore isn't a path, so declaring the 'mon' var correctly was useless, you didn't take advantage of that - you were supposed to use 'mon.type' in the loop...

Second method doesn't store created objects in the list; it stores names associated by the appropriate type path, thereby saving memory and created objects.

All of this information WAS present in the example post, but again, you just skimmed it and decided not to read it all - in which case I can't help you - I offer help, and you refuse to read it entirely... It's clear you know enough in coding to follow this, though it's also, unfortunately, clear that you're frequently just skimming posts, sometimes even lazily posting incorrect or irrelevant code. I don't mean to bash you, but c'mon, if you're offered help - read it. If you're told you missed something in the previous post and you should carefully re-read it, then do so.
Good luck. :)
Dec 14 2006, 8:12 am
In response to Kaioken
I did read it, I read it more than once even, I also tried your code and I didn't get any monsters in the list, nothing happened when I used the verb. This is my latest code.
            GM_Kill_All_Monsters()
                set category = "GM"
                var/monster/mon = input("What type?","Kill Monster")as null|anything in monsters
                if(!mon)return
                world<<'hit.wav'
                for(var/monster/M in world)
                    if(!istype(M,monsters[mon])) continue
                    flick("hit",M)
                    usr<<"<font color=red>[M] has been defeated by [src]</font>"
                    del M
                var/T
                for(T in typesof(/monster)-list(/monster,/monster/Boss))
                    monsters += new T
Dec 14 2006, 8:57 am
In response to Xx Dark Wizard xX
Xx Dark Wizard xX wrote:
This is my latest code.

Now you mixed the 2 methods up! Argh! :X

Please re-read my posts, separate between the 2 methods, only use one...preferably my 2nd, suggested one, unless you really do need the list to have created object instances.
First method, uses the current "format" of the monsters list you have, is to choose a monster item from the list, and use it's 'type' var (you'll have to typecast it though - you have) that all datums have that contains the object's path type.

Second method is to use an associative list automatically generated at world/New() that contains the monster's name as the indexes, and the associated values are the respective type paths.

If you specifically don't understand something, just ask.
Dec 14 2006, 9:11 am
In response to Kaioken
Well I tried your second code and it doesent show up in the list, I don't get a pop-up choice.
Dec 15 2006, 5:58 am
In response to Xx Dark Wizard xX
Xx Dark Wizard xX wrote:
Well I tried your second code and it doesent show up in the list, I don't get a pop-up choice.

I just tested it; it works fine (apart from the fact I missed a ")" in the first for() loop line; edited and fixed my post, but the compiler should of notified you of that anyway). Sorry but you must of screwed something up. >_> Post your code.

BTW, here's the complete code I used to test it. 
monster
    parent_type=/mob
    Your_Typical_Monster
    Dude/name="Mwaha"
    SomeMonster/name="Fire Dragon"
    BigMonster/name="Angry Elephant"
world/New()
    . = ..()
    for(var/type in typesof(/monster) - list(/monster))
        var/monster/M = new type() //create a new monster, declare it as one (declaring as just an atom would be enough though)
        monsters[M.name] = type //add it's name to the list, associated with the type
        del M //delete it as we only needed it to get the name


mob/verb/Bla()
    var/choice = input("Choose a monster") as null|anything in monsters
    if(!choice) return
    usr << "Your choice was: {[choice]}.\nThe type path is: [monsters[choice]]"