User Tools

Site Tools


dev_todo:refactor

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

dev_todo:refactor [2006/10/28 12:00]
dev_todo:refactor [2006/11/14 13:30] (current)
Line 1: Line 1:
 +====== Object-type refactoring ======
 +===== Reason =====
 +Currently in the crossfire code, how objects behave is intertwined all throughout the code, and this is in many ways a problem as it makes it difficult to find code related to a type of object. It's current state also makes it relatively difficult to add code for new object types. Though it will never be that easy to add object types, refactoring could improve this significantly. Also, refactoring this to separate code for different object types, will make it easier to find current overloading of object attributes, making it easier to eventually refactor the overloading out.
 +
 +===== Proposal =====
 +==== General Idea ====
 +  * Allow operations such as '​apply'​ to be specified in pointers per object type number, however only the last being used in the refactoring
 +  * Separate object type specific code into separate files.
 +  * Make sure it is easy to a later stage implement per-object and per-archetype hooks.
 +
 +==== Implementation Details ====
 +=== Code organization ===
 +  * Use form of types/foo.c or types/​foo/​bar.c in the server tree, depending on if the object type requires multiple C files to be clean.
 +  * Code used by multiple distinct types, but is not generic to all object types should be put in types/​common/​ with a logical filename.
 +  * Doesn'​t necessarily mean one type number per file, due to cases such as different types of armor, where all behave the same but have different type numbers. Just make sure it is logical.
 +=== Function pointers ===
 +  * Function pointers to the '​object methods'​ will be stored in a struct in along the lines of:<​code>​typedef struct ob_methods {
 +    ob_methods *fallback;
 +    int (*apply)(ob_methods *context, object *ob, object *pl);
 +    int (*drop)(ob_methods *context, object *ob);
 +    ...
 +} ob_methods;</​code>​For per-type hooks, they would be stored in an array of "​ob_methods type_methods[MAXTYPE];"​. Function pointers will be NULL if unset.
 +For per-object hooks __if/when__ implemented,​ objects would have a struct member added as "//​ob_methods *methods;//"​ (accessed as "//​ob->​methods//"​). Per-arch hooks if/when implemented would be done as a similar struct member added to the archetype struct. In those cases, the "​methods"​ pointer will be NULL unless there are methods in it.
 +  * One would not access function pointers directly, instead one would use functions to add hooks, and functions in the convention of "​ob_apply()"​ and "​ob_drop()"​ to run them.
 +  * Each file would register whatever hooks it wanted in an init function for that specific file.
 +    * Will have to put an "​init_ob_types()"​ function in the main code, containing calls to all type_specific init functions.
 +  * The "​fallback"​ is a pointer to another ob_methods structure that is used if the current ob_methods structure doesn'​t have a callback for the event.
 +    * All types will normally fall back to a "​base_type"​ structure, containing default actions for all object types (i.e. for things like dropping and picking up)
 +    * The "​base_type"​ structure will fall back to a "​legacy_type"​ structure, which contains pointers to pre-refactor legacy functions. When the refactoring is complete, the "​legacy_type"​ structure will be removed.
 +  * The "​context"​ paramater passed to callbacks, is a pointer to the ob_methods structure that it was called from. This is to allow callbacks to call their fallback.
 +  * As a convention, all callbacks will have their first two parameters as "​ob_methods *context, object *ob".
 +  * Will take some forethought to make sure that one provides good enough callback parameters for all uses of a hook.
 +  * Checks such as checking unpaid status should be done from callback functions. If the code for any such common checks is more than one or two lines, it should have a function in types/​common.
 +    * Should the smallest of checks such as unpaid status ones have macros that include the return statement?
 +
 +===== Plan =====
 +==== To start ====
 +  * Begin refactoring some things to the new system.
 +  * Start with moving all events for a single type at a time.
 +    * When making a new function pointer, move legacy code for that function pointer'​s action, that you arn't refactoring yet, into "​types/​legacy/​*.c"​ or at least call it from a callback function defined in "​types/​legacy/​*.c"​. Remember to set the "​legacy_type"​ structure to point to it.
 +  * Aim for ~90% of type-specific code moved into the new system.
 +
 +==== Policy ====
 +  * Commit refactoring changes frequently in very small chunks. As small chunks as possible while keeping the SVN code working is preferable.
 +  * Try to give a bit of notice before commiting refactoring changes, noting specifically what refactoring changes you intend to commit. The type/​ChangeLog message is a clear format for telling others what you're refactoring from where to where.
 +  * Document what you refactor, when you refactor it, and it's form in the system. Documentation including:
 +    * In addition to updating the ChangeLog, make a more detailed entry in types/​ChangeLog,​ in a format following the conventions listed there.
 +
 +==== Later ====
 +  * Clean up the refactoring.
 +  * Attempt to get the remaining type-specific code moved into the system.
 +  * Collapse some groups of types - there are a bunch of different armor types, but for logical purposes, they are the same, just go on different body parts.
 +==== Eventually... ====
 +  * Implement some sort of [[dev_todo:​unified_event_system|unified event system]] and integrate with this object-type separation. Not a priority right now, but something to keep in mind for later.
 +
 +===== More information =====
 +  * [[http://​thread.gmane.org/​gmane.games.crossfire.general/​1928|Crossfire mailing list: Code restructuring]]
 +  * [[http://​thread.gmane.org/​gmane.games.crossfire.general/​2412|Crossfire mailing list: 2.0 object-type refactoring]]