Page MenuHomeWildfire Games

Basic implementation of extended restricted bartering support.
Changes PlannedPublic

Authored by Freagarach on Jun 6 2019, 8:19 PM.

Details

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

Depends on D1846.
Very basic implementation of support for a "currency"-system. (Petra-AI complies.) All barterable resources can only be sold into the "currency"-resource(s).
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

Give 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.Jun 6 2019, 8:19 PM
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Jun 6 2019, 8:19 PM
Stan added a subscriber: Stan.Jun 6 2019, 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.Jun 6 2019, 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.Jun 7 2019, 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.EditedJun 19 2019, 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..Jun 19 2019, 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))

Freagarach planned changes to this revision.Jul 10 2019, 3:08 PM

Not going to work on this any time soon.

Stan added reviewers: Restricted Owners Package, Silier.Mar 16 2020, 10:44 AM
Freagarach removed reviewers: Restricted Owners Package, Silier.Mar 17 2020, 8:04 AM

Still not high on my priority list. The summary shows what it should be so it is not even close to reviewable.
I think the "properties" was - for now - a big enough leap forward :)

Stan added a comment.Mar 17 2020, 9:22 AM

Still not high on my priority list. The summary shows what it should be so it is not even close to reviewable.
I think the "properties" was - for now - a big enough leap forward :)

Still the reviewers were correct.

Sorry for removing them then.
IMHO An entry on a review list is like a question: "Would you like to review this, please?" But since that is not the case here I reckoned it is not fair to add them. My mistake then.

Stan added reviewers: Restricted Owners Package, Silier.Mar 17 2020, 9:28 AM

Sorry for removing them then.
IMHO An entry on a review list is like a question: "Would you like to review this, please?" But since that is not the case here I reckoned it is not fair to add them. My mistake then.

It's alright :)

Silier resigned from this revision.Jul 31 2023, 5:24 PM
Herald added a reviewer: Restricted Owners Package. · View Herald TranscriptJul 31 2023, 5:24 PM
Herald added a subscriber: Silier. · View Herald Transcript