widelands-dev team mailing list archive

[Nasenbaer <Nasenbaer@xxxxxxxxxxxx>]

Review: Needs Information

I am between "Approve" and "Needs fixing" therefore I set it "Needs information"

"Approve", because the patch is fine as it is and if you (and nobody else) want to do the job described below, we can just merge it as it is


"Needs fixing" because as we change the "skipped" parts, we should improve the performance of those checks as well and there is quite some room for improvement :)

E.g. file 'tribes/atlanteans/coalmine/conf':
return=skipped unless economy needs coal or not economy needs bread
return=skipped unless economy needs coal or not economy needs smoked_fish

equals
return=skipped unless economy needs coal or not economy needs bread
return=skipped unless not economy needs smoked_fish

difference is, that Widelands won't doublecheck whether the economy needs coal, which (even if little) costs a bit of performance - the more production sites are running the more performance...

cghislai, do you take this task? :)
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug988870/+merge/174639
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug988870 into lp:widelands.