NPC AI Threat System; removing a threat problem

ID:264860
 
Jan 8 2011, 4:31 am
Code:
mob/npc/var/list/threats
mob/npc/var/mob/target
mob/npc/proc
  add_threat(mob/a)
    if(!src.threats)
      src.threats = new
    else
      if(a in src.threats)
        return 0 //threat already recognized
    src.threats += a
    src.threats[a] = 1000
    return 1 //successfully recognized new threat
  remove_threat(mob/a)
    for(vat/mob/m in src.threats)
      if(m == a) src.threats -= m
    if(!src.threats.len) //error line: "cannot modify null.len"
      src.threats = null
  moving()
    while(src.target)
      if(src.target in oview(src.aggro,src))
        step_to(src,src.target)
        sleep(src.move_delay)
      else
        src.remove_target(src.target)
        if(src.threats) //if there are still threats around
          src.target = src.threats[1]
        else src.spawning() //proc to return to its spawn location


Problem description:
Cannot modify null.len is the runtime error produced when I move outside the NPC's "aggro range" and it tries to remove me as a target. The error line is commented in the above snippets. I do not understand why the list would be null when it is clearly initialized and added to upon the addition of a threat. Maybe it has to do with how I sort the threats list when I add new threats to it? I use merge sort to organize the list from highest to lowest threat.
Jan 8 2011, 5:39 am
In the proc remove_threat, when the threats 'list' is empty, threats is explicitly set to null, and threats.len is now invalid. If remove_threat is called after that, you'll experience the runtime error.
Jan 8 2011, 7:26 am
In response to Lewzer
Ohhh. I was unaware of that. I always thought that a list's len that becomes 0 will just remain at 0; still initialized, but with a len of 0. Okay. Thanks for the info, Lewzer.

I guess the check is unnecessary then, haha.
Jan 8 2011, 8:01 am (Edited on Jan 8 2011, 10:00 am)
In response to Spunky_Girl
Spunky_Girl wrote:
Ohhh. I was unaware of that. I always thought that a list's len that becomes 0 will just remain at 0; still initialized, but with a len of 0. Okay. Thanks for the info, Lewzer.

I guess the check is unnecessary then, haha.

AFAIK, if the length of the list is 0, the list still exists.

But to clarify on what Lewzer was getting at, If you're trying to save room and "green program" setting the list to null will delete it if there are no other references to it. (http://www.byond.com/members/ DreamMakers?command=view_post&post=39699) Therefore after the list is garbage collected (deleted) and you call the remove_threat proc again but it's checking !src.threats.len, but since src.threats is already deleted it's spitting out the runtime.

Therefore, you might want to check to make sure the list still exists if(src.threats && src.threats.len <= 0).
Jan 8 2011, 8:19 am
In response to Spunky_Girl
Please, let me clarify... :)

I meant to say that you are explicitly setting the threats variable to null. When you set it to null, it is not a list anymore, and there is no len property to reference.

Your code, with my comments added:
mob/npc/var/list/threats
mob/npc/var/mob/target
mob/npc/proc
  // ...
  remove_threat(mob/a)
    for(vat/mob/m in src.threats)
      if(m == a) src.threats -= m
    if(!src.threats.len) //error line: "cannot modify null.len"
      src.threats = null
      // threats is no longer a list!
      // threats.len is now invalid.
      //  The next time the if statement runs, there will be an error
  // ...


A suggestion: Instead of setting threats to null and testing for that, keep the variable as a list, and check for threats.len instead.

mob/npc/var/list/threats
mob/npc/var/mob/target

mob/npc/New()
    // Initialize the threat list when the npc is created
    threats = new

mob/npc/proc

  add_threat(mob/a)
    // threats is initialized in npc.New()
    if(a in src.threats)
      return 0 //threat already recognized
    src.threats[a] = 1000
    return 1 //successfully recognized new threat

  remove_threat(mob/a)
    src.threats -= a
    // there is no need to set threats to null anymore
    // check for (0 != threats.len) instead of (null != threats)

  moving()
    while(src.target)
      if(src.target in oview(src.aggro,src))
        step_to(src,src.target)
        sleep(src.move_delay)
      else
        src.remove_target(src.target)
        if(src.threats.len) //if there are still threats around -- now checking threats.len instead of (null == threats)
          src.target = src.threats[1]
        else src.spawning() //proc to return to its spawn location


---
Edit:
Thanks, Kalzar! You beat me to the punch. :)
Jan 8 2011, 8:31 am
In response to Lewzer
I was told specifically to never initialize a list or keep a list initialized when it is not being used. So when the list's len variable is 0, it should be set to null, and then when a value wants to be added to it, it should be initialized if it has not been already. I am more inclined to go with Kalzar's advice and do the added check, which works just fine.
mob/npc/remove_threat(mob/a)
  for(var/mob/m in src.threats)
    if(m == a)
      src.threats -= m
      break
  if(src.threats && !src.threats.len)
    src.threats = null
  return 1
Jan 8 2011, 9:21 am
In response to Spunky_Girl
So-so, personally.

You can push lists up to 16 million of them at runtime, so in itself it's not an issue. The issue however would be lists that keep reference to things and thus stop them being garbage collected.
Jan 8 2011, 9:26 am
Out of curiosity, why are you doing this:
   remove_threat(mob/a)
     for(var/mob/m in src.threats)
       if(m == a)
         src.threats -= m
         break

Shouldn't if(src.threats) src.threats -= m do exactly the same thing?
Jan 8 2011, 9:33 am
In response to Nickr5
I changed it to the loop because I was getting a runtime error saying it was a mismatched type. However, since I fixed the runtime error this thread talks about, this now seems to work, which is what I had originally, before using the loop.
mob/npc/proc/remove_threat(mob/a)
  if(src.threats && (a in src.threats))
    src.threats -= a
    if(src.threats && !src.threats.len)
      src.threats = null
    return 1 //successfully removed threat
  return 0 //no threats to remove or threat not found