widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Code LGTM - just 1 nit: get_y_fropreceding_ => get_y_from_preceding. I have marked 2 instances with diff comments.

Diff comments:

> 
> === modified file 'src/ui_fsmenu/campaign_select.cc'
> --- src/ui_fsmenu/campaign_select.cc	2015-10-23 18:03:33 +0000
> +++ src/ui_fsmenu/campaign_select.cc	2016-01-28 20:02:32 +0000
> @@ -42,71 +42,71 @@
>   */
>  FullscreenMenuCampaignSelect::FullscreenMenuCampaignSelect() :
>  	FullscreenMenuLoadMapOrGame(),
> -	m_table(this, tablex_, tabley_, tablew_, tableh_, false),
> +	table_(this, tablex_, tabley_, tablew_, tableh_, false),
>  
>  	// Main Title
> -	m_title
> +	title_
>  		(this, get_w() / 2, tabley_ / 3,
>  		 _("Choose a campaign"),
>  		 UI::Align_HCenter),
>  
>  	// Campaign description
> -	m_label_campname
> +	label_campname_
>  		(this, right_column_x_, tabley_,
>  		 "",
>  		 UI::Align_Left),
> -	m_ta_campname(this,
> -					  right_column_x_ + indent_, get_y_from_preceding(m_label_campname) + padding_,
> -					  get_right_column_w(right_column_x_) - indent_, m_label_height),
> -
> -	m_label_tribename
> -		(this, right_column_x_, get_y_from_preceding(m_ta_campname) + 2 * padding_,
> -		 "",
> -		 UI::Align_Left),
> -	m_ta_tribename(this,
> -						 right_column_x_ + indent_, get_y_from_preceding(m_label_tribename) + padding_,
> -						 get_right_column_w(right_column_x_ + indent_), m_label_height),
> -
> -	m_label_difficulty
> -		(this, right_column_x_, get_y_from_preceding(m_ta_tribename) + 2 * padding_,
> -		 "",
> -		 UI::Align_Left),
> -	m_ta_difficulty(this,
> -						 right_column_x_ + indent_, get_y_from_preceding(m_label_difficulty) + padding_,
> -						 get_right_column_w(right_column_x_ + indent_), 2 * m_label_height - padding_),
> -
> -	m_label_description
> -		(this, right_column_x_, get_y_from_preceding(m_ta_difficulty) + 2 * padding_,
> +	ta_campname_(this,
> +					  right_column_x_ + indent_, get_y_fropreceding_(label_campname_) + padding_,
> +					  get_right_column_w(right_column_x_) - indent_, label_height_),
> +
> +	label_tribename_
> +		(this, right_column_x_, get_y_fropreceding_(ta_campname_) + 2 * padding_,
> +		 "",
> +		 UI::Align_Left),
> +	ta_tribename_(this,
> +						 right_column_x_ + indent_, get_y_fropreceding_(label_tribename_) + padding_,
> +						 get_right_column_w(right_column_x_ + indent_), label_height_),
> +
> +	label_difficulty_
> +		(this, right_column_x_, get_y_fropreceding_(ta_tribename_) + 2 * padding_,
> +		 "",
> +		 UI::Align_Left),
> +	ta_difficulty_(this,
> +						 right_column_x_ + indent_, get_y_fropreceding_(label_difficulty_) + padding_,
> +						 get_right_column_w(right_column_x_ + indent_), 2 * label_height_ - padding_),
> +
> +	label_description_
> +		(this, right_column_x_, get_y_fropreceding_(ta_difficulty_) + 2 * padding_,
>  		 _("Description:"),
>  		 UI::Align_Left),
> -	m_ta_description
> +	ta_description_
>  		(this,
>  		 right_column_x_ + indent_,
> -		 get_y_from_preceding(m_label_description) + padding_,
> +		 get_y_fropreceding_(label_description_) + padding_,

get_y_fropreceding_ => get_y_from_preceding

>  		 get_right_column_w(right_column_x_ + indent_),
> -		 m_buty - get_y_from_preceding(m_label_description) - 4 * padding_)
> +		 buty_ - get_y_fropreceding_(label_description_) - 4 * padding_)
>  {
> -	m_title.set_textstyle(UI::TextStyle::ui_big());
> +	title_.set_textstyle(UI::TextStyle::ui_big());
>  	back_.set_tooltip(_("Return to the main menu"));
>  	ok_.set_tooltip(_("Play this campaign"));
> -	m_ta_campname.set_tooltip(_("The name of this campaign"));
> -	m_ta_tribename.set_tooltip(_("The tribe you will be playing"));
> -	m_ta_difficulty.set_tooltip(_("The difficulty of this campaign"));
> +	ta_campname_.set_tooltip(_("The name of this campaign"));
> +	ta_tribename_.set_tooltip(_("The tribe you will be playing"));
> +	ta_difficulty_.set_tooltip(_("The difficulty of this campaign"));
>  
>  	ok_.sigclicked.connect(boost::bind(&FullscreenMenuCampaignSelect::clicked_ok, boost::ref(*this)));
>  	back_.sigclicked.connect(boost::bind(&FullscreenMenuCampaignSelect::clicked_back, boost::ref(*this)));
> -	m_table.selected.connect(boost::bind(&FullscreenMenuCampaignSelect::entry_selected, this));
> -	m_table.double_clicked.connect(boost::bind(&FullscreenMenuCampaignSelect::clicked_ok, boost::ref(*this)));
> +	table_.selected.connect(boost::bind(&FullscreenMenuCampaignSelect::entry_selected, this));
> +	table_.double_clicked.connect(boost::bind(&FullscreenMenuCampaignSelect::clicked_ok, boost::ref(*this)));
>  
>  	/** TRANSLATORS: Campaign difficulty table header */
> -	m_table.add_column(45, _("Diff."), _("Difficulty"), UI::Align_Left);
> -	m_table.add_column(100, _("Tribe"), _("Tribe Name"), UI::Align_Left);
> -	m_table.add_column(m_table.get_w() - 100 - 45, _("Campaign Name"), _("Campaign Name"), UI::Align_Left);
> -	m_table.set_column_compare
> +	table_.add_column(45, _("Diff."), _("Difficulty"), UI::Align_Left);
> +	table_.add_column(100, _("Tribe"), _("Tribe Name"), UI::Align_Left);
> +	table_.add_column(table_.get_w() - 100 - 45, _("Campaign Name"), _("Campaign Name"), UI::Align_Left);
> +	table_.set_column_compare
>  			(0,
>  			 boost::bind(&FullscreenMenuCampaignSelect::compare_difficulty, this, _1, _2));
> -	m_table.set_sort_column(0);
> -	m_table.focus();
> +	table_.set_sort_column(0);
> +	table_.focus();
>  	fill_table();
>  }
>  
> 
> === modified file 'src/ui_fsmenu/load_map_or_game.cc'
> --- src/ui_fsmenu/load_map_or_game.cc	2015-10-02 07:02:00 +0000
> +++ src/ui_fsmenu/load_map_or_game.cc	2016-01-28 20:02:32 +0000
> @@ -39,35 +39,35 @@
>  		// Values for alignment and size
>  		padding_(4),
>  		indent_(10),
> -		m_label_height(20),
> +		label_height_(20),
>  		tablex_(get_w() *  47 / 2500),
>  		tabley_(get_h() * 17 / 50),
>  		tablew_(get_w() * 711 / 1250),
>  		tableh_(get_h() * 6083 / 10000),
> -		m_right_column_margin(15),
> -		right_column_x_(tablex_ + tablew_ + m_right_column_margin),
> -		m_buty (get_h() * 9 / 10),
> -		m_butw ((get_w() - right_column_x_ - m_right_column_margin) / 2 - padding_),
> +		right_column_margin_(15),
> +		right_column_x_(tablex_ + tablew_ + right_column_margin_),
> +		buty_ (get_h() * 9 / 10),
> +		butw_ ((get_w() - right_column_x_ - right_column_margin_) / 2 - padding_),
>  		buth_ (get_h() * 9 / 200),
> -		m_right_column_tab(get_w() - m_right_column_margin - m_butw),
> +		right_column_tab_(get_w() - right_column_margin_ - butw_),
>  
>  		// Main buttons
>  		back_
>  		  (this, "back",
> -			right_column_x_, m_buty, m_butw, buth_,
> +			right_column_x_, buty_, butw_, buth_,
>  			g_gr->images().get("pics/but0.png"),
>  			_("Back"), std::string(), true, false),
>  		ok_
>  		  (this, "ok",
> -			get_w() - m_right_column_margin - m_butw, m_buty, m_butw, buth_,
> +			get_w() - right_column_margin_ - butw_, buty_, butw_, buth_,
>  			g_gr->images().get("pics/but2.png"),
>  			_("OK"), std::string(), false, false)
>  	{}
>  
> -int32_t FullscreenMenuLoadMapOrGame::get_y_from_preceding(UI::Panel& preceding_panel) {
> +int32_t FullscreenMenuLoadMapOrGame::get_y_fropreceding_(UI::Panel& preceding_panel) {

get_y_fropreceding_ => get_y_from_preceding

>  	return preceding_panel.get_y() + preceding_panel.get_h();
>  }
>  
>  int32_t FullscreenMenuLoadMapOrGame::get_right_column_w(int32_t x) {
> -	return get_w() - m_right_column_margin - x;
> +	return get_w() - right_column_margin_ - x;
>  }


-- 
https://code.launchpad.net/~klaus-halfmann/widelands/bug-1395278-ui_fsmenu/+merge/284339
Your team Widelands Developers is requested to review the proposed merge of lp:~klaus-halfmann/widelands/bug-1395278-ui_fsmenu into lp:widelands.