Merge lp:~kxq/widelands/bug-1348800-forester_adds_weight into lp:widelands

Proposed by Teppo Mäenpää on 2015-01-12
Status: Merged
Merged at revision: 7357
Proposed branch: lp:~kxq/widelands/bug-1348800-forester_adds_weight
Merge into: lp:widelands
Diff against target: 33 lines (+21/-2)
1 file modified
src/logic/ (+21/-2)
To merge this branch: bzr merge lp:~kxq/widelands/bug-1348800-forester_adds_weight
Reviewer Review Type Date Requested Status
SirVer 2015-01-12 Approve on 2015-01-16
Review via email:

Description of the change

Forester now favours the most suitable tree(s).

To post a comment you must log in.
7348. By Teppo Mäenpää on 2015-01-12

Seemingly, I messed something up just before previous commit. Now this might even compile.

SirVer (sirver) wrote :

some nits and some need for explanation.

review: Needs Information
7349. By Teppo Mäenpää on 2015-01-14


Teppo Mäenpää (kxq) :
Teppo Mäenpää (kxq) wrote :

code comments seem to have disappeared.

I put the additional addition to "choice", to ensure that it is always positive and therefore state.ivar2 is always assigned.

7350. By Teppo Mäenpää on 2015-01-14

Removed the epsilon.

SirVer (sirver) wrote :

> code comments seem to have disappeared.

you have to select the diff you want to look at below. Unfortunately, the diff comments are bound to the diff and not listed again. I added one more to the old diff.

review: Needs Fixing
Teppo Mäenpää (kxq) wrote :

Would you mind writing it here? For some reason, I cannot see any new comments.

Maybe Launchpad is still updating itself.

7351. By Teppo Mäenpää on 2015-01-14

Even smaller tuning.

Teppo Mäenpää (kxq) wrote :

>you have to select the diff you want to look at below.
I added one more to the old diff.

Are you sure you clicked the "save comment" button? I still cannot find the new comment; guessing that it should have propagated through launchpad by now.

SirVer (sirver) wrote :


> for (auto bsii
use const auto& to avoid reconstructing the tuple. for both loops. Please also add {} for the second for.

> Adding epsilon
I was not concerned about performance - this is not hot inner loop code anyways. I just didn't get it. In fact, I still do not get it - espilon() is 1e-18 or so, why does adding it turn choice into a positive number?

logic_rand_as_double always returns a positive number so it is probably not needed. I just want to understand it.

SirVer (sirver) wrote :

In fact I did not save the comment before ... sorry :/

7352. By Teppo Mäenpää on 2015-01-15

A performance improvement of dozens of percents (in a minuscule loop).

Teppo Mäenpää (kxq) wrote :

Now I have solved all the issues I am aware of.

Teppo Mäenpää (kxq) wrote :

Ready for a re-review.

SirVer (sirver) wrote :

looks good to me. Could you merge?

review: Approve
7353. By Teppo Mäenpää on 2015-01-16

merged trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/logic/'
2--- src/logic/ 2014-12-28 16:45:37 +0000
3+++ src/logic/ 2015-01-16 19:01:32 +0000
4@@ -843,8 +843,27 @@
5 }
7 // Randomly pick one of the immovables to be planted.
8- const uint32_t idx = game.logic_rand() % best_suited_immovables_index.size();
9- state.ivar2 = std::get<1>(*std::next(best_suited_immovables_index.begin(), idx));
11+ // Each candidate is weighted by its probability to grow.
12+ double total_weight = 0.0;
13+ for (const auto& bsii : best_suited_immovables_index)
14+ {
15+ double weight = std::get<0>(bsii);
16+ total_weight += weight * weight;
17+ }
19+ double choice = logic_rand_as_double(&game) * total_weight;
21+ for (const auto& bsii : best_suited_immovables_index)
22+ {
23+ double weight = std::get<0>(bsii);
24+ state.ivar2 = std::get<1>(bsii);
25+ choice -= weight * weight;
26+ if (0 > choice)
27+ {
28+ break;
29+ }
30+ }
32 Immovable& newimm =
33 game.create_immovable(pos, state.ivar2, state.svar1 == "tribe" ? &descr().tribe() : nullptr);


People subscribed via source and target branches

to status/vote changes: