Replied to diff comments

Diff comments:

> === modified file 'src/economy/roadbase.cc'
> --- src/economy/roadbase.cc   2019-03-12 12:11:23 +0000
> +++ src/economy/roadbase.cc   2019-03-12 12:11:50 +0000
> @@ -69,6 +71,68 @@
>       return *flags_[FlagStart];
>  }
>  
> +// This returns true if and only if this is a road that covers the specified 
> edge and
> +// both triangles adjacent to that edge are unwalkable
> +bool RoadBase::is_bridge(const EditorGameBase& egbase, const FCoords& field, 
> uint8_t dir) const {

This function is currently called only from mark_map() when a new road is 
initialized or its state changes between normal and busy. The result is cached 
in the RoadType by mark_map().

I deliberately don´t recalculate anything on terrain change, because I think it 
looks more natural when a bridge isn´t built/removed immediately when the land 
is flooded/drained.

> +     if (descr().type() != MapObjectType::ROAD) {
> +             // waterways can't be bridges...
> +             return false;
> +     }
> +
> +     const Map& map = egbase.map();
> +
> +     FCoords iterate = map.get_fcoords(path_.get_start());
> +     const Path::StepVector::size_type nr_steps = path_.get_nsteps();
> +     bool found = false;
> +     for (Path::StepVector::size_type i = 0; i <= nr_steps; ++i) {
> +             if (iterate == field) {
> +                     if ((i < nr_steps && path_[i] == dir) || (i > 0 && 
> path_[i - 1] == get_reverse_dir(dir))) {
> +                             found = true;
> +                             break;
> +                     }
> +                     return false;
> +             }
> +             if (i < nr_steps) {
> +                     map.get_neighbour(iterate, path_[i], &iterate);
> +             }
> +     }
> +     if (!found) {
> +             return false;
> +     }
> +
> +     FCoords fr, fd;
> +     switch (dir) {
> +             case WALK_SW:
> +                     fd = field;
> +                     map.get_ln(field, &fr);
> +                     break;
> +             case WALK_SE:
> +                     fd = field;
> +                     fr = field;
> +                     break;
> +             case WALK_NW:
> +                     map.get_tln(field, &fd);
> +                     fr = fd;
> +                     break;
> +             case WALK_NE:
> +                     map.get_trn(field, &fd);
> +                     map.get_tln(field, &fr);
> +                     break;
> +             case WALK_W:
> +                     map.get_tln(field, &fd);
> +                     map.get_ln(field, &fr);
> +                     break;
> +             case WALK_E:
> +                     map.get_trn(field, &fd);
> +                     fr = field;
> +                     break;
> +             default:
> +                     NEVER_HERE();
> +     }
> +     return (egbase.world().terrain_descr(fd.field->terrain_d()).get_is() & 
> TerrainDescription::Is::kUnwalkable) &&
> +                     
> (egbase.world().terrain_descr(fr.field->terrain_r()).get_is() & 
> TerrainDescription::Is::kUnwalkable);
> +}
> +
>  /**
>   * Return the cost of getting from fromflag to the other flag.
>   */
> @@ -107,15 +171,25 @@
>               // mark the road that leads up to this field
>               if (steps > 0) {
>                       const Direction dir = get_reverse_dir(path_[steps - 1]);
> -                     if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> -                             egbase.set_road(curf, dir, type_);
> +                     if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> +                             uint8_t type = type_;
> +                             if (is_bridge(egbase, curf, dir)) {
> +                                     type += RoadType::kBridgeNormal - 
> RoadType::kNormal;

kBusy = kNormal + 1
kBridgeBusy = kBridgeNormal + 1
So this calculation catches both cases.

> +                             }
> +                             egbase.set_road(curf, dir, type);
> +                     }
>               }
>  
>               // mark the road that leads away from this field
>               if (steps < path_.get_nsteps()) {
>                       const Direction dir = path_[steps];
> -                     if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E)
> -                             egbase.set_road(curf, dir, type_);
> +                     if (dir == WALK_SW || dir == WALK_SE || dir == WALK_E) {
> +                             uint8_t type = type_;
> +                             if (is_bridge(egbase, curf, dir)) {
> +                                     type += RoadType::kBridgeNormal - 
> RoadType::kNormal;
> +                             }
> +                             egbase.set_road(curf, dir, type);
> +                     }
>  
>                       map.get_neighbour(curf, dir, &curf);
>               }
> 
> === modified file 'src/logic/map_objects/tribes/road_textures.cc'
> --- src/logic/map_objects/tribes/road_textures.cc     2019-03-12 12:11:23 
> +0000
> +++ src/logic/map_objects/tribes/road_textures.cc     2019-03-12 12:11:50 
> +0000
> @@ -17,10 +17,25 @@
>   *
>   */
>  
> +#include "base/wexception.h"
>  #include "logic/map_objects/tribes/road_textures.h"
>  
>  #include <memory>
>  
> +const Image& RoadTextures::get_texture(
> +             const Widelands::RoadType type, const Widelands::Coords& 
> coords, int direction) const {
> +     switch (type) {
> +             case Widelands::RoadType::kNormal:
> +                     return get_normal_texture(coords, direction);
> +             case Widelands::RoadType::kBusy:
> +                     return get_busy_texture(coords, direction);
> +             case Widelands::RoadType::kWaterway:
> +                     return get_waterway_texture(coords, direction);

This refers to the waterways used by ferries. This convenience function has 
nothing to do with bridges, it´s just a leftover from a previous attempt. I´ll 
revert this.

> +             default:
> +                     NEVER_HERE();
> +     }
> +}
> +
>  const Image& RoadTextures::get_normal_texture(const Widelands::Coords& 
> coords,
>                                                int direction) const {
>       return *normal_textures_.at((coords.x + coords.y + direction) % 
> normal_textures_.size());
> 
> === modified file 'src/wui/interactive_player.cc'
> --- src/wui/interactive_player.cc     2019-03-12 12:11:23 +0000
> +++ src/wui/interactive_player.cc     2019-03-12 12:11:50 +0000
> @@ -353,6 +353,21 @@
>                                       }
>                               }
>                       }
> +                     if (f->road_e == Widelands::RoadType::kBridgeNormal || 
> f->road_e == Widelands::RoadType::kBridgeBusy) {
> +                             dst->blit_animation(f->rendertarget_pixel, 
> scale, f->owner->tribe().bridge_animation(
> +                                             Widelands::WALK_E, f->road_e == 
> Widelands::RoadType::kBridgeBusy),
> +                                             f->vision == 1 ? 0 : gametime, 
> f->owner->get_playercolor());

Okay, will do that

> +                     }
> +                     if (f->road_sw == Widelands::RoadType::kBridgeNormal || 
> f->road_sw == Widelands::RoadType::kBridgeBusy) {
> +                             dst->blit_animation(f->rendertarget_pixel, 
> scale, f->owner->tribe().bridge_animation(
> +                                             Widelands::WALK_SW, f->road_sw 
> == Widelands::RoadType::kBridgeBusy),
> +                                             f->vision == 1 ? 0 : gametime, 
> f->owner->get_playercolor());
> +                     }
> +                     if (f->road_se == Widelands::RoadType::kBridgeNormal || 
> f->road_se == Widelands::RoadType::kBridgeBusy) {
> +                             dst->blit_animation(f->rendertarget_pixel, 
> scale, f->owner->tribe().bridge_animation(
> +                                             Widelands::WALK_SE, f->road_se 
> == Widelands::RoadType::kBridgeBusy),
> +                                             f->vision == 1 ? 0 : gametime, 
> f->owner->get_playercolor());
> +                     }
>  
>                       draw_border_markers(*f, scale, *fields_to_draw, dst);
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/bridges/+merge/364318
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bridges into lp:widelands.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to