Page MenuHomeWildfire Games

remove "Goods" from trade window stings
AcceptedPublic

Authored by Nescio on Sep 19 2019, 7:34 PM.

Details

Reviewers
Gallaecio
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

Resources should be referred to as “Resources”, not as “Goods”. This patch updates the Barter & Trade window to reflect that.

Test Plan

Check for mistakes and omissions.

Event Timeline

Nescio created this revision.Sep 19 2019, 7:34 PM
Owners added a subscriber: Restricted Owners Package.Sep 19 2019, 7:34 PM

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

bb added a subscriber: bb.Sep 22 2019, 4:41 PM

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

Still one case here, since the name isn't used in the code, we can simply remove the name tag

74

The string is too long for the gives size, so we need to resize some gui elements...

Nescio updated this revision to Diff 9922.Sep 22 2019, 9:05 PM
Nescio added inline comments.
binaries/data/mods/public/gui/session/trade_window.xml
74

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

Gallaecio accepted this revision.Sun, Oct 20, 1:09 PM

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.

This revision is now accepted and ready to land.Sun, Oct 20, 1:09 PM
Gallaecio added inline comments.Sun, Oct 20, 1:11 PM
binaries/data/mods/public/gui/session/trade_window.xml
70

I agree, I would remove the attribute altogether. We should only be using name attributes for a reason.

elexis added a subscriber: elexis.Sun, Oct 20, 1:12 PM

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,