widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Review: Approve

Code LGTM, 2 nits.

Needs testing.

Diff comments:

> === modified file 'src/editor/editorinteractive.cc'
> --- src/editor/editorinteractive.cc	2017-09-02 19:34:45 +0000
> +++ src/editor/editorinteractive.cc	2017-09-03 13:04:48 +0000
> @@ -322,6 +344,39 @@
>  			constexpr int kStartingPosHotspotY = 55;
>  			blit_overlay(player_image, Vector2i(player_image->width() / 2, kStartingPosHotspotY));
>  		}
> +
> +		// Draw selection markers on the field.
> +		if (selected_nodes.count(field.fcoords) > 0) {
> +			const Image* pic = get_sel_picture();
> +			blit_overlay(pic, Vector2i(pic->width() / 2, pic->height() / 2));
> +		}
> +
> +		// Draw selection markers on the triangles.
> +		if (field.all_neighbors_valid()) {
> +			const FieldsToDraw::Field& rn = fields_to_draw->at(field.rn_index);
> +			const FieldsToDraw::Field& brn = fields_to_draw->at(field.brn_index);
> +			const FieldsToDraw::Field& bln = fields_to_draw->at(field.bln_index);
> +			if (selected_triangles.count(
> +			       Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::R))) {
> +				const Vector2f tripos(
> +				   (field.rendertarget_pixel.x + rn.rendertarget_pixel.x + brn.rendertarget_pixel.x) /
> +				      3.f,
> +				   (field.rendertarget_pixel.y + rn.rendertarget_pixel.y + brn.rendertarget_pixel.y) /
> +				      3.f);
> +				const Image* pic = get_sel_picture();
> +				blit(pic, tripos, Vector2i(pic->width() / 2, pic->height() / 2));
> +			}
> +			if (selected_triangles.count(

Why is this checked twice?

> +			       Widelands::TCoords<>(field.fcoords, Widelands::TriangleIndex::D))) {
> +				const Vector2f tripos(
> +				   (field.rendertarget_pixel.x + bln.rendertarget_pixel.x + brn.rendertarget_pixel.x) /
> +				      3.f,
> +				   (field.rendertarget_pixel.y + bln.rendertarget_pixel.y + brn.rendertarget_pixel.y) /
> +				      3.f);
> +				const Image* pic = get_sel_picture();
> +				blit(pic, tripos, Vector2i(pic->width() / 2, pic->height() / 2));
> +			}
> +		}
>  	}
>  }
>  
> 
> === modified file 'src/wui/interactive_base.h'
> --- src/wui/interactive_base.h	2017-09-02 19:36:19 +0000
> +++ src/wui/interactive_base.h	2017-09-03 13:04:48 +0000
> @@ -239,45 +244,49 @@
>  		uint32_t radius;
>  		const Image* pic;
>  		FieldOverlayManager::OverlayId jobid;
> -	} sel_;
> -
> -	MapView map_view_;
> -	ChatOverlay* chat_overlay_;
> -
> -	// These get collected by add_toolbar_button
> -	// so we can call unassign_toggle_button on them in the destructor.
> -	std::vector<UI::UniqueWindow::Registry> registries_;
> -
> -	UI::Box toolbar_;
> -	// No unique_ptr on purpose: 'minimap_' is a UniqueWindow, its parent will
> -	// delete it.
> -	MiniMap* minimap_;
> -	MiniMap::Registry minimap_registry_;
> -	QuickNavigation quick_navigation_;
> -
> -	std::unique_ptr<FieldOverlayManager> field_overlay_manager_;
> -
> -	// The roads that are displayed while a road is being build. They are not
> -	// yet logically in the game, but need to be displayed for the user as
> -	// visual guide. The data type is the same as for Field::road.
> -	std::map<Widelands::Coords, uint8_t> road_building_preview_;
> -
> -	std::unique_ptr<Notifications::Subscriber<GraphicResolutionChanged>>
> -	   graphic_resolution_changed_subscriber_;
> -	std::unique_ptr<Notifications::Subscriber<NoteSound>> sound_subscriber_;
> -	Widelands::EditorGameBase& egbase_;
> -	uint32_t display_flags_;
> -	uint32_t lastframe_;        //  system time (milliseconds)
> -	uint32_t frametime_;        //  in millseconds
> -	uint32_t avg_usframetime_;  //  in microseconds!
> -
> -	FieldOverlayManager::OverlayId road_buildhelp_overlay_jobid_;
> -	Widelands::CoordPath* buildroad_;  //  path for the new road
> -	Widelands::PlayerNumber road_build_player_;
> -
> -	UI::UniqueWindow::Registry debugconsole_;
> -	std::unique_ptr<UniqueWindowHandler> unique_window_handler_;
> -	std::vector<const Image*> workarea_pics_;
> +		} sel_;
> +
> +		MapView map_view_;
> +		ChatOverlay* chat_overlay_;
> +
> +		// These get collected by add_toolbar_button
> +		// so we can call unassign_toggle_button on them in the destructor.
> +		std::vector<UI::UniqueWindow::Registry> registries_;
> +
> +		UI::Box toolbar_;
> +		// No unique_ptr on purpose: 'minimap_' is a UniqueWindow, its parent will
> +		// delete it.
> +		MiniMap* minimap_;
> +		MiniMap::Registry minimap_registry_;
> +		QuickNavigation quick_navigation_;
> +
> +		std::unique_ptr<FieldOverlayManager> field_overlay_manager_;
> +
> +		// The currently enabled work area previews. They are keyed by the
> +		// coordinate that the building that shows the work area is positioned.
> +	   std::map<Widelands::Coords, const WorkareaInfo*> work_area_previews_;
> +
> +	   // The roads that are displayed while a road is being build. They are not

being built

> +		// yet logically in the game, but need to be displayed for the user as
> +		// visual guide. The data type is the same as for Field::road.
> +		std::map<Widelands::Coords, uint8_t> road_building_preview_;
> +
> +		std::unique_ptr<Notifications::Subscriber<GraphicResolutionChanged>>
> +		   graphic_resolution_changed_subscriber_;
> +		std::unique_ptr<Notifications::Subscriber<NoteSound>> sound_subscriber_;
> +		Widelands::EditorGameBase& egbase_;
> +		uint32_t display_flags_;
> +		uint32_t lastframe_;        //  system time (milliseconds)
> +		uint32_t frametime_;        //  in millseconds
> +		uint32_t avg_usframetime_;  //  in microseconds!
> +
> +		FieldOverlayManager::OverlayId road_buildhelp_overlay_jobid_;
> +		Widelands::CoordPath* buildroad_;  //  path for the new road
> +		Widelands::PlayerNumber road_build_player_;
> +
> +		UI::UniqueWindow::Registry debugconsole_;
> +		std::unique_ptr<UniqueWindowHandler> unique_window_handler_;
> +		std::vector<const Image*> workarea_pics_;
>  };
>  
>  #endif  // end of include guard: WL_WUI_INTERACTIVE_BASE_H


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