widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Rebutalled some comments, the rest are done.

Thanks for the review!

@bunnybot merge

Diff comments:

> 
> === modified file 'src/logic/map_objects/tribes/militarysite.cc'
> --- src/logic/map_objects/tribes/militarysite.cc	2017-06-06 05:26:58 +0000
> +++ src/logic/map_objects/tribes/militarysite.cc	2017-06-19 07:06:15 +0000
> @@ -43,6 +43,114 @@
>  
>  namespace Widelands {
>  
> +// TODO(sirver): This method should probably return a const reference.

remode the TODOs for now.

> +std::vector<Soldier*> MilitarySite::SoldierControl::present_soldiers() const {
> +	std::vector<Soldier*> soldiers;
> +
> +	for (Worker* worker : military_site_->get_workers()) {
> +		if (upcast(Soldier, soldier, worker)) {
> +			if (military_site_->is_present(*soldier)) {
> +				soldiers.push_back(soldier);
> +			}
> +		}
> +	}
> +	return soldiers;
> +}
> +
> +// TODO(sirver): This method should probably return a const reference.
> +std::vector<Soldier*> MilitarySite::SoldierControl::stationed_soldiers() const {
> +	std::vector<Soldier*> soldiers;
> +
> +	for (Worker* worker : military_site_->get_workers()) {
> +		if (upcast(Soldier, soldier, worker)) {
> +			soldiers.push_back(soldier);
> +		}
> +	}
> +	return soldiers;
> +}
> +
> +Quantity MilitarySite::SoldierControl::min_soldier_capacity() const {
> +	return 1;
> +}
> +
> +Quantity MilitarySite::SoldierControl::max_soldier_capacity() const {
> +	return military_site_->descr().get_max_number_of_soldiers();
> +}
> +
> +Quantity MilitarySite::SoldierControl::soldier_capacity() const {
> +	return military_site_->capacity_;
> +}
> +
> +void MilitarySite::SoldierControl::set_soldier_capacity(uint32_t const capacity) {
> +	assert(min_soldier_capacity() <= capacity);
> +	assert(capacity <= max_soldier_capacity());
> +	assert(military_site_->capacity_ != capacity);
> +	military_site_->capacity_ = capacity;
> +	military_site_->update_soldier_request();
> +}
> +
> +void MilitarySite::SoldierControl::drop_soldier(Soldier& soldier) {
> +	Game& game = dynamic_cast<Game&>(military_site_->owner().egbase());
> +
> +	if (!military_site_->is_present(soldier)) {
> +		// This can happen when the "drop soldier" player command is delayed
> +		// by network delay or a client has bugs.
> +		military_site_->molog("MilitarySite::drop_soldier(%u): not present\n", soldier.serial());
> +		return;
> +	}
> +	if (present_soldiers().size() <= min_soldier_capacity()) {
> +		military_site_->molog("cannot drop last soldier(s)\n");
> +		return;
> +	}
> +
> +	soldier.reset_tasks(game);
> +	soldier.start_task_leavebuilding(game, true);
> +
> +	military_site_->update_soldier_request();
> +}
> +
> +int MilitarySite::SoldierControl::incorporate_soldier(EditorGameBase& egbase, Soldier& s) {
> +	if (s.get_location(egbase) != military_site_) {
> +		s.set_location(military_site_);
> +	}
> +
> +	// Soldier upgrade is done once the site is full. In soldier upgrade, we
> +	// request one new soldier who is better suited than the existing ones.
> +	// Normally, I kick out one existing soldier as soon as a new guy starts walking
> +	// towards here. However, since that is done via infrequent polling, a new soldier
> +	// can sometimes reach the site before such kick-out happens. In those cases, we
> +	// should either drop one of the existing soldiers or reject the new guy, to
> +	// avoid overstocking this site.
> +
> +	if (stationed_soldiers().size() > military_site_->descr().get_max_number_of_soldiers()) {
> +		return military_site_->incorporate_upgraded_soldier(egbase, s) ? 0 : -1;
> +	}
> +
> +	if (!military_site_->didconquer_) {
> +		military_site_->conquer_area(egbase);
> +		// Building is now occupied - idle animation should be played
> +		military_site_->start_animation(egbase, military_site_->descr().get_animation("idle"));
> +
> +		if (upcast(Game, game, &egbase)) {
> +			military_site_->send_message(
> +			   *game, Message::Type::kEconomySiteOccupied, military_site_->descr().descname(),
> +			   military_site_->descr().icon_filename(), military_site_->descr().descname(),
> +			   military_site_->descr().occupied_str_, true);
> +		}
> +	}
> +
> +	if (upcast(Game, game, &egbase)) {
> +		// Bind the worker into this house, hide him on the map
> +		s.reset_tasks(*game);
> +		s.start_task_buildingwork(*game);
> +	}
> +
> +	// Make sure the request count is reduced or the request is deleted.
> +	military_site_->update_soldier_request(true);
> +
> +	return 0;
> +}
> +
>  bool MilitarySite::AttackTarget::can_be_attacked() const {
>  	return military_site_->didconquer_;
>  }
> 
> === modified file 'src/logic/map_objects/tribes/trainingsite.cc'
> --- src/logic/map_objects/tribes/trainingsite.cc	2017-04-23 12:11:19 +0000
> +++ src/logic/map_objects/tribes/trainingsite.cc	2017-06-19 07:06:15 +0000
> @@ -178,6 +178,78 @@
>  	}
>  }
>  
> +std::vector<Soldier*> TrainingSite::SoldierControl::present_soldiers() const {
> +	return training_site_->soldiers_;
> +}
> +
> +std::vector<Soldier*> TrainingSite::SoldierControl::stationed_soldiers() const {
> +	return training_site_->soldiers_;
> +}
> +
> +Quantity TrainingSite::SoldierControl::min_soldier_capacity() const {
> +	return 0;
> +}
> +Quantity TrainingSite::SoldierControl::max_soldier_capacity() const {
> +	return training_site_->descr().get_max_number_of_soldiers();
> +}
> +Quantity TrainingSite::SoldierControl::soldier_capacity() const {
> +	return training_site_->capacity_;
> +}
> +
> +void TrainingSite::SoldierControl::set_soldier_capacity(Quantity const capacity) {

I added a TODO to investigate if SoldierControl could be shared between different kind of buildings. So far, for type safety I do not see how this can be shared without templating - which would make the code harder to understand IMHO.

> +	assert(min_soldier_capacity() <= capacity);
> +	assert(capacity <= max_soldier_capacity());
> +	assert(training_site_->capacity_ != capacity);
> +	training_site_->capacity_ = capacity;
> +	training_site_->update_soldier_request();
> +}
> +
> +/**
> + * Drop a given soldier.
> + *
> + * 'Dropping' means releasing the soldier from the site. The soldier then
> + * becomes available to the economy.
> + *
> + * \note This is called from player commands, so we need to verify that the
> + * soldier is actually stationed here, without breaking anything if he isn't.
> + */
> +void TrainingSite::SoldierControl::drop_soldier(Soldier& soldier) {
> +	Game& game = dynamic_cast<Game&>(training_site_->owner().egbase());
> +
> +	std::vector<Soldier*>::iterator it = std::find(training_site_->soldiers_.begin(), training_site_->soldiers_.end(), &soldier);
> +	if (it == training_site_->soldiers_.end()) {
> +		training_site_->molog("TrainingSite::SoldierControl::drop_soldier: soldier not in training site");
> +		return;
> +	}
> +
> +	training_site_->soldiers_.erase(it);
> +
> +	soldier.reset_tasks(game);
> +	soldier.start_task_leavebuilding(game, true);
> +
> +	// Schedule, so that we can call new soldiers on next act()
> +	training_site_->schedule_act(game, 100);
> +	Notifications::publish(NoteTrainingSiteSoldierTrained(training_site_, training_site_->get_owner()));
> +}
> +
> +int TrainingSite::SoldierControl::incorporate_soldier(EditorGameBase& egbase, Soldier& s) {
> +	if (s.get_location(egbase) != training_site_) {
> +		if (stationed_soldiers().size() + 1 > training_site_->descr().get_max_number_of_soldiers())
> +			return -1;
> +
> +		s.set_location(training_site_);
> +	}
> +
> +	// Bind the worker into this house, hide him on the map
> +	if (upcast(Game, game, &egbase))
> +		s.start_task_idle(*game, 0, -1);
> +
> +	// Make sure the request count is reduced or the request is deleted.
> +	training_site_->update_soldier_request();
> +
> +	return 0;
> +}
> +
>  /*
>  =============================
>  


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