Page MenuHomeWildfire Games
Differential D478

Barter modifications
ClosedPublic
Actions

Authored by fatherbushido on May 12 2017, 11:21 AM.

Details

Reviewers
mimo
Commits
rP19615: Add barter multiplier for barter prices. They can be altered by technologies or…
Summary

Use a factor for barter prices (not for barter prices difference which are common to all players).
We could use a factor only for buy prices or even something for all resources but while at it, use the more detailed thing.
Those prices are used when adding resources (and in the gui for preview).
Those factor are modifiable by techs or auras.
Those data are in Player component but could lie elsewhere. Though I find it was the best place.
If I am not wrong AI take care of that.
Add related tests.

Test Plan

./binaries/system/test

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

fatherbushido created this revision.May 12 2017, 11:21 AM
fatherbushido updated this revision to Diff 1868.May 12 2017, 11:23 AM
fatherbushido edited the test plan for this revision. (Show Details)

Revert player_athen template used for testing and another unrelated change.

fatherbushido edited the summary of this revision. (Show Details)May 12 2017, 11:23 AM
fatherbushido mentioned this in D480: Macedonian team bonus.May 12 2017, 12:14 PM
Vulcan added a subscriber: Vulcan.May 12 2017, 12:29 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1158/ for more details.

Harbormaster completed remote builds in B1664: Diff 1867.May 12 2017, 12:29 PM
Vulcan added a comment.May 12 2017, 1:24 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1159/ for more details.

Harbormaster completed remote builds in B1665: Diff 1868.May 12 2017, 1:25 PM
mimo added inline comments.May 19 2017, 7:53 PM
binaries/data/mods/public/simulation/components/Player.js
177 ↗(On Diff #1868)

would be better to do something analoguous to the spyCostMultiplier, i.e.
add this.barterMultiplier which is returned by GetBarterMultiplier
update it only when OnValueModification

to avoid 8 calls to ApplyValueModifications each time GetBarterMultiplier is called.

fatherbushido added a comment.May 19 2017, 8:10 PM

ok, as the component already listen to that, that caching seems good. Thx for the feedback.

fatherbushido updated this revision to Diff 2035.May 19 2017, 9:04 PM

Compute barter multiplier only when needed

fatherbushido planned changes to this revision.May 19 2017, 9:14 PM

grrr doesn't work anymore (for the modification part, I messed something or it doesn't take into account the modification as it arrives after for D480)

fatherbushido updated this revision to Diff 2038.May 19 2017, 9:49 PM

Compute also on init

fatherbushido updated this revision to Diff 2040.May 19 2017, 9:54 PM

stat test leftover

fatherbushido added inline comments.May 19 2017, 10:04 PM
binaries/data/mods/public/simulation/components/Player.js
65 ↗(On Diff #2040)

perhaps we can simply do this.barterMultiplier.buy[res] = +this.template.BarterMultiplier.Buy[res]
but I fear some weird case.

fatherbushido planned changes to this revision.May 19 2017, 10:10 PM
fatherbushido added inline comments.
binaries/data/mods/public/simulation/components/Player.js
770 ↗(On Diff #2040)

Need to change that

776 ↗(On Diff #2040)

Could also apply the modification to only the modified sub value but I don't know if it's a real gain.

fatherbushido added inline comments.May 19 2017, 10:15 PM
binaries/data/mods/public/simulation/components/Player.js
770 ↗(On Diff #2040)

well it's ok

fatherbushido requested review of this revision.May 19 2017, 10:16 PM
Vulcan added a comment.May 20 2017, 1:49 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1258/ for more details.

Harbormaster completed remote builds in B1811: Diff 2035.May 20 2017, 1:49 AM
Vulcan added a comment.May 20 2017, 4:08 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1261/ for more details.

Harbormaster completed remote builds in B1814: Diff 2038.May 20 2017, 4:08 AM
Vulcan added a comment.May 20 2017, 5:42 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1263/ for more details.

Harbormaster completed remote builds in B1816: Diff 2040.May 20 2017, 5:42 AM
fatherbushido updated this revision to Diff 2043.May 20 2017, 8:32 AM

Finally no need to compute on init

Vulcan added a comment.May 20 2017, 9:18 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/1264/ for more details.

Harbormaster completed remote builds in B1818: Diff 2043.May 20 2017, 9:18 AM
mimo accepted this revision.May 20 2017, 9:49 AM

Patch is fine, my only worry is if we do really need all these 8 multipliers, as i'm afraid too much flexibility could be confusing.
All the use cases i've in mind could be done with only modifying one set of multiplier (preferentially the Sell ones as in your mace team bonus patch) but i may miss some and i'm also fine with that patch.

binaries/data/mods/public/simulation/components/Player.js
776 ↗(On Diff #2040)

such changes should be very rare, so does not matter

This revision is now accepted and ready to land.May 20 2017, 9:49 AM
Closed by commit rP19615: Add barter multiplier for barter prices. They can be altered by technologies or… (authored by fatherbushido). · Explain WhyMay 20 2017, 8:02 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
gui/
session/
2 lines
simulation/
ai/
common-api/
4 lines
1 line
components/
13 lines
7 lines
29 lines
tests/
35 lines
24 lines
47 lines
templates/
special/
14 lines
14 lines

Diff 2054

View Options

ps/trunk/binaries/data/mods/public/gui/session/menu.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/gamestate.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/ai/common-api/shared.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/Barter.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/Player.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/tests/test_Barter.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/tests/test_GuiInterface.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/components/tests/test_Player.js

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/templates/special/player.xml

Loading...
View Options

ps/trunk/binaries/data/mods/public/simulation/templates/special/player_gaia.xml

Loading...
Wildfire Games · Phabricator