← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1574379-forester-wit into lp:widelands

 

This is quite simple implementation, and seems to work OK in most cases.

There are some things which I am not perfectly happy with:
- There are quite many nested loops. I worked around by a cache, but that spawns other issues:
-- If the map is changes during a game, via LUA scripting, the forester might not immediately realize this. It would be more civilized if the cache would be invalidated in those cases. This quirk does not harm anything now.
-- The cache is physically located in game.h. This is probably not the best place. What is?

If the cache was done right, it could also be benefitted while working on the forestability tooltip into editor. Not really in scope, but still.

========

Remark, should be actually added to comments. The forester considers six best suited trees while planting, but only the best suited three while searching for spots. This is approximately okay, but could in some situations cause the forester/ranger to prefer next-best locations.

Example.

There is terrain where the growth probabilities for the six best trees are:
a => 0.9, 1e-5, 1e-5, 1e-5, 1e-5, 1e-5
b => 1.0, 0.5,  0.5,  0.5,  0.5,  0.5

The goodness of spot a is 0.9 during search, and goodness of spot b is 1.0. Therefore the forester/ranger select option b.

While planting, in spot b there are non-negligible changes that the forester picks one of the fifty-percent trees (56% of the time). Average tree growth thus happens ~72% of the time.

If the forester would have picked slot a, he would practically always have chosen the best-suited tree, with ~90% growth success rate.

This is of course an exaggerated example, and IMO should be left like this. Feel free to disagree.

=========

Now, the Atlantean forester considers most spots, Barbarian ranger least. This was requested in the comments of one of the duplicate bugs. The new constants (number of slots to consider per plantation) are not well thought-of. If you playtest, give feedback.
-- 
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.


References