[crossfire] Circular lighting & negetive glow_radiuses

[Mark Wedel]

Alex Schultz wrote:
>>
     
     
     >>
     
      common/living.c: Making the glow radius additive is probably a bad 
     
     >>
     
      thing.  A player shouldn't be able to light a few torches and get an 
     
     >>
     
      effect to see everything about them (plus in real life, given the 
     
     >>
     
      (pi)r³ of real lighting, doesn't much up very well.).  Same note 
     
     >>
     
      applies for map.c and server/spell_attack.c
     
     >
     
     
     >
     
     
     >
     
      As I understand the additive lighting does accurately act like in real 
     
     >
     
      life due to to the formulas I used in los.c which is essentially a 
     
     >
     
      condensation of the pythagorean theorem and the inverse square law 
     
     >
     
      (except with different constants to make up for the different units). 
     
     >
     
      Like the old comment said,  2 lights doesn't go twice as far as one, but 
     
     >
     
      the new formulas in los.c do actually account for that and each 
     
     >
     
      successive light is less and less important to the range. However, due 
     
     >
     
      to gameplay balance issues, it may be best to take additive lighting 
     
     >
     
      out. Perhaps additive lighting should happen everywhere except in the 
     
     >
     
      player's inventory, that way the player couldn't just get a ton of 
     
     >
     
      lanterns and use them all at once for such. Also spell effects with 
     
     >
     
      negative glow_radiuses that could be made in the future would work much 
     
     >
     
      better with additive lighting on the ground anyways.
     
     
  Well, that fixes the lantern problem.  But I also wonder how it will work with 
spell effects, like say a firebolt or some others.  While this is one logical 
spell effect, oftentimes, the way merging code and whatever happens, there may 
be in fact be many spell effects objects on the same space (certainly true with 
cone and ball type spells). This change could make those spells very very bright.

  Also, I'm not 100% convinced of your changes.  The various code sets the total 
light level for a space, and the LOS code then figures out effective lighting on 
each space.  The LOS code doesn't know about stacked light effects.

  So this seems to me that if there are 2 brightness 2 objects on the space, for 
all purposes, that is indistinguishable from a 1 brightness 4 object.

  So it seems that either the code would be additive, or objects are effectively 
dimmer than they were before because a more realistic distance calcuation is 
used.  To me, that could create its own problems (I'm thinking that some maps 
were designed with light sources placed at various distances to illuminate the 
right things).  My initial thought when the circular lighting code was put in 
was that it would handle the corners more properly, not necessarily make things 
dimmer.  Although breaking the maps probably isn't that much an issue (biggest 
issue in all this is the rounding errors that are happening)

>
     
     
     >>
     
      common/object.c: op->glow_radius !=0 would be clearer checks that >0 
     
     >>
     
      and <0
     
     >
     
     
     >
     
     
     >
     
      I would have done that in many places, except the old check in many 
     
     >
     
      places was just "if (op->glow_radius)" which would return false upon 
     
     >
     
      glow_radius being null, which as I understand glow_radius != 0 would 
     
     >
     
      return true upon null, which wouldn't be good.
     
     
  if (op->glow_radius) is a zero/nonzero check.  IF glow_radius is non zero, it 
will progress through the code in the if block.  When using pointers, it does 
the check against null and not against 0 (although, on probably most every 
system out there, null is 0).

  So that said, the updated check in common/object.c is still not right - 
checking for non zero against an integer field is not proper code.  The line 
should really just be:

if(QUERY_FLAG(op,FLAG_BLOCKSVIEW)|| (op->glow_radius!=0))
  ...

  so all places where you are comparing glow_radius against NULL are incorrect 
(it is effectively the same as comparing against 0, which you are already 
doing).  However, most compilers will warn against such a structure, because 
NULL is usually defined as (void*)0, so you are comparing an integer against a 
pointer which most compilers don't like.