Page MenuHomeWildfire Games

fix a wrong class (CitizenSoldier)
ClosedPublic

Authored by Nescio on Tue, Aug 20, 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:

  • CitizenSoldierCitizen in five auras; descriptions updated accordingly
  • CitizenSoldierCitizen Soldier in phase technologies; tooltips updated accordingly
  • CitizenSoldierCitizen 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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Nescio created this revision.Tue, Aug 20, 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

Nescio updated this revision to Diff 9429.Wed, Aug 21, 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.Wed, Aug 21, 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

Nescio edited the summary of this revision. (Show Details)Wed, Aug 21, 1:11 PM
wraitii added inline comments.Sat, Aug 24, 10:53 AM
binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18 ↗(On Diff #9429)

Shouldn't this be +?

Nescio added inline comments.Sat, Aug 24, 11:07 AM
binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18 ↗(On Diff #9429)
wraitii accepted this revision.Sat, Aug 24, 12:26 PM
This revision is now accepted and ready to land.Sat, Aug 24, 12:26 PM
Nescio added inline comments.Sat, Aug 24, 12:35 PM
binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18 ↗(On Diff #9429)

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.Fri, Aug 30, 5:25 PM
bb added inline comments.
binaries/data/mods/public/simulation/data/technologies/phase_town_generic.json
18 ↗(On Diff #9429)

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...