BastianKanaan / GMscripts_MyFreeFarm

GMscripts MyFreeFarm
GNU General Public License v3.0
16 stars 31 forks source link

Adviser: Recursive calculation error #15

Open dualsky opened 10 years ago

dualsky commented 10 years ago

There is a bug in counting recursive needs (probably in the calcTotalRecursive function).

Steps to reproduce: I have in a rack:

Minimum values defined:

In the lacks table I see I should buy 2.620 tomato and when hoover over tomato I see:

Header +/- amount should be
Min amount 600 600
Recursive need + 3.560 4.500 = 450 * 50 / 5
In rack - 1.540 1.540
On the market 1.96 ft
Value 5.135 ft 6.978 ft = round(3.560 * 1,96)

IMHO I should buy 3.560 tomato to have 5.100 tomato in total then use 4.500 to produce ketchup and left 600 in the rack.

And the best of all on the end. I have no mayonnaise kitchen, so I shouldn't buy any tomato at all, just the ketchup!

MoeLa commented 10 years ago

Hey dualsky, I already mentioned the same issue with the forestry products (and provided a fix, see Pull Request https://github.com/BastianKanaan/GMscripts_MyFreeFarm/pull/1). My solution only regards forestry products but is easily extendable to the other types.

As you mentioned in your last sentence, the recursive calculation doesn't always make sense for each product and product type. The menu option says explicitly "forestry farmis", so any calculation for e.g. ketchup would be wrong.

If you want to test my fix in https://github.com/BastianKanaan/GMscripts_MyFreeFarm/pull/1 for your issue, you either have to change type = 1 to type = 0 or build a for-loop around lines 1148 to 1184. I'd like to hear your feedback.

BastianKanaan commented 10 years ago

Hi dualsky, the explanation of the amounts is the interpretation of the "recursive need"-value. In the beginning and currently implemented, it was the amount needed to buy so that enough (min rack amount) is available. I agree that this is not intuitive and your interpretation is the one I also prefer. I will change the calculation. MoeLa's worries will be fixed by that, too ;-) I think

MoeLa commented 10 years ago

Thank you. And if you don't want to spend any effort on that issue, fell free to use my solution/pull request!

BastianKanaan commented 10 years ago

Can you please check if the current version matches your expectations.

MoeLa commented 10 years ago

Neat, WhiteScripter, works like expected. But was it possible to intruduce a setting to restrict the recursive calculation to type = 1 (forestry) products? I'd create a pull request, if you don't want to do it.

BastianKanaan commented 10 years ago

Of course are settings possible. I also like the idea of dualsky that (optionally?) only products are calculated which can be produced by the farm.