Design Choice: Which way would you do it?

ID:151662
 
Sep 25 2009, 2:14 pm
Would you set a proc that handles interaction between an item you hold in your hand and a target item on the former or latter?

Example: Former (Proc stored on item held)
obj/proc/Interact(atom/Target,mob/M)

obj/Pickaxe/Interact(atom/Target,mob/M)
 if(istype(Target,/obj/Rock))
  world<<"You macked a rock with a pickaxe!"
 else if(istype(Target,/obj/Wall))
  world<<"You macked a wall with a pickaxe!"


Example: Latter (Proc stored on target)
atom/proc/Interact(obj/Source,mob/M)
obj/Rock/Interact(obj/Source,mob/M)
 if(istype(Source,/obj/Pickaxe))
  world<<"You macked a rock with a pickaxe!"

obj/Wall/Interact(obj/Source,mob/M)
 if(istype(Source,/obj/Pickaxe))
  world<<"You macked a wall with a pickaxe!"


Which one would you feel is more effective, and why?

Just to clarify: When I say effective I mean effective in the sense that is it easier to organize in a large scale.
Sep 25 2009, 2:52 pm
As far as sense goes, hands down the proc should be defined on the action initiater (pick-axe), considering it's what is doing the action, the interaction, not the target (wall). You could always have both kind of procs, though, which can be useful in a complex system. One well-known example is when you use both Bump() and Bumped() - both are for the same identical event, but are just defined on the other side for useful object orientism.
Sep 25 2009, 3:36 pm
In response to Kaioken
Kaioken wrote:
As far as sense goes, hands down the proc should be defined on the action initiater (pick-axe), considering it's what is doing the action, the interaction, not the target (wall). You could always have both kind of procs, though, which can be useful in a complex system. One well-known example is when you use both Bump() and Bumped() - both are for the same identical event, but are just defined on the other side for useful object orientism.


I was asking this question out of curiosity, trying to gain insight into other programmers mind.

The way I approached this was using the target.
The logic I used here was the same as why I use Bumped() and not Bump(). It seems (to me) infinitely easier to create a new object, then create the interactions for it, rather than create a new object and add another if(istype()) situation to an already large list.
Sep 25 2009, 6:43 pm (Edited on Sep 25 2009, 11:54 pm)
This is tough to answer for the simple reason that my answer isn't so black and white.

Simply put: objects should have specific and well-defined actions defined for themselves and other objects should call into those methods. For example, a gate which opens and closes should have specific toggle()/state(), open()/close(), or raise()/lower() procs, and objects such as switches and levers should call into those. The switches/levers should not handle the actual raising or lowering of the gate; that is, they shouldn't be updating the gate's icon or changing its density or whatever.

Another take on the above could happen through specific /interaction datums. Any action that initiates an action on another object could populate an instance of this interaction datum and send it to the action's recipient, which decides to either handle or discard it based on its properties. This is similar to how, say, the Windows API works: you define a WndProc for your window which receives messages (such as those telling you the window was clicked, created, etc), and your WndProc specifically handles those. Consider the following take on this:
interaction
   blade_strike   // for descendants of /obj/blade
      var/obj/blade/blade

      New(blade)
         ASSERT(blade)
         src.blade = blade

atom/proc/handle(interaction/i)   // handle globally-common interactions
   switch(i.type)
      if(/interaction/blade_strike)
         var/interaction/blade_strike/b = i
         b.blade.owner << "You hit [src] with \an [b.blade]!"

obj/blade
   var/mob/owner
   
   New(loc, owner)
      ..(loc)
      src.owner = owner

   proc/interact(atom/target)
      // the output could happen here instead
      target.handle( new/interaction/blade_strike(src) )
   
   pickaxe
   sword

obj/wall/handle(interaction/i)
   ..(i)   // default handle() proc outputs a message for blade strikes

   switch(i.type)
      if(/interaction/blade_strike)
         var/interaction/blade_strike/b = i
         
         world << "\The [src] has been damaged by \the [b.blade]!"

         // stuff that happens upon being hit (damage, etc)

obj/invulnerable_wall   // does not respond to blade_strike

// etc.

/*
    Testing:
*/

mob/verb/Test()
   var/obj/blade/sword/s = new(null, usr)   // loc=null, owner=usr
   var/obj/blade/pickaxe/p = new(null, usr)
   
   var/obj/wall/w = new
   var/obj/invulnerable_wall/iw = new
   
   p.interact(w)
   p.interact(iw)
   
   usr << "<hr>"
   
   s.interact(w)
   s.interact(iw)


The default handle() proc would handle interactions that remain the same for every object; in this case it handles the outputting of the message to the player. Likely, a better place for this specific thing (outputting that message) would be in the pickaxe's interact() proc, but I wanted to highlight the handling of interactions common to all objects.

So now you can interact your pickaxe with anything and you'll get a message back, but if you interact with an /obj/wall you'll get the message as well as stuff that happens upon being hit (damage, etc).

Edit: Fleshed out the snippet to make it into a small demo.
Sep 25 2009, 9:58 pm (Edited on Sep 25 2009, 10:17 pm)
As Kuraudo has already said, neither choice offered in the original post is acceptable. All objects should define their own behavior. So if a pickaxe can hit an object (regardless of how that object was chosen) then the pickaxe will define how it goes about hitting that object. If the object being hit needs to define how it reacts to getting hit (if it reacts at all), then it needs to define its own reaction. Consider this example:

pickaxe
    parent_type = /tool
    proc/pick_at(var/pickable/target, var/player/picker)
        if(!istype(target))
            return // if we picking at a player, or something
        var/resource/result = target.pick(src, picker)
        picker.show_text("You receive [result.amount] [result.material].")

resource
    var
        material
        amount
    New(var/_material, var/_amount=1)
        . = ..()
        material = _material
        amount   = _amount

pickable
    parent_type = /turf // or /obj, whatever your case is
    var
        material
        amount = 1
        amount_per_pick = 1
    proc/pick(var/pickaxe/tool, var/player/picker)
        var/return_amount = min(amount_per_pick, amount)
        var/resource/result = new(material, return_amount)
        amount -= return_amount
        return result
    maple_tree
        icon = 'tree.dmi'
        icon_state = "maple"
        material = MATERIAL_WOOD
        amount = 10
        amount_per_pick = 3
    iron_vein
        icon = 'mines.dmi'
        icon_state = "iron_ore"
        material = MATERIAL_IRON_ORE
        amount = 15
        pick(var/pickaxe/tool, var/player/picker)
            if(rand()*16 > 15) // 1 in 16 chance
                tool.break()   // of the tool breaking
            if(rand()*4 > 1) // 3 in 4 chance
                return new /resource(MATERIAL_STONE, 1)
            . = ..()


Notice how every object defines its own behavior. Notice how the objects communicate with each other by calling procs. Notice how whenever a proc is called it is given all the information it may need to know. For instance, it may look pointless to tell the pickable object who is picking at it, or what tool that person is picking with, but it comes in handy later on when we want iron_ore to have a change of breaking the tool.

Another very important thing to take note of is how every proc makes good use of return values. For instance, we don't call target.pick() and then manually check the different between how much resources it has now and how much it used to have, and we don't manually check what sort of resource it gives out. Instead, we pick at it and then read the result of that action. This is important because the vein of iron doesn't always return the same resource as its material; in fact, three out of four times the picker gets stone instead.

All of this is custom behavior which makes your game fun, robust, and feature rich. When each object defines its own behavior, and each proc is given all the info available, the result is new features.



Sometimes it's not possible or preferable to say that only pickaxes can pick at certain materials, and sometimes not all pickable materials derive from the same type (some could be turfs while others are objs, mobs, or even non-atomic objects). In these cases, we can't use a simple istype() check. Kuraudo's suggestion deals with this situation by giving all objects a generic proc which accepts all sorts of messages, and simply does nothing if it's not "watching" that message. That is a very powerful approach, and there are many instances where it is appropriate.

There's another approach that DM makes available, though to be honest I prefer either of the two options I mentioned above. Instead of checking istype() to make sure that the object has that function, we can check hascall(target,"pick"). If that returns true, then we can use call(target,"pick")(src,picker). This is not as robust as sending messages via a common proc, though, because each time pick() is added to an object, we have to make sure that it accepts the same arguments in the same order as all other versions of pick() on different objects... and that's saying nothing about what happens if you update the process to use more variables or change a type path somewhere; you have to update all of the procs all over again. Using named arguments could mitigate some of these issues.

Technically, both the hascall/call method and Kuraudo's suggestion are the same thing, and both require the same amount of work to maintain. The difference is only in that hascall/call gives you a false sense of security because DM is checking things for us. It's a false sense of security because we could have defined pick() wrong on the target object, or perhaps that pick() isn't even meant to work with pickaxes, but was defined when we were picking the next player from a queue, or whatever.
Sep 25 2009, 10:18 pm
Thank you all for your input. It was enlightening. :P
Sep 25 2009, 11:39 pm
In response to IainPeregrine
All very good information, I just wanted to point out one tiny word of caution.
> pickable
>     /* ... */
>     proc/pick(var/pickaxe/tool, var/player/picker)
>         var/return_amount = min(amount_per_pick, amount)
>         var/resource/result = new(material, return_amount)
>         amount -= return_amount
>         return result

Be careful of naming with reserved words like "pick" which correspond to, say, the built-in pick() proc. ;)