[PATCH] Only one woman can gather from a tree

[Kieran P]

See http://www.wildfiregames.com/forum/index.php?showtopic=15971

[historic_bruno]

It's a bit tricky to reproduce. If you send a group of females, they all seem to gather, you have to send them individually to find the bug.

[gudo]

Has anybody tried to reproduce this with other gatherable resources? shrubs, farms, rocks, mines, etc.

[O.Davoodi]

Using Alpha-9: There is no problem while gathering from food sources. But when you are gathering from metal mines, the gather rate of other women gets lower (instead of zero gather rate). Stone mines have the same problem as trees (zero gather rate).

[Matt Lott]

The math in ResourceSupply.prototype.TakeResources() (in ResourceSupply.js) rounds/ceilings away resources based on the gather rate.

2 women gathering on a tree is the easiest repro I've found, because the rate is .5 which makes every other call to TakeResources return 1 wood resource, and the other returns 0. One woman gathers, one doesn't.

This should repro with any even number of women gathering wood, but I've only tested 2 and 4.

Here's the relevant part of TakeResources():

ResourceSupply.prototype.TakeResources = function(rate)
{
	// Internally we handle fractional resource amounts (to be accurate
	// over long periods of time), but want to return integers (so players
	// have a nice simple integer amount of resources). So return the
	// difference between rounded values:

	var old = this.amount;
	this.amount = Math.max(0, old - rate);

	// (use ceil instead of floor so that we continue 
        // returning non-zero values even if 0 < amount < 1)
	var change = Math.ceil(old) - Math.ceil(this.amount);

 	return { "amount": change, "exhausted": (old == 0) };
};

Wood example from TakeResources(): 

gather rate = .5 

1st call 
old amount = 10 
new amount = 10 - .5 = 9.5 
gathered amount = ceil(10) - ceil(9.5) = 0 
0 resources returned

2nd call 
old amount = 9.5 
new amount = 9.5 - .5 = 9 
gathered amount  = ceil(9.5) - ceil(9) = 1 
1 resource returned

I'll look into a fix, unless someone is already working on it.

[leper]

Replying to mattlott:
Thanks for your work on this. We had already identified the problem, but finding a good solution for this takes some time. If you have an idea how to properly solve this (without converting the whole resource system to floating point) please join us in ​#0ad-dev on quakenet.

[Matt Lott]

This updates how gather rate is applied in order to avoid starving gatherers.

[leper]

I took a look at your patch and I have some suggestions.

You should use tabs and not spaces for indentation (We use spaces in some xml files, but you should use the style that is already present in a file).

We should change ResourceSupply to use integers only.

You should use

if (!rate)
{
   this.FinishOrder();
   return;
}

in UnitAI.GATHERING so we don't have to indent the code already in place (and it keeps the logic close together).

The offset should be 1000 (or at least larger than the repeat time, but I would set it to 1000). Otherwise a user can cheat by cycling start stop start stop (See comments in Attack and Heal). Don't bother about modifying the animation speed.

ResourceGatherer
Move the interval-related comment out of the function body. We should still check if a unit can gather from the target in ResourceGatherer, even though UnitAI currently checks that.

[Matt Lott]

Updated based on review feedback

[leper]

In 11689:

Change gathering behaviour to fix #1353, based on patch by mattlott.

[leper]

Thanks for your patch. There was an issue with a target getting exhausted, but I went ahead and fixed that. The target stopped existing while the unit was returning resources and after that we had no rate.