Added some diff comments.
Diff comments:
> === modified file 'src/logic/productionsite.cc'
> --- src/logic/productionsite.cc 2015-11-11 09:53:54 +0000
> +++ src/logic/productionsite.cc 2015-11-21 19:58:38 +0000
> @@ -284,33 +284,56 @@
> }
>
> /**
> - * Detect if the workers are experienced enough for an upgrade
> + * Detect if the workers are experienced enough for an target building
> + * Buildable workers are skipped, but upgraded ones (required be target
> site) are tested
> * @param idx Index of the enhancement
> */
> bool ProductionSite::has_workers(DescriptionIndex targetSite, Game & /* game
> */)
> {
> // bld holds the description of the building we want to have
> if (upcast(ProductionSiteDescr const, bld,
> owner().tribe().get_building_descr(targetSite))) {
> - // if he has workers
> +
> if (bld->nr_working_positions()) {
> - DescriptionIndex need =
> bld->working_positions()[0].first;
> - for (unsigned int i = 0; i <
> descr().nr_working_positions(); ++i) {
> - if (!working_positions()[i].worker) {
> - return false; // no one is in this house
> - } else {
> - DescriptionIndex have =
> working_positions()[i].worker->descr().worker_index();
> - if
> (owner().tribe().get_worker_descr(have)->can_act_as(need)) {
> - return true; // he found a lead
> worker
> +
> + // Iterating over workers positions in target building
> + for (const auto& wp : bld->working_positions()) {
> +
> + // If worker for this position is buildable,
> just skip him
> + if
> (owner().tribe().get_worker_descr(wp.first)->is_buildable()){
> + continue;
> + }
> +
> + // This position needs promoted worker, so
> trying to find out if there is such worker
> + // currently available in this site
> + const DescriptionIndex needed_worker = wp.first;
> + bool worker_available = false;
> + for (unsigned int i = 0; i <
> descr().nr_working_positions(); ++i) {
> + if (working_positions()[i].worker) {
You are getting working_positions()[i] twice here. Why not just iterate over
working_positions() with a range-based for loop?
> + DescriptionIndex current_worker
> + =
> working_positions()[i].worker->descr().worker_index();
> + if
> (owner().tribe().get_worker_descr(current_worker)->can_act_as(needed_worker))
> {
> + worker_available =
> true; // We found a worker for the position
Add a "break" statement to leave the inner loop - no need to check further, we
have already found one.
> + }
> }
> }
> + if (!worker_available) {
> + // We dont have needed workers in the
> site :(
> + return false;
> + }
> +
> }
> - return false;
> +
> + //if we are here, all needs are satisfied
> + return true;
> +
> + } else {
> + throw wexception("Building, index: %d, needs no
> workers!\n", targetSite);
> }
> - return true;
> - } else return true;
> + } else { //NOCOM
Remove the NOCOM unless you have a question.
> + throw wexception("No such building, index: %d\n", targetSite);
> + }
> }
>
> -
> WaresQueue & ProductionSite::waresqueue(DescriptionIndex const wi) {
> for (WaresQueue * ip_queue : m_input_queues) {
> if (ip_queue->get_ware() == wi) {
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1518223/+merge/278249
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1518223 into lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
