Review: Approve

Code LGTM, just a few tiny nits. Not tested.

There were a lot of entries in the diff that don't contain any code changes at 
all - please use a merge tool like Meld to compare to trunk the next time this 
happens. This will save the reviewer a lot of time.

Diff comments:

> === modified file 'src/ai/ai_help_structs.cc'
> --- src/ai/ai_help_structs.cc 2017-11-24 09:19:52 +0000
> +++ src/ai/ai_help_structs.cc 2018-01-06 19:40:55 +0000
> @@ -236,6 +236,7 @@
>       unowned_land_nearby(0),
>       enemy_owned_land_nearby(0U),
>       unowned_buildable_spots_nearby(0U),
> +     unowned_portspace_vicinity_nearby(0U),

"vicinity" means the same as "nearby", so you can just drop it from the 
variable name.

>       nearest_buildable_spot_nearby(0U),
>       near_border(false),
>       unowned_mines_spots_nearby(0),
> 
> === modified file 'src/ai/defaultai.cc'
> --- src/ai/defaultai.cc       2017-12-10 21:19:02 +0000
> +++ src/ai/defaultai.cc       2018-01-06 19:40:55 +0000
> @@ -997,6 +979,19 @@
>               }
>       }
>  
> +     // getting list of all fields nearby port space
> +     // TODO it seems port spaces can change over time so ports_vicinity 
> needs to be refreshed from

TODO(tiborb):

> +     // time to time
> +     for (const Coords& c : map.get_port_spaces()) {
> +             MapRegion<Area<FCoords>> mr(map, 
> Area<FCoords>(map.get_fcoords(c), 3));
> +             do {
> +                     const uint32_t hash = mr.location().hash();
> +                     if (!ports_vicinity.count(hash)) {
> +                             ports_vicinity.insert(hash);
> +                     }
> +             } while (mr.advance(map));
> +     }
> +
>       // printing identified basic buildings if we are in the basic economy 
> mode
>       basic_economy_established = 
> persistent_data->remaining_basic_buildings.empty();
>       if (!basic_economy_established) {
> @@ -2454,10 +2436,21 @@
>  
>                       int32_t prio = 0;  // score of a bulding on a field
>  
> +                     // testing for reserved ports
> +                     if (!bo.is(BuildingAttribute::kPort)) {
> +                             if (bf->portspace_nearby == 
> ExtendedBool::kTrue) {
> +                                     if (num_ports == 0) {
> +                                             continue;
> +                                     }
> +                                     // If we have at least on port, we can 
> perhaps build here something

on -> one .... build something here

> +                                     // but decreasing the score to 
> discourage it
> +                                     prio -= 5 * 
> std::abs(management_data.get_military_number_at(52));
> +                             }
> +                     }
> +
>                       if (bo.type == BuildingObserver::Type::kProductionsite) 
> {
>  
> -                             prio = 
> management_data.neuron_pool[44].get_result_safe(bf->military_score_ / 20) / 5;
> -                             assert(prio >= -20 && prio <= 20);
> +                             prio += 
> management_data.neuron_pool[44].get_result_safe(bf->military_score_ / 20) / 5;
>  
>                               // this can be only a well (as by now)
>                               if (bo.is(BuildingAttribute::kWell)) {


-- 
https://code.launchpad.net/~widelands-dev/widelands/ai_portspaces/+merge/335785
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/ai_portspaces.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to