Hello, 

Thanks for the comments.

The key name "saplingsearches" is silly, and should be replaced by ... hmm, 
what? Any English-speaking people here?

The larger the saplingsearches-number is, the smaller the odds that the 
forester plants the tree to marginal land. This becomes important when the area 
where trees grow well is very small. If there is a borderline of two poor 
terrains, and the terrain property merging algorithm happens to make the 
borderline itself okay for something, you want many dozens of attempts to pick 
that line.

Should we go there? If a significant fraction of all available spots will often 
be inspected by the forester, then we could consider removing the 
already-instanced slots from the "list" of run_findspace in worker.cc. I do not 
think we want the forester to work this way.

====

I agree, that making the saplingsearches-numbers unequal can change balance 
somewhere. We do not have to use that possibility, or use only modest 
differences.

I am not sure if I understood your comment right. The cache must not survive 
from one game to other. Else there is a (small but still) possibility that the 
one player's cached values are from a different but same size map, and 
multiplayer game desyncs before it is catched. Therefore "game" seems like 
logical place, and it cannot be a static class member. If I make it extern and 
instantiate in dot-cc, effectively same problem appears. Where did I get it 
wrong?
Also, I did not understand the "should be const" remark. Values are added to 
the vector, so the vector itself is not constant. Integers are constant by 
definition, the value of one remains one.

Apart from that, I liked what you wrote.

====

If you feel like trying this out: Thanks for the feedback! No particular reason 
to hurry -- we are not close to any deadlines AFAIK.

-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1574379-forester-wit/+merge/336068
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1574379-forester-wit 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

Reply via email to