widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

Review: Approve review, compile

What I get is that you moved around some code from ::init to the CTor for that shipwindow.
See som inline comments.

I see no obvious Problems in that code, no let mey try to download it from appvoyer ors such.


Diff comments:

> 
> === modified file 'src/logic/map_objects/tribes/ship.cc'
> --- src/logic/map_objects/tribes/ship.cc	2017-07-05 19:21:57 +0000
> +++ src/logic/map_objects/tribes/ship.cc	2017-08-09 18:01:24 +0000
> @@ -942,9 +940,6 @@
>  
>  	// Delete the expedition and the economy it created.
>  	expedition_.reset(nullptr);
> -
> -	// And finally update our ship window
> -	Notifications::publish(NoteShipWindow(serial(), NoteShipWindow::Action::kRefresh));

Why are these kRefres Actions harmfull?

>  }
>  
>  /// Sinks the ship
> 
> === modified file 'src/wui/itemwaresdisplay.cc'
> --- src/wui/itemwaresdisplay.cc	2017-06-23 17:23:04 +0000
> +++ src/wui/itemwaresdisplay.cc	2017-08-09 18:01:24 +0000
> @@ -88,15 +90,29 @@
>  void ItemWaresDisplay::draw(RenderTarget& dst) {
>  	const Widelands::TribeDescr& tribe(player().tribe());
>  
> -	dst.fill_rect(Recti(0, 0, get_w(), get_h()), RGBAColor(0, 0, 0, 0));
> +	// Snazzy background
> +	const int width = get_w() - 2 * kMargin;
> +	const int height = get_h() - 2 * kMargin;
> +	RGBAColor black(0, 0, 0, 255);
> +	dst.brighten_rect(Recti(kMargin, kMargin, width - 1, height - 1), -BUTTON_EDGE_BRIGHT_FACTOR / 2);
> +	//  bottom edge
> +	dst.brighten_rect(Recti(kMargin, height + 2, width, 2), 1.5 * BUTTON_EDGE_BRIGHT_FACTOR);
> +	//  right edge
> +	dst.brighten_rect(Recti(kMargin + width - 2, kMargin, 2, height - 2), 1.5 * BUTTON_EDGE_BRIGHT_FACTOR);
> +	//  top edge
> +	dst.fill_rect(Recti(kMargin, kMargin, width - 1, 1), black);
> +	dst.fill_rect(Recti(kMargin, kMargin + 1, width - 2, 1), black);
> +	//  left edge
> +	dst.fill_rect(Recti(kMargin, kMargin, 1, height - 1), black);
> +	dst.fill_rect(Recti(kMargin + 1, kMargin, 1, height - 2), black);

Mhh, this does not really belong to the adressed bug, must check this :-)

>  
>  	for (uint32_t idx = 0; idx < items_.size(); ++idx) {
>  		const Item& it = items_[idx];
>  		uint32_t row = idx / items_per_row_;
>  		uint32_t col = idx % items_per_row_;
>  
> -		uint32_t x = IWD_HBorder / 2 + col * IWD_ItemWidth;
> -		uint32_t y = IWD_VBorder + row * IWD_ItemHeight;
> +		uint32_t x = IWD_HBorder / 2 + col * IWD_ItemWidth + kMargin;
> +		uint32_t y = IWD_VBorder + row * IWD_ItemHeight + kMargin;
>  
>  		if (it.worker) {
>  			y += IWD_WorkerBaseline;


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash/+merge/318986
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1664052-expedition-shipwindow-crash.