Page MenuHomeWildfire Games

Basic implementation of extended restricted bartering support.
Needs ReviewPublic

Authored by Freagarach on Thu, Jun 6, 8:19 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Very basic implementation of support for a "currency"-system. (Petra-AI complies.)
This should be extended to something like:

Resource A can be bartered into [Resource B, Resource C].
Resource B can be bartered into [Resource A, Resource D].
Resource C can be bartered into [Resource D].
Resource D can be bartered into [Resource A, Resource B, Resource C].

for maximum moddability.
For now it is mainly a reference for @wowgetoffyourcellphone and @Nescio.

Test Plan

None yet, it only works after D1846 is commited and one gives the "currency"-property to a resource.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7807
Build 12705: Vulcan BuildJenkins
Build 12704: arc lint + arc unit

Event Timeline

Freagarach created this revision.Thu, Jun 6, 8:19 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Thu, Jun 6, 8:19 PM
Stan added a subscriber: Stan.Thu, Jun 6, 8:20 PM
Stan added inline comments.
binaries/data/mods/public/gui/session/menu.js
859

I guess you can inline that variable :)

Vulcan added a comment.Thu, Jun 6, 8:22 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before '='.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 428| 428| 	let candidate = { "gain": 0 };
| 429| 429| 	let potential = { "gain": 0 };
| 430| 430| 	let bestIndex = { "gain": 0 };
| 431|    |-	let bestLand  = { "gain": 0 };
|    | 431|+	let bestLand = { "gain": 0 };
| 432| 432| 
| 433| 433| 	let mapSize = gameState.sharedScript.mapSize;
| 434| 434| 	let traderTemplatesGains = gameState.getTraderTemplatesGains();
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 466| 466| 			if (m1.foundationProgress() === undefined && m2.foundationProgress() === undefined)
| 467| 467| 			{
| 468| 468| 				if (accessIndex)
| 469|    |-				{
|    | 469|+				
| 470| 470| 					if (gameState.ai.accessibility.regionType[accessIndex] == "water" && sea == accessIndex)
| 471| 471| 					{
| 472| 472| 						if (gain < bestIndex.gain)
| 485| 485| 							continue;
| 486| 486| 						bestLand = { "source": m1, "target": m2, "gain": gain, "land": land, "sea": sea };
| 487| 487| 					}
| 488|    |-				}
|    | 488|+				
| 489| 489| 				if (gain < candidate.gain)
| 490| 490| 					continue;
| 491| 491| 				candidate = { "source": m1, "target": m2, "gain": gain, "land": land, "sea": sea };
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 605| 605| 		return;	// position found, but not enough gain compared to our present route
| 606| 606| 
| 607| 607| 	if (this.Config.debug > 1)
| 608|    |-	{
|    | 608|+	
| 609| 609| 		if (this.potentialTradeRoute)
| 610| 610| 			API3.warn("turn " + gameState.ai.playedTurn + "we could have a new route with gain " +
| 611| 611| 				marketPos[3] + " instead of the present " + this.potentialTradeRoute.gain);
| 612| 612| 		else
| 613| 613| 			API3.warn("turn " + gameState.ai.playedTurn + "we could have a first route with gain " +
| 614| 614| 				marketPos[3]);
| 615|    |-	}
|    | 615|+	
| 616| 616| 
| 617| 617| 	if (!this.tradeRoute)
| 618| 618| 		gameState.ai.queueManager.changePriority("economicBuilding", 2*this.Config.priorities.economicBuilding);
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-in'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 668| 668| 
| 669| 669| 	let ret = {};
| 670| 670| 	for (let key in route)
| 671|    |-	{
|    | 671|+	
| 672| 672| 		if (key == "source" || key == "target")
| 673| 673| 		{
| 674| 674| 			if (!route[key])
| 677| 677| 		}
| 678| 678| 		else
| 679| 679| 			ret[key] = route[key];
| 680|    |-	}
|    | 680|+	
| 681| 681| 	return ret;
| 682| 682| };
| 683| 683| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-in'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 688| 688| 
| 689| 689| 	let ret = {};
| 690| 690| 	for (let key in route)
| 691|    |-	{
|    | 691|+	
| 692| 692| 		if (key == "source" || key == "target")
| 693| 693| 		{
| 694| 694| 			ret[key] = gameState.getEntityById(route[key]);
| 697| 697| 		}
| 698| 698| 		else
| 699| 699| 			ret[key] = route[key];
| 700|    |-	}
|    | 700|+	
| 701| 701| 	return ret;
| 702| 702| };
| 703| 703| 
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'for-in'.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/simulation/ai/petra/tradeManager.js
| 715| 715| m.TradeManager.prototype.Deserialize = function(gameState, data)
| 716| 716| {
| 717| 717| 	for (let key in data)
| 718|    |-	{
|    | 718|+	
| 719| 719| 		if (key == "tradeRoute" || key == "potentialTradeRoute")
| 720| 720| 			this[key] = this.routeIdToEnt(gameState, data[key]);
| 721| 721| 		else
| 722| 722| 			this[key] = data[key];
| 723|    |-	}
|    | 723|+	
| 724| 724| };
| 725| 725| 
| 726| 726| return m;
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/menu.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/session/menu.js
| 876| 876| 	Engine.GetGUIObjectByName("barterHelp").hidden = !canBarter;
| 877| 877| 
| 878| 878| 	if (canBarter)
| 879|    |-		g_ResourceData.GetCodes().forEach((resCode, i) => { barterUpdateCommon(resCode, i, "barter", g_ViewedPlayer) });
|    | 879|+		g_ResourceData.GetCodes().forEach((resCode, i) => { barterUpdateCommon(resCode, i, "barter", g_ViewedPlayer); });
| 880| 880| }
| 881| 881| 
| 882| 882| function getIdleLandTradersText(traderNumber)

binaries/data/mods/public/gui/session/menu.js
| 482| »   »   button.onPress·=·(function(player,·stance)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'stance' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 514| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 514| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'resCode' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 514| »   »   button.onPress·=·(function(i,·resCode,·button)·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 557| »   button.onPress·=·(function(i)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 623| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 623| »   button.onPress·=·(function(i,·button)·{·return·function()·{
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'button' is already declared in the upper scope.

binaries/data/mods/public/gui/session/menu.js
| 879| »   »   g_ResourceData.GetCodes().forEach((resCode,·i)·=>·{·barterUpdateCommon(resCode,·i,·"barter",·g_ViewedPlayer)·});
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1628/display/redirect

Freagarach added inline comments.Fri, Jun 7, 6:54 PM
binaries/data/mods/public/gui/session/menu.js
859

Could be replaced by

let currencyCodes = g_ResourceData.GetCodes("currency");
barterButton.Buy.hidden = currencyCodes.some(x => x == g_BarterSell) && isSelected || !currencyCodes.some(x => x == resourceCode);

;)
I agree :)

So I tried out incorporating D1846 and D1957 into my 0abc mod; silver has
"properties": [ "barterable", "currency", "tradable", "tributable" ],
the other four resources only [ "barterable", "tributable" ]. Also, silver, food, wood, stone, and metal have a "truePrice" of respectively 100, 20, 40, 60, 80 (to correspond to the fact they have different gather rates).
When launching a game, building a market, and opening the trade window, this is the result:


Most things seem to work (thank you!), but a few things go wrong:

  • the -100 numbers on top of the sell resources are not really meaningful; what we actually want to know is how much silver we get if we sell 100 of a resource
  • the barter and trade window width apparently only takes the tradable resources into account and overlooks the barterable resources need to be displayed as well
  • the position of the currency resource silver is a bit odd; perhaps we should have three rows: select currency (in case there are more than one), sell resource for currency, buy resource with currency
  • and a js error:

ERROR: JavaScript error: simulation/ai/petra/diplomacyManager.js line 412 SyntaxError: missing ; before statement InitGame@simulation/helpers/InitGame.js:57:1
ERROR: Failed to load script simulation/ai/petra/diplomacyManager.js
The line in question is:

requiredTribute = gameState.ai.HQ.pickMostNeededResources(gameState).find(res => tributableResources.indexOf(res.type) != -1));

What I don't know for sure is what causes the errors (D1846, D1957, me including them, some other file in my mod, or something missing in A23 compared to A24, something else).

Alternatively, keep the resource amounts fixed at 100, and display the currency equivalents:

     || 100 food  | 100 wood  | 100 stone | 100 iron
buy  || −a silver | −b silver | −c silver | −d silver
sell || +w silver | +x silver | +y silver | +z silver
In D1957#83317, @Nescio wrote:

What I don't know for sure is what causes the errors (D1846, D1957, me including them, some other file in my mod, or something missing in A23 compared to A24, something else).

Thanks for the report! I reproduced this and found the errors. Mentioned in D1846.

In D1957#83318, @Nescio wrote:

Alternatively, keep the resource amounts fixed at 100, and display the currency equivalents:

     || 100 food  | 100 wood  | 100 stone | 100 iron
buy  || −a silver | −b silver | −c silver | −d silver
sell || +w silver | +x silver | +y silver | +z silver

This would be nice, but won't work when this is extended to the length we want it to (specify for each resource what it can be exchanged to).
I think the system works as you want?
(...) what we actually want to know is how much silver we get if we sell 100 of a resource. If you select food to sell, you should get a number on the coin? If that is not the case, please report :)

Nescio added a comment.EditedWed, Jun 19, 8:46 PM

If you select food to sell, you should get a number on the coin? If that is not the case, please report :)

You're right, I mixed up, selling works fine. My problem is actually with the buying part. Instead of bartering a fixed 100 silver and getting +409 food in return, I want to buy 100 food and pay only the silver equivalent.
For comparison, if you go to the supermarket, you don't want to know how much bread you can get with €100, you want to know how much one bread will cost you.
Basically, there is a fundamental difference between a barter-based economy and a currency-based economy.

[EDIT] This patch doesn't really support currency, it supports restricted bartering, which works but is not exactly the same.

In D1957#83325, @Nescio wrote:

You're right, I mixed up, selling works fine. My problem is actually with the buying part. Instead of bartering a fixed 100 silver and getting +409 food in return, I want to buy 100 food and pay only the silver equivalent.
For comparison, if you go to the supermarket, you don't want to know how much bread you can get with €100, you want to know how much one bread will cost you.
Basically, there is a fundamental difference between a barter-based economy and a currency-based economy.
[EDIT] This patch doesn't really support currency, it supports restricted bartering, which works but is not exactly the same.

Ah, I understand what you mean!
That would indeed be a proper method. I'll rename this ;)

Freagarach retitled this revision from Basic implementation of currency support. to Basic implementation of extended restricted bartering support..Wed, Jun 19, 9:09 PM
Freagarach edited the summary of this revision. (Show Details)

Another error occurs when the AI has built a market:

ERROR: JavaScript error: simulation/ai/petra/tradeManager.js line 240 ReferenceError: GetCodes is not defined m.TradeManager.prototype.performBarter@simulation/ai/petra/tradeManager.js:240:11 m.TradeManager.prototype.update@simulation/ai/petra/tradeManager.js:638:5 m.HQ.prototype.update@simulation/ai/petra/headquarters.js:2755:3 m.PetraBot.prototype.OnUpdate@simulation/ai/petra/_petrabot.js:119:3 m.BaseAI.prototype.HandleMessage@simulation/ai/common-api/baseAI.js:64:2

The offending line is:
if (!GetCodes("currency").some(x => x == sell || x == buy))