To allow future changes in the way trade gain is computed (either in vanilla game or mods), two functions have been added to globalscripts (one for the gain normalization, and one for its dependence with distance), and changes for them to be used by simulation and the ai are added.
The normalization function currently has only the map size as arguments, but others could be added if needed.
Details
- Reviewers
elexis - Commits
- rP20320: Unify trade gain computation between simulation and ai
Check that everything works as expected.
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
Build is green
Updating workspaces... FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’: FCollada/FCDocument/FCDLibrary.cpp:149:30: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0); ^ FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’: FCollada/FCDocument/FCDLibrary.cpp:150:34: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’: FCollada/FCDocument/FCDLibrary.cpp:151:27: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’: FCollada/FCDocument/FCDLibrary.cpp:152:31: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’: FCollada/FCDocument/FCDLibrary.cpp:153:27: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’: FCollada/FCDocument/FCDLibrary.cpp:154:28: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’: FCollada/FCDocument/FCDLibrary.cpp:155:31: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’: FCollada/FCDocument/FCDLibrary.cpp:156:29: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’: FCollada/FCDocument/FCDLibrary.cpp:157:26: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’: FCollada/FCDocument/FCDLibrary.cpp:158:26: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’: FCollada/FCDocument/FCDLibrary.cpp:159:29: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’: FCollada/FCDocument/FCDLibrary.cpp:160:30: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’: FCollada/FCDocument/FCDLibrary.cpp:161:33: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’: FCollada/FCDocument/FCDLibrary.cpp:162:36: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ s
http://jenkins-master:8080/job/phabricator/2130/ for more details.
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/simulation/ai/petra/headquarters.js | 398| » » » let·ratioMax·=·0.70·+·randFloat(0.,·0.1); | | [NORMAL] JSHintBear: | | A trailing decimal point can be confused with a dot: '0.'. binaries/data/mods/public/simulation/ai/petra/headquarters.js | 403| » » » » ratioMax·=·0.85·+·randFloat(0.,·0.1); | | [NORMAL] JSHintBear: | | A trailing decimal point can be confused with a dot: '0.'. binaries/data/mods/public/simulation/ai/petra/headquarters.js |1454| » » if·(highLevel·==·0·||·lowLevel·>·1) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/simulation/ai/petra/headquarters.js |2384| » » if·(gameState.ai.playedTurn·%·4·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section XML GUI...
http://jenkins-master:8080/job/phabricator_lint/601/ for more details.
Agree with the patch, it is needed since we want to modify the train gain.
Agree with the new globalscripts file (would be nice to split globalscripts to 0ad logic and 0ad-agnostic logic some day, but currently it already contains much of both).
binaries/data/mods/public/globalscripts/Trade.js | ||
---|---|---|
2 ↗ | (On Diff #3903) | 320 is easier to test ingame, but 1024 will be easier to handle from a formula point of view, so ack |
15 ↗ | (On Diff #3903) | (perhaps distanceSquared to make it slightly quicker to read) |
binaries/data/mods/public/simulation/ai/petra/headquarters.js | ||
1108 ↗ | (On Diff #3903) | . |
1181 ↗ | (On Diff #3903) | Do we actually need the two new variables? Seems like all of this could be squased to: maxVal = Math.max(maxVal, gainMultiplier * NormalizedTradeGain(API3.SquareVectorDistance(market.position(), pos))); |
binaries/data/mods/public/globalscripts/Trade.js | ||
---|---|---|
15 ↗ | (On Diff #3903) | ok |
binaries/data/mods/public/simulation/ai/petra/headquarters.js | ||
1181 ↗ | (On Diff #3903) | The problem is that this is called really a lot when we look for the best position for a new market (all possible positions are tested vs all other existing markets), so calling each time NormalizedTradeGain would be really bad performance wise, specially if that function becomes more involved (using sqrt or tanh or whatever). |
Build is green
Updating workspaces... FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’: FCollada/FCDocument/FCDLibrary.cpp:149:30: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0); ^ FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’: FCollada/FCDocument/FCDLibrary.cpp:150:34: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’: FCollada/FCDocument/FCDLibrary.cpp:151:27: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’: FCollada/FCDocument/FCDLibrary.cpp:152:31: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’: FCollada/FCDocument/FCDLibrary.cpp:153:27: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’: FCollada/FCDocument/FCDLibrary.cpp:154:28: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’: FCollada/FCDocument/FCDLibrary.cpp:155:31: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’: FCollada/FCDocument/FCDLibrary.cpp:156:29: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’: FCollada/FCDocument/FCDLibrary.cpp:157:26: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’: FCollada/FCDocument/FCDLibrary.cpp:158:26: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’: FCollada/FCDocument/FCDLibrary.cpp:159:29: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’: FCollada/FCDocument/FCDLibrary.cpp:160:30: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’: FCollada/FCDocument/FCDLibrary.cpp:161:33: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable] FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’: FCollada/FCDocument/FCDLibrary.cpp:162:36: required from here FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ s
http://jenkins-master:8080/job/phabricator/2131/ for more details.
Executing section Default... Executing section Source... Executing section JS... binaries/data/mods/public/simulation/ai/petra/headquarters.js | 398| » » » let·ratioMax·=·0.70·+·randFloat(0.,·0.1); | | [NORMAL] JSHintBear: | | A trailing decimal point can be confused with a dot: '0.'. binaries/data/mods/public/simulation/ai/petra/headquarters.js | 403| » » » » ratioMax·=·0.85·+·randFloat(0.,·0.1); | | [NORMAL] JSHintBear: | | A trailing decimal point can be confused with a dot: '0.'. binaries/data/mods/public/simulation/ai/petra/headquarters.js |1454| » » if·(highLevel·==·0·||·lowLevel·>·1) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. binaries/data/mods/public/simulation/ai/petra/headquarters.js |2384| » » if·(gameState.ai.playedTurn·%·4·==·0) | | [NORMAL] JSHintBear: | | Use '===' to compare with '0'. Executing section XML GUI...
http://jenkins-master:8080/job/phabricator_lint/602/ for more details.
Code correct according to my reading of it.
If the AI part is complete, then this iteration of the patch should be complete too since the GUI pulls the trade gain through the GUI interface which gets it from the Trader component.
(Didn't test.)
binaries/data/mods/public/simulation/ai/petra/headquarters.js | ||
---|---|---|
1181 ↗ | (On Diff #3903) | I see. |