[crossfire] 2.0 object-type refactoring

[Mark Wedel]

Alex Schultz wrote:
>>   The other reason I'd like there to be common functions is that I think it will 
>> make debugging a little easier.  With a single function that then makes the 
>> callbacks if necessary, I can easily set a breakpoint in there to catch when any 
>> object of any type does that action.  Otherwise, there isn't any easy way to 
>> catch 'if any object of any type is dropped' type of thing, unless there is a 
>> common function that every object type uses 100% of the time.
>>   
> Well, I was planning on having ob_apply(), ob_drop() functions to run
> the callbacks. I would say that's even easier to do breakpoints and
> debugging with, than leaving things to run a callback directly in many
> places in the code such as the commands files as it would be if we just
> inserted a callback check before the existing case statements and such.
> I do think there should be common functions, but I believe they should
> be in a central file relating to this callback system.

  Ok.  If that's the case, that is fine.

  What it sound liked was that you were going to change it so that where right 
now it does something like apply_special() call, it would instead be replaced 
with callbacks[ob->type](...)

  If all you are changing ins apply_special() to ob_apply() (or similar type 
logic) and ob_apply is then doing the callbacks[ob->type](), that is fine.

  In that mode, I still think it would be easy to do an object type at a time - 
you'd have something like:

  ob_apply()
{
   if (we have callback) do_callback()
  else call_old_apply_function()
}

  type of thing.

  OTOH, all this discussion about intermediary progress may really just be 
overthinking it.  With a concerted effort, it shouldn't take too long to make 
all the changes and then it no longer becomes any issue what things looked like 
while the changes were being made.

> 
>>   But it probably is more stylistically - to me, this seems cleaner:
>>
>> void action()
>> {
>>   some common code
>>   make object callback
>> }
>>
>> vs
>>
>> void action_for_type()
>> {
>>     make_common_call_for_all_types()
>>     do my specific code
>> }
>>
>>   It just seems that if everything is going to call 
>> make_common_call_for_all_types(), why not just call that in the first place?
>>   
> Well, the issue IMHO, is for things such as applying, what if one
> specific type wants to put restrictions on when one can apply an object?
> Due to that, it seems to me that the second option would be a more
> flexible and desirable way to call the common code for all types. Also,
> I believe that such common code should be more specific name than
> "make_common_call_for_all_types()", perhaps more along the lines of
> something like can_apply() and complete_apply(), with can_apply() doing
> common checks on if it can be applied, and complete_apply() doing common
> code about what applying the object does.
> (also, just a note for others reading, for the sake of clarity I'll
> note, in the second case, the action() function becomes simply "void
> action() { make object callback }" and eventually extended to include
> things like integrating into the plugin system.)

  I'd say we are probably talking about two different things.

  If certain object types want to put further restrictions on objects, that is 
completely reasonable.  It would have that logic in its type specific handling 
function, or if several types of objects wanted that logic, would perhaps have 
some common function to handle that.

  To some extent, that is the entire purpose of the type specific code.

  What I'm talking about is common behavior that all objects share, regardless 
of type, and changing that behavior really shouldn't be allowed.

  The previous example I had was about applying unpaid objects.  I can't think 
of any case where that should be allowed, and if there was some specific reason, 
I think a flag on the object should control that, and not object type (as 
presumably, that object would have some behavior like existing objects).  I 
think it is cleaner to have it like:

  ob_apply()
{
   if object_is_unpaid { display message about unpaid objects, return}
}

vs

ob_weapon_apply()
{
    if common_object_apply_check() return
}

  where presumably common_object_apply_check() will print out any messages.
Once again, may not be as big a deal, but it just seems cleaner to me to have 
the upper function do those checks vs the lower level functions having to make 
calls into the same function.

  IMO, the only case it makes sense for the type specific function to make the 
function call for the common logic is if there is no upper level function from 
which to do those checks.

  (another different case here is dropping of damned/cursed objects - regardless 
of type, that shouldn't be allowed save for something special (DM))