widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Needs Fixing

couple of nits not related to your changes. But while you are digging through the code....

Diff comments:

> === modified file 'src/ui_basic/slider.cc'
> --- src/ui_basic/slider.cc	2014-11-27 12:02:08 +0000
> +++ src/ui_basic/slider.cc	2015-07-27 07:23:23 +0000
> @@ -113,8 +113,10 @@
>   */
>  void Slider::set_max_value(int32_t new_max) {
>  	assert(m_min_value <= new_max);
> -	if (m_max_value != new_max)
> +	if (m_max_value != new_max) {
> +		calc_cursor_pos();

while you around that code: calculate_cursor_position()

>  		update();
> +	}
>  	m_max_value = new_max;
>  	set_value(m_value);
>  }
> 
> === modified file 'src/wui/attack_box.cc'
> --- src/wui/attack_box.cc	2015-03-30 08:45:09 +0000
> +++ src/wui/attack_box.cc	2015-07-27 07:23:23 +0000
> @@ -30,6 +30,8 @@
>  #include "graphic/text_constants.h"
>  #include "logic/soldier.h"
>  
> +constexpr int32_t kUpdateTime = 1000;  //  1 second, gametime

kUpdateTimeInGametimeMs

> +
>  AttackBox::AttackBox
>  	(UI::Panel              * parent,
>  	 Widelands::Player      * player,
> @@ -44,7 +46,9 @@
>  	m_node(target),
>  	m_slider_soldiers(nullptr),
>  	m_text_soldiers(nullptr),
> -	m_add_soldiers(nullptr)
> +	m_less_soldiers(nullptr),
> +	m_add_soldiers(nullptr),

consistency: m_[add|subtract]_soldiers or m_[more|less]_soldiers.

> +	lastupdate_(0)
>  {
>  	init();
>  }
> @@ -52,6 +56,7 @@
>  AttackBox::~AttackBox() {
>  	delete m_slider_soldiers;
>  	delete m_text_soldiers;
> +	delete m_less_soldiers;

make these into unique_ptrs? naked deletes are so 80ths.

>  	delete m_add_soldiers;
>  }
>  
> @@ -119,6 +124,17 @@
>  	return *button;
>  }
>  
> +/*
> + * Update available soldiers
> + */
> +void AttackBox::think() {
> +	int32_t const gametime = m_pl->egbase().get_gametime();

const

> +	if ((gametime - lastupdate_) > kUpdateTime) {
> +		update_attack();
> +		lastupdate_ = gametime;
> +	}
> +}
> +
>  void AttackBox::update_attack() {
>  	assert(m_slider_soldiers);
>  	assert(m_text_soldiers);


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