rP22964 nuked a round causing the recent bug where traders have huge values displayed.
Details
- Reviewers
wraitii - Commits
- rP23016: Add missing round when calculating trader gain, removed in rP22964
- Trac Tickets
- #5605
Test that everything is back to normal.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/315/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/830/display/redirect
Indeed however a level 0 means the markets are too close, and if we set the gain to 1 that would possibly mean that given two very close markets one could generate a huge income quickly, not unlike dancing.
In that regard using Math.Ceil() would be bad, and Math.Floor() wouldn't change things much, so only reverting back to Math.Round() sounds sane.
When trade is 0, this means it's impossible to set up trade between the two markets, without notification, which seems to be compatible with what existed before.
Thanks for the patch.
! In D2327#97705, @elexis wrote:
Loot too
I definitely saw a missing-round number in the summary screen too in one svn match (refs #5605).
I see Loot.js has a Math.floor already, so it were only looted trade carts that showed non-rounded numbers in the summary screen?
Looter.js doesn't seem to have rounding
kush_hero_nastasen_1.json has
{ "value": "Looter/Resource/food", "multiply": 1.5 },
If ApplyValueModificationsToEntity doesn't round and Looter.prototype.Collect doesn't round, it sounds like it might still occur (whichever commit it was)? anyway
Just to be clear, I'm talking about Loot, not trader gain.
There are more than one way to obtain Loot.
So unless you tested that all loot in the summary screen of that replay was attributed to trade carts, then you don't know.
Other than that I also spoke of the game I saw, not the game bb had uploaded, which I just uploaded at https://trac.wildfiregames.com/attachment/ticket/5605/commands.zip
But there wasnt a Kushite hero so I guess but dont actually know that it were trade carts in that replay too.
Now I've tried a replay and attacked soldiers who gathered, but didn't reproduce.
(Until I've seen the round in the code, I will have to suspect that it might be missing, regardless which commit.)
So doing one replay, I find a unit motion bug.
Doing another replay, I found summary screen still shows decimal numbers.
So I really wonder how you can say yes it was a trade cart, and yes it was tested and fixed when it's not, nor was it considered in the code review.
I mentioned it in the ticket and in the revision proposal before the commit, and after the commit.