[crossfire] 2.0 object-type refactoring

[Alex Schultz]

Mark Wedel wrote:
> Alex Schultz wrote:
>
>   
>> Well, the way I was thinking of doing it, would be to replace specific
>> calls to apply, drop, etc as they are sprinkled throughout the code, and
>> keep common code such as you say, in a "common" subdirectory of the
>> types directory, and then have the callback called by ob_apply(), call
>> something like ob_apply_common() at the start of it's code.
>> IMHO, with this method, the best way to handle legacy non-separated
>> logic would be to move that legacy logic to the "types/common"
>> subdirectory, and set it up so ob_apply() and such will call that if it
>> can't find a callback registered for the type.
>>     
>
>   That works I suppose.  You do get into various questions that have no clear 
> answer.  Like 'should the main drop function be in the commands.c file (since it 
> is a command the player issues) or should it be in the types/common.c file'
>
>   I'd be somewhat inclined to have the command logic still in the command 
> (c_...) files, and from there, it does the callback.  It seems (to me at least) 
> that is a little better - the basic command logic is still in the commands file, 
> and the actual object specific drop is in the types area.  But I can also see 
> the other point - lets put all the common object operations together.
>   
To me it also seems best to keep command logic in the command files, and
that does the callback. However IMHO only bits about decoding the
command, and providing certain types of player feedback, should be
handled in the command files. That way, it is easy to make code that
does an action that is (currently) normally a player action, with code
common to it, and without giving feedback to the player that isn't valid
considering the player didn't make that action.
Basically, I believe that player-actions should keep their
command-decoding, player-feedback, and other code that should only
happen when a player is triggering the action directly, in the command
code, but delegate all action code in the callback system.
Also, along the same sort of lines, unpaid checks should for the most
part be done in the command code as opposed to the actual action code.

> <snip>
>   
>   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.

>   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.)

Alex Schultz