FreeCol / freecol

FreeCol: FreeCol is a turn-based strategy game based on the old game Colonization, and similar to Civilization. The objective of the game is to create an independent nation.
GNU General Public License v2.0
585 stars 170 forks source link

Change sign of goods due to mixup buy/sell #85

Closed bjorneg closed 3 years ago

bjorneg commented 3 years ago

There is a mixup of buy/sell when a unit is equiped with tools in Europe.

See attached before screen-shot: bug_before

And the after screen-shot: bug_after

This a purchase of tools. The total amount of gold is decreased correct, However, the purchase is recorded as a sell. (and vice versa for drop tools)

To recreate:

In Europe:

Note: This fix need to be reviewed - it fixes the issue, but is it the right way to fix it? I may be wrong but usually a buy is a negative number, but sometimes not.

mpope042 commented 3 years ago

Good catch, but the fix is insufficient as the amount is wrong as well (i.e. when equipping tools, the price should have been 300). Looking into this now.

bjorneg commented 3 years ago

Good catch, but the fix is insufficient as the amount is wrong as well (i.e. when equipping tools, the price should have been 300). Looking into this now.

With fix applied it says "Purchase 100 tools @3 Price 300". Seems OK to me.

mpope042 commented 3 years ago

Seems OK to me.

You are right. I was even more confused than normal with this area. Consequently I have added a more detailed comment and moved the sign change up to where the MarketWas is created in the hope of not being fooled again. Committed with credit in git.49f90f26 (sf).