search

Thorinair / Stardew-Profits

A Stardew Valley crop profits calculator and visualizer. Link: https://thorinair.github.io/Stardew-Profits/
MIT License
164 stars 79 forks source link

Corrections #7

Closed Berbe closed 7 years ago

Berbe commented 7 years ago

Commit https://github.com/Thorinair/Stardew-Profits/pull/7/commits/239a042dd081b8d21f1cd4af3dae60adc302c2f2 has a wrong message.

I mixed up crop.produce.raw* with level.ratio*.

The actual error is even worse! You multiply the yield of a normal production with the on of the extra production! Your productivity is sky-high, as well as the erroneous fashion of the results...

Berbe commented 7 years ago

Actually... Commit https://github.com/Thorinair/Stardew-Profits/pull/7/commits/239a042dd081b8d21f1cd4af3dae60adc302c2f2 incorrect! The other one is valid, but maybe not worth a PR by itself.

Thorinair commented 7 years ago

Wait, what.... Haha

Berbe commented 7 years ago

Newbie syndrome. Sorry for any inconvenience ^^ However, youwould still safely implement https://github.com/Thorinair/Stardew-Profits/pull/7/commits/dd7c0aaaf541a65593e91b7dae649c42c8a63e12 on your side, although as stated above I did not consider worth it to push another PR...

You might be willing to give a shot at an improvement I made on my fork: Berbe/Stardew-Profits, or try it online. It is preliminary, as https://github.com/Berbe/Stardew-Profits/commit/a0d1c2c682745dcad74498183e9010e5447c1e23 details what is missing (might be coming?).

Thorinair commented 7 years ago

If you are confident that what you coded is all correct, I can merge it back into my fork. I don't currently have time to properly review it, so let me know if you think it is okay for me to merge. :)