widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Mhh, you did annotate this as "needs fixing". 
If you insist I will include these changes in some other branch ...

Diff comments:

> 
> === modified file 'src/graphic/align.h'
> --- src/graphic/align.h	2017-01-25 18:55:59 +0000
> +++ src/graphic/align.h	2017-02-14 18:23:39 +0000
> @@ -24,29 +24,40 @@
>  
>  namespace UI {
>  
> +/**
> + * This Enum is a binary mix of one-dimensional and two-dimensional alignments.
> + *
> + * bits 0,1 values 0,1,2,3  are horizontal
> + * bits 2,3 values 0,4,8,12 are vertical
> + *
> + * mixed aligenments are results of a binary | operation.
> + */
> +
> + // TODO(klaus.halfmann): as this is not a real enum all compiler warnings about
> + // incomplete usage are useless.
> +
>  enum class Align {
> -	kLeft = 0,
> -	kHCenter = 1,
> -	kRight = 2,
> -	kHorizontal = 3,
> -
> -	kTop = 0,
> -	kVCenter = 4,
> -	kBottom = 8,
> -	kVertical = 12,
> -
> -	kTopLeft = 0,
> -	kCenterLeft = Align::kVCenter,
> -	kBottomLeft = Align::kBottom,
> -
> -	kTopCenter = Align::kHCenter,
> -	kCenter = Align::kHCenter | Align::kVCenter,
> -	kBottomCenter = Align::kHCenter | Align::kBottom,
> -
> -	kTopRight = Align::kRight,
> -	kCenterRight = Align::kRight | Align::kVCenter,
> -
> -	kBottomRight = Align::kRight | Align::kBottom,
> +	kLeft       = 0x00,
> +	kHCenter    = 0x01,
> +	kRight      = 0x02,
> +	kHorizontal = 0x03,
> +
> +	kTop        = 0x00,
> +	kVCenter    = 0x04,
> +	kBottom     = 0x08,
> +	kVertical   = 0x0C,
> +
> +	kTopLeft        = kLeft | kTop,
> +	kCenterLeft     = kLeft | kVCenter,
> +	kBottomLeft     = kLeft | kBottom,
> +
> +    kTopCenter      = kHCenter | kTop,
> +	kCenter         = kHCenter | kVCenter,
> +	kBottomCenter   = kHCenter | kBottom,
> +
> +	kTopRight       = kRight | kTop,
> +	kCenterRight    = kRight | kVCenter,
> +	kBottomRight    = kRight | kBottom,
>  };

As this is no sematic changed at all (only comments, indentation and conversion to hex)
I would consider this an improvement.

>  
>  inline Align operator&(Align a, Align b) {
> 
> === modified file 'src/wui/building_ui.cc'
> --- src/wui/building_ui.cc	2017-01-25 18:55:59 +0000
> +++ src/wui/building_ui.cc	2017-02-14 18:23:39 +0000
> @@ -68,7 +68,6 @@
>  	if (optionswindow_) {
>  		Vector2i window_position = optionswindow_->get_pos();
>  		hide_options();
> -		show_options(igb, true);
> -		optionswindow_->set_pos(window_position);
> +		show_options(igb, true, window_position);

Code is identical but avoid duplicate call to set_pos().

Is it strictly forbidden to include such improvements?

>  	}
>  }


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1658489-epedition-tab/+merge/317047
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1658489-epedition-tab.