widelands-dev team mailing list archive

[Teppo Mäenpää <tm@xxxxxx>]

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.