Resources should be referred to as “Resources”, not as “Goods”. This patch updates the Barter & Trade window to reflect that.
Details
- Reviewers
Gallaecio - Group Reviewers
Restricted Owners Package (Owns No Changed Paths) - Commits
- rP23177: remove "Goods" from trade window stings
Check for mistakes and omissions.
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 failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/764/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/252/display/redirect
String here seems to originate from rP14417, for me "Resources" is indeed an improvement. One might want to nuke "goods" in the code too
binaries/data/mods/public/gui/session/trade_window.xml | ||
---|---|---|
70 ↗ | (On Diff #9866) | Still one case here, since the name isn't used in the code, we can simply remove the name tag |
74 ↗ | (On Diff #9866) | The string is too long for the gives size, so we need to resize some gui elements... |
binaries/data/mods/public/gui/session/trade_window.xml | ||
---|---|---|
74 ↗ | (On Diff #9866) | Good point! Let's add 20 pixels: |
By the way, why does it say <repeat count="10"> in line 35 but <repeat count="8"> in line 72?
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/805/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/290/display/redirect
I’m not against the use of ‘goods’ in the context of trade, where it’s the right word to refer to “tradeable resources”, but since in 0 A.D. all resources are tradeable, I’m OK with the change.
binaries/data/mods/public/gui/session/trade_window.xml | ||
---|---|---|
70 ↗ | (On Diff #9866) | I agree, I would remove the attribute altogether. We should only be using name attributes for a reason. |
The XML file was moved to session/trade/ and the barter and trade panel of the dialog were moved to individual files in rP23072. The values didn't change if Im not mistaken.
(Also notice that the code still uses the wording goods:)
Trader.js: this.goods = { Trader.js: this.goods.amount = this.CalculateGain(this.markets[0], this.markets[1]); Trader.js: // Drop carried goods if markets were changed Trader.js: this.goods.amount = null; Trader.js: cmpPlayer.AddResource(this.goods.type, gain); Trader.js: this.AddResources(this.entity, this.goods.amount.traderGain); Trader.js: if (this.goods.amount.market1Gain) Trader.js: this.AddResources(currentMarket, this.goods.amount.market1Gain); Trader.js: if (this.goods.amount.market2Gain) Trader.js: this.AddResources(nextMarket, this.goods.amount.market2Gain); Trader.js: this.goods.amount = null; Trader.js: if (this.goods.amount && this.goods.amount.traderGain) Trader.js: this.goods.type = cmpPlayer.GetNextTradingGoods(); Trader.js: this.goods.amount = this.CalculateGain(currentMarket, nextMarket); Trader.js: this.goods.origin = currentMarket; Trader.js:Trader.prototype.GetGoods = function() Trader.js: return this.goods; Trader.js: this.goods.amount = null; Trader.js: this.goods.amount = this.CalculateGain(this.markets[0], this.markets[1]); UnitAI.js: let amount = cmpTrader.GetGoods().amount; Player.js: this.tradingGoods = []; // goods for next trade-route and its proba in % (the sum of probas must be 100) Player.js: // Trading goods probability in steps of 5 Player.js: this.tradingGoods.push({ Player.js: "goods": resTradeCodes[i], Player.js:Player.prototype.GetNextTradingGoods = function() Player.js: var last = this.tradingGoods.length - 1; Player.js: sumProba += this.tradingGoods[i].proba; Player.js: return this.tradingGoods[i].goods; Player.js: return this.tradingGoods[last].goods; Player.js:Player.prototype.GetTradingGoods = function() Player.js: var tradingGoods = {}; Player.js: for (let resource of this.tradingGoods) Player.js: tradingGoods[resource.goods] = resource.proba; Player.js: return tradingGoods; Player.js:Player.prototype.SetTradingGoods = function(tradingGoods) Player.js: for (let resource in tradingGoods) Player.js: if (resTradeCodes.indexOf(resource) == -1 || tradingGoods[resource] < 0) Player.js: error("Invalid trading goods: " + uneval(tradingGoods)); Player.js: sumProba += tradingGoods[resource]; Player.js: error("Invalid trading goods probability: " + uneval(sumProba)); Player.js: this.tradingGoods = []; Player.js: for (let resource in tradingGoods) Player.js: this.tradingGoods.push({ Player.js: "goods": resource, Player.js: "proba": tradingGoods[resource] Looter.js: cmpTrader && cmpTrader.GetGoods() GuiInterface.js: "goods": cmpTrader.GetGoods() GuiInterface.js: result.gain = cmpEntityTrader.GetGoods().amount; GuiInterface.js: "gain": cmpEntityTrader.GetGoods().amount, GuiInterface.js:GuiInterface.prototype.GetTradingGoods = function(player) GuiInterface.js: return QueryPlayerIDInterface(player).GetTradingGoods(); GuiInterface.js: "GetTradingGoods": 1,
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/624/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1140/display/redirect