widelands-dev team mailing list archive

[Benedikt Straub <benedikt-straub@xxxxxx>]

Formatted the tooltips and replied to diff comments.

> Did you remove the changes from this branch? […]

No, I didn´t even touch the file where that code is located?

Diff comments:

> 
> === modified file 'src/wui/attack_box.cc'
> --- src/wui/attack_box.cc	2019-02-23 11:00:49 +0000
> +++ src/wui/attack_box.cc	2019-05-07 12:26:33 +0000
> @@ -169,7 +237,128 @@
>  	soldiers_slider_->set_value(soldiers_slider_->get_value() + 1);
>  }
>  
> -uint32_t AttackBox::soldiers() const {
> -	assert(soldiers_slider_.get());
> -	return soldiers_slider_->get_value();
> -}
> +size_t AttackBox::count_soldiers() const {
> +	return attacking_soldiers_->count_soldiers();
> +}
> +
> +std::vector<Widelands::Serial> AttackBox::soldiers() const {
> +	std::vector<Widelands::Serial> result;
> +	for (const auto& s : attacking_soldiers_->get_soldiers()) {
> +		result.push_back(s->serial());
> +	}
> +	return result;
> +}
> +
> +constexpr int kSoldierIconWidth = 30;
> +constexpr int kSoldierIconHeight = 29;
> +
> +AttackBox::ListOfSoldiers::ListOfSoldiers(UI::Panel* const parent,
> +	           AttackBox* parent_box,
> +	           int32_t const x,
> +	           int32_t const y,
> +	           int const w,
> +	           int const h,
> +               const std::string& tooltip,
> +               int16_t max_size,
> +               bool restrict_rows)
> +   : UI::Panel(parent, x, y, w, h, tooltip),
> +     size_restriction_(max_size),
> +     restricted_row_number_(restrict_rows),
> +     attack_box_(parent_box) {
> +	update_desired_size();
> +}
> +
> +bool AttackBox::ListOfSoldiers::handle_mousepress(uint8_t btn, int32_t x, int32_t y) {
> +	if (btn != SDL_BUTTON_LEFT || !other_) {
> +		return UI::Panel::handle_mousepress(btn, x, y);
> +	}
> +	if (SDL_GetModState() & KMOD_CTRL) {
> +		for (const auto& s : get_soldiers()) {
> +			remove(s);
> +			other_->add(s);
> +		}
> +	} else {
> +		const Widelands::Soldier* s = soldier_at(x, y);
> +		if (!s) {
> +			return UI::Panel::handle_mousepress(btn, x, y);
> +		}
> +		remove(s);
> +		other_->add(s);
> +	}
> +	attack_box_->update_attack(true);
> +	return true;
> +}
> +
> +Widelands::Extent AttackBox::ListOfSoldiers::size() const {
> +	const size_t nr_soldiers = count_soldiers();
> +	if (nr_soldiers == 0) {
> +		// At least one icon
> +		return Widelands::Extent(1, 1);
> +	}
> +	uint32_t rows = nr_soldiers / size_restriction_;
> +	if (rows * size_restriction_ < nr_soldiers) {
> +		++rows;
> +	}
> +	if (restricted_row_number_) {
> +		return Widelands::Extent(rows, size_restriction_);
> +	} else {
> +		return Widelands::Extent(size_restriction_, rows);
> +	}
> +}
> +
> +void AttackBox::ListOfSoldiers::update_desired_size() {
> +	const Widelands::Extent e = size();
> +	set_desired_size(e.w * kSoldierIconWidth, e.h * kSoldierIconHeight);
> +}
> +
> +const Widelands::Soldier* AttackBox::ListOfSoldiers::soldier_at(int32_t x, int32_t y) const {
> +	if (x < 0 || y < 0 || soldiers_.empty()) {
> +		return nullptr;
> +	}
> +	const int32_t col = x / kSoldierIconWidth;
> +	const int32_t row = y / kSoldierIconHeight;
> +	assert(col >= 0);

Columns and rows are counted from 0 up, so the leftmost col or the topmost row have index 0

> +	assert(row >= 0);
> +	if ((restricted_row_number_ ? row : col) >= size_restriction_) {
> +		return nullptr;
> +	}
> +	const int index = restricted_row_number_ ? size_restriction_ * col + row : size_restriction_ * row + col;
> +	assert(index >= 0);
> +	return static_cast<unsigned int>(index) < soldiers_.size() ? soldiers_[index] : nullptr;
> +}
> +
> +void AttackBox::ListOfSoldiers::add(const Widelands::Soldier* s) {
> +	soldiers_.push_back(s);
> +	update_desired_size();
> +}
> +
> +void AttackBox::ListOfSoldiers::remove(const Widelands::Soldier* s) {
> +	const auto it = std::find(soldiers_.begin(), soldiers_.end(), s);
> +	assert(it != soldiers_.end());
> +	soldiers_.erase(it);
> +	update_desired_size();
> +}
> +
> +void AttackBox::ListOfSoldiers::draw(RenderTarget& dst) {
> +	const size_t nr_soldiers = soldiers_.size();
> +	int32_t column = 0;
> +	int32_t row = 0;
> +	for (uint32_t i = 0; i < nr_soldiers; ++i) {
> +		Vector2i location(column * kSoldierIconWidth, row * kSoldierIconHeight);
> +		soldiers_[i]->draw_info_icon(location, 1.0f, false, &dst);
> +		if (restricted_row_number_) {
> +			++row;
> +			if (row >= size_restriction_) {
> +				row = 0;
> +				++column;
> +			}
> +		} else {
> +			++column;
> +			if (column >= size_restriction_) {
> +				column = 0;
> +				++row;
> +			}
> +		}
> +	}
> +}
> +
> 
> === modified file 'src/wui/attack_box.h'
> --- src/wui/attack_box.h	2019-02-23 11:00:49 +0000
> +++ src/wui/attack_box.h	2019-05-07 12:26:33 +0000
> @@ -88,6 +91,76 @@
>  	std::unique_ptr<UI::Button> less_soldiers_;
>  	std::unique_ptr<UI::Button> more_soldiers_;
>  
> +	// A SoldierPanel is not applicable here as it's keyed to a building and thinks too much

No, the approach to realize the list of soldiers is completely different. SoldierPanel is more elaborate, with animations of new soldiers sliding in, and an intelligent state update on every frame, but everything it does requires access to a SoldierControl object, which is not applicable here without much hacking (doable, but would be very ugly).

> +	struct ListOfSoldiers : public UI::Panel {
> +		ListOfSoldiers(UI::Panel* const parent,
> +		               AttackBox* parent_box,
> +		               int32_t const x,
> +		               int32_t const y,
> +		               int const w,
> +		               int const h,
> +	                   const std::string& tooltip,
> +		               int16_t max_size = 8,
> +		               bool restrict_rows = false);
> +
> +		bool handle_mousepress(uint8_t btn, int32_t x, int32_t y) override;
> +
> +		const Widelands::Soldier* soldier_at(int32_t x, int32_t y) const;
> +		void add(const Widelands::Soldier*);
> +		void remove(const Widelands::Soldier*);
> +		bool contains(const Widelands::Soldier* soldier) const {
> +			for (const auto& s : soldiers_) {
> +				if (s == soldier) {
> +					return true;
> +				}
> +			}
> +			return false;
> +		}
> +
> +		std::vector<const Widelands::Soldier*> get_soldiers() const {
> +			return soldiers_;
> +		}
> +		const Widelands::Soldier* get_soldier() const {
> +			return soldiers_.back();
> +		}
> +
> +		size_t count_soldiers() const {
> +			return soldiers_.size();
> +		}
> +		Widelands::Extent size() const;
> +		bool row_number_restricted() const {
> +			return restricted_row_number_;
> +		}
> +		int16_t size_restriction() const {
> +			return size_restriction_;
> +		}
> +		void set_size_restriction(int16_t r) {
> +			size_restriction_ = r;
> +		}
> +		void set_row_number_restricted(bool r) {
> +			restricted_row_number_ = r;
> +		}
> +
> +		void draw(RenderTarget& dst) override;
> +
> +		void set_complement(ListOfSoldiers* o) {
> +			other_ = o;
> +		}
> +
> +	private:
> +		int16_t size_restriction_; // Highest number of rows or columns
> +		bool restricted_row_number_;
> +		std::vector<const Widelands::Soldier*> soldiers_;
> +
> +		ListOfSoldiers* other_;
> +		AttackBox* attack_box_;
> +
> +		void update_desired_size() override;
> +	};
> +
> +	std::unique_ptr<ListOfSoldiers> attacking_soldiers_;
> +	std::unique_ptr<ListOfSoldiers> remaining_soldiers_;
> +
>  	/// The last time the information in this Panel got updated
>  	uint32_t lastupdate_;
>  };


-- 
https://code.launchpad.net/~widelands-dev/widelands/choose-attack-soldiers/+merge/367041
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/choose-attack-soldiers into lp:widelands.