[CF-Devel] button.c magic mouth suggestions

Tue Sep 10 01:38:00 CDT 2002

Kurt Fitzner wrote:
>

      On 08-Sep-2002 Mark Wedel wrote:

     >

>

      The problem is that it hasn't been done the right way from the beginning. 

     >

      I'm not trying to point any fingers here - the way it is now is a natural

     >

      progression from where it started to where it is.  What I'm saying, though,

     >

      is we can add a gazillion flags for everything.  Letting the flags/attributes

     >

      grow ad-infinitum will only cause more problems than it will solve.  Memory

     >

      wastage is one.  CPU wastage for memcopies is another.  But really, the issue

     >

      becomes planning for the future.  This is a policy decision that will come

     >

      back and bite you in the heiny.  Two years from now when there are a million

     >

      new features and you're looking at bloated objects and an organizational

     >

      nightmare, you'll regret going with 'new feature, new flag' for every little

     >

      type-specific behavior you want to add to the system.  Your rule needs to be

     >

      'if the property of flag is meaningful to the majority of object types then

     >

      give it its own value in the object struct - otherwise, use existing space'.  I

     >

      promise you, anything else will be more of a nightmare than overloading is now.

  I'm not certain that your forecast is correct.  Crossfire has been around for 
10 years or something.  Given that timeframe, the object structure has 
increased, but isn't a nightmare now, and I doubt would be even if the current 
overloading of some values was removed.

  It should be mentioned what I'm mostly against is using fields of the object 
structure to enforce/use some feature which is completely non obvious.  I don't 
for example have a problem with 'sp' being used to denote the spell to cast for 
many objects - sp can at least look like spell.

>

     >

      What I suggest we do that will (I believe) solve both issues is we

     >

      create 32 bits of new flag space in each object, 16 ints, pointers and

     >

      floats (that count was picked out of the air - we can choose more or less, or

     >

      expand later).  We designate these as "type-specific storage" and make nice

     >

      #define's for them like TYPE_FLAG_1, TYPE_INT_1, etc.  We then use

     >

      them for any type-specific storage.  Anything that makes sense only for one

     >

      object type.  The reversal flag on buttons (currently sp = 1), the activate

     >

      only on button press/rel easef or signs (currently sp = 1/-1) etc.  The way we

     >

      make it readable, then, will not be in the map editors though.  I would

     >

      suggest that this is not the appropriate place to hide the complexity.  We

     >

      make it readable by overloading their meaning in loader.l.  We can assign

     >

      "ACTIVATE_ON_PUSH_ONLY" to set TYPE_FLAG_1 (bit 1 of the type-specific

     >

      flag area).  I don't even suggest we make loader.l context-sensitive.  Of

     >

      course, this will make it so that if that is set on the wrong object type,

     >

      then wierdness will result. This is where the map editors would need to show a

     >

      little inteligence, only writing "ACTIVATE_ON_PUSH_ONLY 1" to the map file for

     >

      signs/magic mouths.  If, in the future, there is a lot of problem with the

     >

      wrong flags ending up on the wrong object types, we can start to add

     >

      contextual sensitivuty to the loader.  I don't foresee this as a problem,

     >

      though.

  I'm presuming by the above, access would be like:

  op->int[WAND_NUM_CHARGES]
  op->flag[BUTOTN_ACTIVE_ON_PUSH_ONLY]
  op->ptr[WEAPON_SLAYING]

  or the like?

  That works.  Does make debugging more difficult - right now I find it a pain 
simply to debug the flag values, because I have to do the translation by looking 
at define.h (eg, value 10 in the FLAG tables is ..., so it means this object has 
that set).

  If this happens for a lot of fields, looking at the object structure would be 
pretty difficult.

  I had briefly started such an endeavor before - it was difficult to keep up 
with the changing code.  However, I took a different approach:

  The common elements in the object structure remained as they are now.  Things 
which maybe weren't common to every object, but were to most of them were still 
in the object structure.

  For the other ones, it was done like:

     union {
         Player  *player;
         Stationary_Caster       *caster;
         Monster *monster;
     } cst;

  And extend as needed.  At least in that way, you could look at the type, see 
what type of object it was, and look at the appropriate field in the union.

  However, this approach required a bit more work.  Some obvious problem areas 
in trying to fix this up are fields at are currently overloaded and saved that way.

  For example, you could argue that fixing up exits so that destination doesn't 
use slaying and hp and sp.  But in my system, very hard to do that - if you get 
the slaying field, you would need to know the object type to store it in the 
right place.

  You say you wouldn't worry about that, which certainly makes it easier.  But 
then it seems the problem is that exits would still use something like 
MON_SLAYING and MON_HP and MON_SP.

  The exits are arguably a bad example, as there currently are macros that help 
control that, and slaying, hp, and sp are pretty common values, so should be in 
the top level object.

  However, I think it probably is safe to say that there are some objects out 
there that are overloading values that shouldn't do so.  Its unclear to me how 
you would handle that in your loader, other than use the misnamned fields - in 
purpose of this discussion, buttons currently use the 'sp' field to control 
inversion.  But how to transfer that the BUTTON_INVERSION and not ITEM_SP is tricky.

>

      The good news is this is something we can do today.  It's a couple hours worth

     >

      of work to get the initial support in for it.  Once it's started, we can even

     >

      begin to un-overload the existing stuff into the type-specific areas.  There

     >

      will have to be a few caveats.  I suggest we enforce a documentation rule

     >

      where we have an external document that outlines what type-specific flag, int,

     >

      float, and pointer means in relation to each object type.  I suggest we

     >

      religeously maintain this, and also put copious documentation into the source

     >

      and into loader.l each time we support a new name in it.

  Not sure the right approach on this - as said above, the use of unions or the 
like would certainly make debugging easier than needing to deal with offsets.

  But the other problem is knowing how objects deal with each other and 
forseeing future needs.  And you have cases where objects of different types 
have similar functions, and it may not be clear how the objects should get 
grouped together.  Eg, you could have an object of type X, which has some 
features of A, and some features of B.  A and B are different enough not to use 
the same values.  Do you put object X as yet another class?  Do you try to 
extend A or B for X?

  While I know that predicting the future is difficult, before going down this 
road, I'd like to see what features may be out there to cause the object field 
to bloat to some extent.  Taking a quick look over the TODO list, some entries 
will cause some bloat, but I can't see more than about 100 bytes being added.

  While not a defense, the power of computers has grown much faster than 
crossfires object or other growth.  At current time, I'd much rather trade ease 
of debugging and code readability for a few megabytes of ram and some percentage 
of cpu impact.

  After all, right now, the model for exits (using EXIT_PATH, EXIT_X and EXIT_Y 
macros that refer to the actual values in the object could be used.  The problem 
with this approach is when and object is extended to the extend that using those 
values doesn't work anymore because it makes more sense for the extensions to 
use those values.

  In summary, I'm not sure what should be done.  Eliminating overloaded values 
is something that should be done.  Yet, I don't want it to be harder than it 
currently is to debug the code.  I'm also not sure that redoing the object 
structure following the current method will add that much bloat to the object 
structure.