Re: [crossfire] 2.0 object-type refactoring
Alex Schultz wrote:
It may be better working on getting all of this infrastructure/callback
methods
in place, and implementing one type of object using them instead of focusing
on
the specific callback method (the web page mentioned doing all the apply
stuff).
Why? That way, if someone does write code for a new object type, all the
pieces are in place so they can use the new method. If only the apply
callback
is implemented, they can use that logic, but are left using old coding
method
for the other areas, that then needs to be converted down the road.
Also, by doing all the different callback methods, it provides better
coverage
of the callback logic, eg, if there are issues with any of the methods, they
will be found sooner vs later.
Well, my original thinking, was that if I were to do it by completing an
object type, instead of a callback type, then it would be somewhat of a
pain to make it redirect to legacy methods for other object types not
yet implemented in the new system. What's your view on that issue?
Some of it depends on how this is done.
If you replaced the specific calls to apply, drop, etc as they are sprinkled
throughout the code, then doing one type of function is easier.
However, what I was thinking is those functions would still exist, but
instead
of the case statements they have now, they would have something like:
if (callback_Exists_for_this_type) do_callback()
else { do switch statement}
In that model, doing a few types at a time is easier to do.
I also think you still want to have common top level/generic functions so
that
you can have some common code.
An example of this (albeit a trivial one) is checking that objects are paid
for before doing an apply. Rather than have that duplicated in every object
type apply function, you have it in the one generic one that calls the specific
ones.
A better case here may be something like examine/describe item, where many
objects may have some specific/unique properties, but many also have common
properties (value, cursed, applied, etc).
Second, if we're going to be writing all new functions, we should use some
other
'best practices'. In particular, for anything that would return a string
value,
they should be modified so that a string pointer (and length of that
pointer) is
passed in, instead of using static return buffers. This will assist if we
ever
thread the server, but it will also help prevent a few bugs (related to
this,
sprintf, strcat, and strcpy should never be used, but rather snprintf,
strncat,
and strncpy instead)
Well, I wouldn't say necessarily writing all new functions: I believe
some existing functions can be tweaked fairly easily to fit into the new
system. However I believe that when moving functions that map well over,
we should hand review them anyways, and perhaps use a checklist of
things such as those issues you mention like avoiding static buffer returns.
I think a lot of existing code can be re-used. I'm not sure how many
functions can be re-used completely.
I suppose the apply code is a reasonable example. For some object types, the
logic has been broken apart into its own function.
But some object types are still handled within a case statement. In those
cases, you'll basically need to write a new function.
But this really needs to be thought at the time of callbacks, as you'll need
to set up the function pointers to consistent.
To be honest, the only case I can think of right now would be the
describe/examine object logic.
Other thought not related to the above:
Given this is a major restructure, would it make sense to collapse/redo some of
the types, particularly the equipable equipment?
With the change of equipment (body info), for the most part, there is not the
need of all the types right now (boots, shield, helmet, etc) Really, the only
types (or subtypes) needed are likely:
WEAPON - some classes or gods are prohibited from using them so need to know
what are weapons and what are not.
BOW - like weapon above, but these also fire other objects
ARMOR - some classes can't use armor, so need this info. helmets, boots,
gloves, shields, etc fall into this.
MISC - anything else equipable that is not in the above that conveys
adjustments
to the players (eg, wands/rods/horns are not here, because they do not provide
ac, protections to the player simply by equipping them). But things like
rings,
amulets, necklaces (some of these don't exist but are being used as examples)
would be in this category
___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire
Re: [crossfire] 2.0 object-type refactoring
Hello. For a while now I've been throwing the idea of separating the server's object-type-specific code by object types. Currently things are rather disorganized with code for how an object type works being spread all around the code. I plan to begin to implement the plan on the following wiki page soon if nobody has any objections: /http://wiki.metalforge.net/doku.php/dev_todo:refactor/ Sounds ok to me. I guess not everything is planned, so there'll probably be adjustments later on, but fine for me :) Also, would be nice for some others to help a bit once it's started :) I'll try to help when i get time. Nicolas ___ crossfire mailing list crossfire@metalforge.org http://mailman.metalforge.org/mailman/listinfo/crossfire
Re: [crossfire] 2.0 object-type refactoring
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.
I also do think that if there are standards in place that all objects should
follow (like not letting unpaid objects be used), that should be enforced, and
not something that is affected by object type. If some objects really should
change the behaviour, I'd be more inclined to say they should have specific
flags for such variations - one reason there are some many object types right
now is that there is some of that 'if type==FOO, I behave 95% like BAR, except
for this one little case'. The problem this leads to is down the road, someone
says 'which do I use, FOO or BAR', where if there was just a flag which
controlled that basic actions, that is clearer.
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.
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?
Other thought not related to the above:
Given this is a major restructure, would it make sense to collapse/redo some
of
the types, particularly the equipable equipment?
IMHO This should be done as well, though one question is: would it be
worth completely redoing the type numbers and converting all of the
maps/arches? Or should we try to keep the numbers the same where
possible, and have the loader change type values that were collapsed to
the new value?
If maps are changing the object types, they are broken. While it is
sometimes
done, changing object types is pretty much never good behaviour, because other
aspects of the object are not updated.
However, for the archetypes, they should be updated with the new numbers - it
shouldn't be hard to do, and is definitely the right approach.
___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire
Re: [crossfire] 2.0 object-type refactoring
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
___
crossfire mailing list
crossfire@metalforge.org
http://mailman.metalforge.org/mailman/listinfo/crossfire
