[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-863185-census-on-destroyed-building into lp:widelands
Continuous integration builds have changed state: Travis build 1619. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/176633949. Appveyor build 1457. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_863185_census_on_destroyed_building-1457. -- https://code.launchpad.net/~widelands-dev/widelands/bug-863185-census-on-destroyed-building/+merge/309818 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-863185-census-on-destroyed-building. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-863185-census-on-destroyed-building into lp:widelands
Bunnybot encountered an error while working on this merge proposal:
('The read operation timed out',)
--
https://code.launchpad.net/~widelands-dev/widelands/bug-863185-census-on-destroyed-building/+merge/309818
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-863185-census-on-destroyed-building.
___
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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/animation_scaling into lp:widelands
Review: Approve
some minor nits in the code.
This is way better encapsulated than before, but it still does not make me all
warm and fuzzy inside - it feels weird that the animation passes out its
calculated source rect, just to get it back in the blitting call. I do not have
any more immediate suggestions how to make this better though.
Diff comments:
>
> === modified file 'src/graphic/animation.cc'
> --- src/graphic/animation.cc 2016-10-26 10:56:02 +
> +++ src/graphic/animation.cc 2016-11-18 17:52:33 +
> @@ -256,20 +257,35 @@
> }
> }
>
> +Rectf NonPackedAnimation::source_rectangle(int percent_from_bottom) const {
const int per...
> + ensure_graphics_are_loaded();
> + float height = percent_from_bottom * frames_[0]->height() / 100;
> + return Rectf(0.f, frames_[0]->height() - height, frames_[0]->width(),
> height);
> +}
> +
> +Rectf NonPackedAnimation::destination_rectangle(const Vector2f& position,
> +const Rectf& source_rect,
> +float scale) const {
const float
> + ensure_graphics_are_loaded();
> + return Rectf(position.x - (hotspot_.x - source_rect.x / scale_) * scale,
> + position.y - (hotspot_.y - source_rect.y / scale_) * scale,
> + source_rect.w * scale / scale_, source_rect.h * scale /
> scale_);
> +}
> +
> void NonPackedAnimation::blit(uint32_t time,
> - const Rectf& dstrc,
> - const Rectf& srcrc,
> + const Rectf& source_rect,
> + const Rectf& destination_rect,
>const RGBColor* clr,
>Surface* target) const {
> + ensure_graphics_are_loaded();
> assert(target);
> -
> const uint32_t idx = current_frame(time);
> assert(idx < nr_frames());
> -
> if (!hasplrclrs_ || clr == nullptr) {
> - target->blit(dstrc, *frames_.at(idx), srcrc, 1.,
> BlendMode::UseAlpha);
> + target->blit(destination_rect, *frames_.at(idx), source_rect,
> 1., BlendMode::UseAlpha);
> } else {
> - target->blit_blended(dstrc, *frames_.at(idx),
> *pcmasks_.at(idx), srcrc, *clr);
> + target->blit_blended(
> +destination_rect, *frames_.at(idx), *pcmasks_.at(idx),
> source_rect, *clr);
> }
> }
>
>
> === modified file 'src/graphic/animation.h'
> --- src/graphic/animation.h 2016-10-25 08:14:28 +
> +++ src/graphic/animation.h 2016-11-18 17:52:33 +
> @@ -57,8 +58,17 @@
> }
>
> /// The dimensions of this animation.
Fix comment.
> - virtual uint16_t width() const = 0;
> - virtual uint16_t height() const = 0;
> + virtual float height() const = 0;
I am not entirely sure, but I *think* this can be in a protected section now.
> +
> + /// The size of the animation source images. Use 'percent_from_bottom'
> to crop the animation.
Units?
> + virtual Rectf source_rectangle(int percent_from_bottom) const = 0;
> +
> + /// Calculates the destination rectangle for blitting the animation.
> + /// 'position' is where the top left corner of the animation will end
> up,
> + /// 'source_rect' is the rectangle calculated by source_rectangle,
> + /// 'scale' is the zoom scale.
> + virtual Rectf
> + destination_rectangle(const Vector2f& position, const Rectf&
> source_rect, float scale) const = 0;
>
> /// The number of animation frames of this animation.
> virtual uint16_t nr_frames() const = 0;
--
https://code.launchpad.net/~widelands-dev/widelands/animation_scaling/+merge/310718
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/animation_scaling.
___
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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/toolbar_cleanup into lp:widelands
Review: Approve just drive by to say thanks to notabilis to also pick up some reviews! that is really awesome and shows great citizenship. This is a good change and moving away from macros towards functions is definitively the right direction! Just skimming the diff I wondered if toolbar::add_button makes sense? we already have add_space and IMHO that are the only two things we do with toolbars - we never add any other widget to them, but buttons. -- https://code.launchpad.net/~widelands-dev/widelands/toolbar_cleanup/+merge/35 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar_cleanup. ___ 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
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/toolbar_cleanup into lp:widelands
Continuous integration builds have changed state: Travis build 1632. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/177718951. Appveyor build 1470. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_toolbar_cleanup-1470. -- https://code.launchpad.net/~widelands-dev/widelands/toolbar_cleanup/+merge/35 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar_cleanup. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/toolbar_cleanup into lp:widelands
I can confirm the fixed crash. Since the crash is gone and the spacing is back I would say the branch is ready for merge. That is, if you design change is planned for another branch. -- https://code.launchpad.net/~widelands-dev/widelands/toolbar_cleanup/+merge/35 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar_cleanup. ___ 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/toolbar_cleanup into lp:widelands
Should be fixed now. Good point about the editor toolbar - I have had an idea for a design change now though, so I'd rather not spend and time on it right now. I also noticed the button flicker, but I didn't find a ready fix for it. Since it's already in trunk - a problem for another day. -- https://code.launchpad.net/~widelands-dev/widelands/toolbar_cleanup/+merge/35 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/toolbar_cleanup. ___ 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
