fix a wrong class (CitizenSoldier)
ClosedPublic
Actions

Authored by Nescio on Aug 20 2019, 9:54 AM.

Details

Reviewers

wraitii

Group Reviewers

Restricted Owners Package

(Owns No Changed Paths)

Commits

rP22809: Change CitizenSoldier class to Citizen and/or Soldier in auras and technologies.

Summary

Currently one aura uses the Citizen class, five others CitizenSoldier. This is inconsistent and potentially confusing. Moreover, as @fatherbushido correctly pointed out ( https://wildfiregames.com/forum/index.php?/topic/26834-answered-need-more-info-about-unit-and-building-classes/ ), CitizenSoldier is a non-visible class and should thus not be used in data files; the visible Citizen class ought to be used instead.
This patch corrects that:

CitizenSoldier → Citizen in five auras; descriptions updated accordingly
CitizenSoldier → Citizen Soldier in phase technologies; tooltips updated accordingly
CitizenSoldier → Citizen in gui (single occurrence, idle worker button)

Test Plan

Effectively nothing should change, because all units with the CitizenSoldier class also have the Citizen and Soldier classes.

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

/ps/trunk

Lint

Lint OK

Unit

No Unit Test Coverage

Build Status

Buildable 8986
Build 14724: Vulcan Build	Jenkins
Build 14723: arc lint + arc unit

Event Timeline

Nescio created this revision.Aug 20 2019, 9:54 AM

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

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/434/display/redirect

Harbormaster completed remote builds in B8934: Diff 9413.Aug 20 2019, 10:29 AM

wraitii added a reviewer: wraitii.Aug 20 2019, 11:57 AM

Nescio updated this revision to Diff 9429.Aug 21 2019, 1:04 PM

Nescio retitled this revision from fix a wrong class to fix a wrong class (CitizenSoldier).

Nescio edited the summary of this revision. (Show Details)

Nescio edited the test plan for this revision. (Show Details)

Nescio added a subscriber: fatherbushido.

Owners added a subscriber: Restricted Owners Package.Aug 21 2019, 1:04 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 6 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
| 392| 392| 				// Players see colors depending on diplomacy
| 393| 393| 				g_DisplayedPlayerColors[i] =
| 394| 394| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 395|    |-					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
|    | 395|+						g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 396| 396| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 397| 397| 					getDiplomacyColor("enemy");
| 398| 398| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 7 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
| 393| 393| 				g_DisplayedPlayerColors[i] =
| 394| 394| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 395| 395| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 396|    |-					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
|    | 396|+							g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 397| 397| 					getDiplomacyColor("enemy");
| 398| 398| 
| 399| 399| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 8 tabs but found 5.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
| 394| 394| 					g_ViewedPlayer == i ? getDiplomacyColor("self") :
| 395| 395| 					g_Players[g_ViewedPlayer].isAlly[i] ? getDiplomacyColor("ally") :
| 396| 396| 					g_Players[g_ViewedPlayer].isNeutral[i] ? getDiplomacyColor("neutral") :
| 397|    |-					getDiplomacyColor("enemy");
|    | 397|+								getDiplomacyColor("enemy");
| 398| 398| 
| 399| 399| 		g_DisplayedPlayerColors[0] = g_Players[0].color;
| 400| 400| 	}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
| 656| 656| 					"civ": setStringTags(g_CivData[g_Players[g_ViewedPlayer].civ].Name, { "font": "sans-bold-stroke-14" }),
| 657| 657| 					"hotkey_civinfo": colorizeHotkey("%(hotkey)s", "civinfo"),
| 658| 658| 					"hotkey_structree": colorizeHotkey("%(hotkey)s", "structree")
| 659|    |-			});
|    | 659|+				});
| 660| 660| 	}
| 661| 661| 
| 662| 662| 	// Following gaia can be interesting on scripted maps
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|1211|1211| 
|1212|1212| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1213|1213| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1214|    |-		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|    |1214|+			"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1215|1215| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1216|1216| 	});
|1217|1217| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|1212|1212| 	let orderHotkeyTooltip = Object.keys(viewablePlayerStates).length <= 1 ? "" :
|1213|1213| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1214|1214| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1215|    |-		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|    |1215|+			"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1216|1216| 	});
|1217|1217| 
|1218|1218| 	let resCodes = g_ResourceData.GetCodes();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 1.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/session.js
|1213|1213| 		"\n" + sprintf(translate("%(order)s: %(hotkey)s to change order."), {
|1214|1214| 		"hotkey": setStringTags("\\[Click]", g_HotkeyTags),
|1215|1215| 		"order": tooltipSort == 0 ? translate("Unordered") : tooltipSort == 1 ? translate("Descending") : translate("Ascending")
|1216|    |-	});
|    |1216|+		});
|1217|1217| 
|1218|1218| 	let resCodes = g_ResourceData.GetCodes();
|1219|1219| 	for (let r = 0; r < resCodes.length; ++r)

binaries/data/mods/public/gui/session/session.js
|1072| »   let·getPanelEntNameTooltip·=·panelEntState·=>·"[font=\"sans-bold-16\"]"·+·template.name.specific·+·"[/font]";
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'panelEntState' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1147| »   »   button.onpress·=·(function(i)·{·return·function()·{·performGroup((Engine.HotkeyIsPressed("selection.add")·?·"add"·:·"select"),·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1148| »   »   button.ondoublepress·=·(function(i)·{·return·function()·{·performGroup("snap",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.

binaries/data/mods/public/gui/session/session.js
|1149| »   »   button.onpressright·=·(function(i)·{·return·function()·{·performGroup("breakUp",·i);·};·})(i);
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'i' is already declared in the upper scope.
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/443/display/redirect

Harbormaster completed remote builds in B8986: Diff 9429.Aug 21 2019, 1:09 PM

Nescio edited the summary of this revision. (Show Details)Aug 21 2019, 1:11 PM

wraitii added inline comments.Aug 24 2019, 10:53 AM

binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18	Shouldn't this be `+`?

Nescio added inline comments.Aug 24 2019, 11:07 AM

binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18	No. `["X Y"]` means “X and Y” and `["X", "Y"]` means “X or Y”. See e.g. https://trac.wildfiregames.com/browser/ps/trunk/binaries/data/mods/public/simulation/data/technologies/upgrade_rank_advanced_cavalry.json

wraitii accepted this revision.Aug 24 2019, 12:26 PM

This revision is now accepted and ready to land.Aug 24 2019, 12:26 PM

Nescio added inline comments.Aug 24 2019, 12:35 PM

binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18	In the XML templates it's indeed the other way around: `X+Y` means “X and Y” and `X Y` means “X or Y” there. But these here are JSON data files, hence a different syntax.

bb added a subscriber: bb.Aug 30 2019, 5:25 PM

bb added inline comments.

binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18	To answer the first question: yes it should be `+`, at least conceptually. However the implementation here is weird and wants , changing it is still on my todo list...