Author Topic: AddEnemy query versus member_array  (Read 3143 times)

Offline detah

  • BFF
  • ***
  • Posts: 190
  • Ruler of 2D
    • View Profile
AddEnemy query versus member_array
« on: September 21, 2007, 08:43:58 AM »
This question regards some combat code in Dead Souls 2.5a12.
But the real question is about good old fashioned LPC, so I put it in this section of the forum, as any LPC coder might know this.

The goal is to check if the array, Enemies, contains a particular object called target.

The current code has if(AddEnemy(target)). AddEnemy returns 1 when the obj is added to the array Enemies. Specifically

AddEnemy looks like this:
Code: [Select]
int AddEnemy(object ob) {
    if( !ob || (member_array(ob, Enemies) != -1) ) {
return 0;
    }
    if( !living(ob) ) {
return 0;
    }
    Enemies += ({ ob });
    return 1;
}

So when Enemies += ({ ob }) it returns 1.

This seems this seems like a very convoluted way to do the standard
if(member_array(ob, Enemies)!=-1)

Can anyone think of a reason why the former is prefered to the latter?

Detah@Arcania

Offline Jimorie

  • Friend
  • **
  • Posts: 58
    • View Profile
Re: AddEnemy query versus member_array
« Reply #1 on: September 21, 2007, 09:16:48 AM »
I don't know, but could the reason perhaps be that the goal is not to check whether the array Enemies contains the particular object, but rather to add it? Seems far-fetched, but who knows!

 :P

Otherwise, the code is not just convoluted but plain wrong.

Offline cratylus

  • Your favorite and best
  • Administrator
  • ***
  • Posts: 1020
  • Cratylus@Dead Souls <ds> np
    • View Profile
    • About Cratylus
Re: AddEnemy query versus member_array
« Reply #2 on: September 21, 2007, 09:53:45 AM »
Quote
Can anyone think of a reason why the former is prefered to the latter?

This is partly a code aesthetics question, and I'm loath to
dip into such things because one will always be right to some degree,
and wrong to some degree, in such matters.

However, the question is based on incorrect premises, and
that much I can help clarify.

Quote
So when Enemies += ({ ob }) it returns 1.

This will seem like pedantic quibbling, but it really is not
meant to be. Your statement makes it sound like returning
1 is dependent on the array element addition. You probably did
not mean to make it sound that way, but it's important to clarify
that the two statements are sequential but not dependent.

Though it's hard to imagine a circumstance where the two
do not occur given the preceding statements, I felt it was important
to clarify this. A clearer statement would be:

So after Enemies += ({ ob }) it returns 1.

Quote
This seems this seems like a very convoluted way to do the standard
if(member_array(ob, Enemies)!=-1)

The reason it seems convoluted is that it isn't just
doing that. It is also checking whether it has
received a valid object. If there is no object, or if
the object is not a living thing, then it will return 0.

While you can argue the aesthetics of how this object
checking occurs, based on my experience with LPC
and LPC coders, not having such a check will cause
more problems than having it.

-Crat

Offline Nulvect

  • BFF
  • ***
  • Posts: 127
    • View Profile
Re: AddEnemy query versus member_array
« Reply #3 on: September 21, 2007, 03:49:26 PM »
AddEnemy here has one main purpose: to add a living object to your list of enemies.

I mentally refer to functions like this as "action" functions, or "do" functions, because they are supposed to do something. "Action" functions don't need to return anything.

Some functions are "query", or "info" functions, that are supposed to simply give you some piece of information. "Query" functions normally must use return to give you that info.

However, it saves code if you can simultaneously try to do something and check for success/failure all at once. The code that uses if (AddEnemy(target)) is doing that. It means, "Try to add target as an enemy. If this succeeds, then..."

There is probably a GetEnemies function and maybe an IsEnemy function (I haven't looked), and it could have alternatively been written as AddEnemy(target); if (IsEnemy(target)) or AddEnemy(target); if (member_array(target, GetEnemies()) != -1), but, in my opinion, it's cleaner and easier to read the way it is now. Of minor consequence is that either of these alternatives would actually do an extra call to member_array.

Offline detah

  • BFF
  • ***
  • Posts: 190
  • Ruler of 2D
    • View Profile
Re: AddEnemy query versus member_array
« Reply #4 on: September 21, 2007, 04:12:54 PM »
I do not object to the language in AddEnemy. The purpose of my question lies with how if(AddEnemy(target) is used in the SetAttack function (also in combat.c).

I should have made it more clear that I want to understand combat.c SetAttack function L259:

while(i--) if( AddEnemy(target) ) AddHostile(target);

You can see here that they are using the AddEnemy() function in this if conditional.
I thought this was confusing because instead of just doing the plain old member_array(), the coder chose to use the AddEnemy function which adds a new monster to the Enemies array. Its wierd (to me) that you should use a condition to actually 'do' something.

In my mind, this whole section in SetAttack should go like this:

Player enters room with a new monster and types 'kill orc'.
1) Add the new monster to Enemies (using the appropriate function, which is AddEnemy). If the monster is already in Enemies at this point, AddEnemy should recognize this and pass along the pointer. I think AddEnemy does a good job of doing this as written. Like I said, I have no problem with AddEnemy.
2) Do an if check to make sure that the new monster is in Enemies, using member_array(). This is a bit of overkill, but it may prevents errors, so I can understand putting in the if check instead of doing a plain old:

AddHostile(target);

So using the full if statement, I think 2) should look like:

if(member_array(target, Enemies)) AddHostile(target);

As far as I can tell, this section of the code is where combat actually starts. So this is an extremely important piece of code to understand. Now the next item in the combat flowchart starts the eventExecuteAttack function. Voila! Combat begins.

The idea of using an if statement to do the action of adding an enemy to the Enemies list seems odd to me. Ive always thought of if statements as checks on variables, nothing more. I assure you, in the new combat I am writing this will be much clearer in the code. And more importantly, IT WILL BE WELL-COMMENTED. [no offense, C]

Detah@Arcania

Offline Nulvect

  • BFF
  • ***
  • Posts: 127
    • View Profile
Re: AddEnemy query versus member_array
« Reply #5 on: September 21, 2007, 04:47:20 PM »
The point is that AddEnemy tells you whether it succeeds in adding a new enemy or not. If AddEnemy(target) returns 1, then target will always be in the Enemies array. Note that target might be in Enemies even if AddEnemy returns 0, because it won't add the same object twice. I'd revise the purpose of AddEnemy to be: add a new enemy to my list of enemies, and tell me if it succeeded. The way it's written, the code assumes that if someone is already in your enemies list, you do not need to do AddHostile() for that person again.

You don't need to do if (member_array(target,Enemies) != -1) after you do AddEnemy(target), since Enemies += ({ target }) can't really fail (besides variable type issues, but then it wouldn't run anyway). The if (AddEnemy(target)) is checking the return value of AddEnemy, which indicates success (1) or failure (0) in adding the new enemy. It takes a little getting used to, but it makes things look much nicer and it kind of forces you to put error checking into your action functions, which, in my opinion, is a good thing.

Offline cratylus

  • Your favorite and best
  • Administrator
  • ***
  • Posts: 1020
  • Cratylus@Dead Souls <ds> np
    • View Profile
    • About Cratylus
Re: AddEnemy query versus member_array
« Reply #6 on: September 21, 2007, 05:14:33 PM »
Quote
I do not object to the language in AddEnemy. The purpose of my question lies with how if(AddEnemy(target) is used in the SetAttack function (also in combat.c).

I should have made it more clear that I want to understand combat.c SetAttack function L259:

while(i--) if( AddEnemy(target) ) AddHostile(target);

Ah! That question makes sense.

Yes, what Nulvect said. It's not as inelegant as it looks at first blush.

-Crat