widelands-dev team mailing list archive

[Benedikt Straub <benedikt-straub@xxxxxx>]

Looks good, one suggestion in the diff

Diff comments:

> 
> === modified file 'src/economy/portdock.h'
> --- src/economy/portdock.h	2019-04-24 06:51:31 +0000
> +++ src/economy/portdock.h	2019-06-24 19:08:56 +0000
> @@ -140,6 +134,14 @@
>  private:
>  	friend struct Fleet;
>  
> +	// Does nothing
> +	void draw(uint32_t gametime,

Since you already declare that here, it would look more natural IMHO if you also move the definition from the .cc to the header

> +	          TextToDraw draw_text,
> +	          const Vector2f& point_on_dst,
> +	          const Coords&,
> +	          float scale,
> +	          RenderTarget* dst) override;
> +
>  	void init_fleet(EditorGameBase& egbase);
>  	void set_fleet(Fleet* fleet);
>  	void update_shippingitem(Game&, std::list<ShippingItem>::iterator);
> 
> === modified file 'src/economy/road.h'
> --- src/economy/road.h	2019-04-24 06:01:37 +0000
> +++ src/economy/road.h	2019-06-24 19:08:56 +0000
> @@ -130,6 +130,8 @@
>  	bool init(EditorGameBase&) override;
>  	void cleanup(EditorGameBase&) override;
>  
> +private:
> +	// Does nothing

Since you already declare that here, it would look more natural IMHO if you move the definition (and also the explanation) from the .cc to the header

>  	void draw(uint32_t gametime,
>  	          TextToDraw draw_text,
>  	          const Vector2f&,


-- 
https://code.launchpad.net/~widelands-dev/widelands/get-rid-of-transport-draw/+merge/369263
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/get-rid-of-transport-draw into lp:widelands.