Re: [glob2-devel] I'm confused by the code in NewNicowar::control_attacks

[Joe Wells]

"Bradley Arsenault" <address@hidden> writes:

> On 8/10/07, Joe Wells <address@hidden> wrote:
>
>     This time, the guilty code is in NewNicowar::control_attacks.
>     [ ... omitted ... ]
>
>     This code is doing several things that I don't understand.
>
>     The part most directly at fault is that it is assuming the command b =
>     echo.get_building_register().get_building(i) stores a valid building
>     in b.  This time, it does not, and dereferencing the building with
>     b->locked[true] causes a crash.  So it should definitely check that b
>     is valid before using it.
>
> get_building(i) should *always* be valid, this is an echo semantic, as long as
> I is up to date.

gdb says b is NULL after this command, so get_building is clearly
returning NULL.

> Nicowar uses iterating to find buildings. I will investigate this
> further.

>     But anyway, how could this call possibly work?  The number i is an
>     index of an attack flag.  (Right?)  How could this possibly be
>     connected to swimming pools?  And why is code for controlling attack
>     flags concerned with swimming pools at all?
>
>             BuildingSearch bs_pool(echo);
>             bs_pool.add_condition(new 
> SpecificBuildingType(IntBuildingType::SWIMSPEED_BUILDING));
>             int num_pool=bs_pool.count_buildings();
>
>             for(int i=0; i<attack_flags.size(); ++i)
>             {
>                     Building* b = 
> echo.get_building_register().get_building(i);
>                     if((num_pool && b->locked[true]) || (num_pool==0 && 
> b->locked[false]))
>                     {
>                             ManagementOrder* mo_destroy=new 
> DestroyBuilding(i);
>                             echo.add_management_order(mo_destroy);
>                     }
>             }
>     }
>
> Having a swimming pool means the units are trained to swim, its used to check
> whether the building is accessible or not.

First, am I correct in understanding that b is actually supposed to be
an attack flag (not a real building)?

Is it always the case that the building register contains only attack
flags?  Otherwise why should entries 0, 1, 2, etc., be guaranteed to
be only attack flags and not also other buildings that are being
managed?

So b->locked[true] indicates whether the attack flag is accessible by
land *or* water, while b->locked[false] indicates whether it is
accessible by land only?  Right?

> If units can swim, more area is
> accessible. This code should be using can_swim i'll look into that. Also, 
> seems
> like attack_flags is storing an integer to a flag that has been destroyed. All
> flags should cause an event to be triggered that causes the removal of it from
> this list. Somehow, that event isn't firing. I'll look into that to.

For what it is worth, this happened when I had wiped out about 1/3 of
Nicowar's base area.  Would that cause Nicowar to delete one of its
attack flags in a strange way?

> Currently my computer is offline except for my windows one, so I have to wait
> to do the remaining bug fixes on beta 1. They include this nicowar fix, a fix
> to YOG, and a couple others that i'll dig through my emails for.

Okay.

-- 
Joe