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.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
