Page MenuHomeWildfire Games

Generalise hard-counter tooltips.
Needs ReviewPublic

Authored by Freagarach on Dec 28 2018, 9:21 AM.

Details

Reviewers
elexis
Feldfeld
Stan
wraitii
Trac Tickets
#4130
Summary

This adds units counter information in unit tooltip in structure tree and in game, for each type of attack a unit can have (currently no units have multiple counters for different attack types but just in case).

Test Plan

Verify that the tooltips for counters are displayed correctly.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
nani awarded a token.Jul 15 2019, 2:39 PM

Wouldn't it be more logical to have the counter shown directly after the attack type? E.g. C-q attack: x hack, Rate: y second(s) (3x vs Classes)?

binaries/data/mods/public/globalscripts/Templates.js
237

.toFixed(x) perhaps? If upgrading of bunuses will be introduced.

Stan updated this revision to Diff 9386.Aug 18 2019, 7:57 PM
Stan marked 4 inline comments as done.
  • Add the two missing templates
  • Remove the object.keys call, and instead prepend the type bonus only if the body array has lenght
  • Use to fixed.
  • Fix encoding issue.
  • Use caching for the bonuses.
  • Remove the clone call.
binaries/data/mods/public/globalscripts/Templates.js
237

.toFixed(8) like used below.

binaries/data/mods/public/gui/common/tooltips.js
895

clip.exe not utf8 happens sometimes.

binaries/data/mods/public/simulation/components/Attack.js
492 ↗(On Diff #8366)

Well with the caching now it's duplicated. Enjoy.

Build failure - The Moirai have given mortals hearts that can endure.

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

An in-game screenshot would be nice.

binaries/data/mods/public/gui/common/tooltips.js
887

Shouldn't there be a s after %(multiplier)?
Also, ×.

binaries/data/mods/public/gui/reference/common/draw.js
17 ↗(On Diff #9386)

Do bonus attacks apply to splash damage as well? If not, shouldn't counters be above then?

binaries/data/mods/public/simulation/templates/structures/athen_defense_tower.xml
16 ↗(On Diff #9386)

?

23 ↗(On Diff #9386)

?

Stan marked 2 inline comments as done.Aug 18 2019, 8:21 PM
Stan added inline comments.
binaries/data/mods/public/gui/common/tooltips.js
887

Right

binaries/data/mods/public/simulation/templates/structures/athen_defense_tower.xml
16 ↗(On Diff #9386)

That's for testing will be removed when comitting

23 ↗(On Diff #9386)

Encoding error wont get committed.

Stan marked 2 inline comments as done.Aug 18 2019, 9:27 PM
Stan added inline comments.
binaries/data/mods/public/gui/reference/common/draw.js
17 ↗(On Diff #9386)

It's unrelated :)

Stan updated this revision to Diff 9390.Aug 18 2019, 9:37 PM
  • Use the special char instead of vs
  • Add missing s
Stan updated this revision to Diff 9391.Aug 18 2019, 9:39 PM
Stan marked 2 inline comments as done.

Build failure - The Moirai have given mortals hearts that can endure.

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

Build failure - The Moirai have given mortals hearts that can endure.

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

smiley removed a subscriber: smiley.Aug 18 2019, 9:41 PM

Use the special char instead of vs

Removing the versus makes the text ambiguous. I'd strongly recommend to stick with the old format, e.g. "1.5× vs Cavalry".

This adds units counter information in unit tooltip in structure tree and in game, for each types of attack a unit can have (currently no units have multiple counters for different attack types but just in case).
Adds a new line for each counter an attack can have.

What happens if an unit has multiple bonuses vs the same class? E.g. infantry spearman has "2.0× vs Cavalry" of its own and inherets "1.5× vs Cavalry" from its parent. Are they displayed separatedly, or combined into a single entry ("3.0× vs Cavalry".)?

binaries/data/mods/public/gui/reference/common/draw.js
17 ↗(On Diff #9386)

In that case I think it would be better to order them as:

	getAttackTooltip,
	getCountersTooltip,
	getSplashDamageTooltip,
Stan updated this revision to Diff 9396.EditedAug 19 2019, 10:14 AM
  • Remove two unneeded files.
  • Fix broken code.
  • Re-add vs.

What happens if an unit has multiple bonuses vs the same class? E.g. infantry spearman has "2.0× vs Cavalry" of its own and inherets "1.5× vs Cavalry" from its parent. Are they displayed separatedly, or combined into a single entry ("3.0× vs Cavalry".)?

The current counter of spearmen displayed is Counter 3x vs Cavalry

binaries/data/mods/public/gui/reference/common/draw.js
17 ↗(On Diff #9386)

Why ? Because it's alphabetical ?

Build failure - The Moirai have given mortals hearts that can endure.

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

The current counter of spearmen displayed is Counter 3x vs Cavalry

Yeah, I know, but that's because the spearman has only one bonus attack. I'm wondering what happens if it would have two bonus attacks vs Cavalry instead.

binaries/data/mods/public/gui/reference/common/draw.js
17 ↗(On Diff #9386)

No, because counters apply to attack and should therefore follow them directly to minimize confusion.

Stan updated this revision to Diff 9400.Aug 19 2019, 11:05 AM
Stan marked 2 inline comments as done.

Fix caching
Move the tooltip up.

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
| 217| »   »   »   »   for·(let·bonus·in·template.Attack[type].Bonuses)·{
|    | [NORMAL] ESLintBear (brace-rules/brace-on-same-line):
|    | Opening curly brace appears on the same line as controlling statement.

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 389| 389| function getRepairTimeTooltip(entState)
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 392|+		"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393|    |-			"details": entState.repairable.numBuilders
|    | 393|+		"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 394|+	}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
| 397| 397| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 411| 411| function getBuildTimeTooltip(entState)
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414|    |-			"label": headerFont(translate("Number of builders:")),
|    | 414|+		"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415|    |-			"details": entState.foundation.numBuilders
|    | 415|+		"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 416|+	}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
| 419| 419| 			"Add another worker to speed up the construction by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
| 807| 807| 		updateEntityColor(data.showAllStatusBars && (i == player || player == -1) ?
| 808| 808| 			[IID_Minimap, IID_RangeOverlayRenderer, IID_RallyPointRenderer, IID_StatusBars] :
| 809| 809| 			[IID_Minimap, IID_RangeOverlayRenderer, IID_RallyPointRenderer],
| 810|    |-			cmpRangeManager.GetEntitiesByPlayer(i));
|    | 810|+		cmpRangeManager.GetEntitiesByPlayer(i));
| 811| 811| 	}
| 812| 812| 	updateEntityColor([IID_Selectable, IID_StatusBars], data.selected);
| 813| 813| 	Engine.QueryInterface(SYSTEM_ENTITY, IID_TerritoryManager).UpdateColors();
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1640|1640| 			{
|1641|1641| 				minDist2 = dist2;
|1642|1642| 				minDistEntitySnapData = {
|1643|    |-						"x": pos.x,
|    |1643|+					"x": pos.x,
|1644|1644| 						"z": pos.z,
|1645|1645| 						"angle": cmpPosition.GetRotation().y,
|1646|1646| 						"ent": ent
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1641|1641| 				minDist2 = dist2;
|1642|1642| 				minDistEntitySnapData = {
|1643|1643| 						"x": pos.x,
|1644|    |-						"z": pos.z,
|    |1644|+					"z": pos.z,
|1645|1645| 						"angle": cmpPosition.GetRotation().y,
|1646|1646| 						"ent": ent
|1647|1647| 				};
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1642|1642| 				minDistEntitySnapData = {
|1643|1643| 						"x": pos.x,
|1644|1644| 						"z": pos.z,
|1645|    |-						"angle": cmpPosition.GetRotation().y,
|    |1645|+					"angle": cmpPosition.GetRotation().y,
|1646|1646| 						"ent": ent
|1647|1647| 				};
|1648|1648| 			}
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 5 tabs but found 6.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/GuiInterface.js
|1643|1643| 						"x": pos.x,
|1644|1644| 						"z": pos.z,
|1645|1645| 						"angle": cmpPosition.GetRotation().y,
|1646|    |-						"ent": ent
|    |1646|+					"ent": ent
|1647|1647| 				};
|1648|1648| 			}
|1649|1649| 		}
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
| 763| 763| 				addResearchToQueue(data.item.researchFacilityId, t);
| 764| 764| 			})(tech);
| 765| 765| 
| 766|    |-			button.onPressRight = (t => function () {
|    | 766|+			button.onPressRight = (t => function() {
| 767| 767| 				showTemplateDetails(
| 768| 768| 					t,
| 769| 769| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);

binaries/data/mods/public/gui/session/selection_panels.js
|  48| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  59| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 731| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 522| 522|  * @param {object} template - The entity's attack template object.
| 523| 523|  * @param {string} type - The attack type.
| 524| 524|  */
| 525|    |-Attack.prototype.RefreshBonusTemplate = function (template, type)
|    | 525|+Attack.prototype.RefreshBonusTemplate = function(template, type)
| 526| 526| {
| 527| 527| 	this.bonuses[type] = {};
| 528| 528| 	for (let bonusKey in template.Bonuses)
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 532| 532| 			"Civ": bonus.Civ || undefined,
| 533| 533| 			"Classes": bonus.Classes,
| 534| 534| 			"Multiplier": ApplyValueModificationsToEntity("Attack/" + type + "/Bonuses/" + bonusKey + "/Multiplier", +bonus.Multiplier, this.entity)
| 535|    |-		}
|    | 535|+		};
| 536| 536| 	}
| 537| 537| }
| 538| 538| 
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 534| 534| 			"Multiplier": ApplyValueModificationsToEntity("Attack/" + type + "/Bonuses/" + bonusKey + "/Multiplier", +bonus.Multiplier, this.entity)
| 535| 535| 		}
| 536| 536| 	}
| 537|    |-}
|    | 537|+};
| 538| 538| 
| 539| 539| /**
| 540| 540|  * Attack the target entity. This should only be called after a successful range check,
|    | [NORMAL] ESLintBear (spaced-comment):
|    | Expected space or tab after '//' in comment.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 557| 557| 
| 558| 558| 		let horizSpeed = +this.template[type].Projectile.Speed;
| 559| 559| 		let gravity = +this.template[type].Projectile.Gravity;
| 560|    |-		//horizSpeed /= 2; gravity /= 2; // slow it down for testing
|    | 560|+		// horizSpeed /= 2; gravity /= 2; // slow it down for testing
| 561| 561| 
| 562| 562| 		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
| 563| 563| 		if (!cmpPosition || !cmpPosition.IsInWorld())
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/simulation/components/Attack.js
| 735| 735| 		cmpUnitAI.UpdateRangeQueries();
| 736| 736| 
| 737| 737| 	let counters = attackTypes.filter(type =>
| 738|    |-		msg.valueNames.indexOf("Attack/" + type + "/Multiplier") != -1)
|    | 738|+		msg.valueNames.indexOf("Attack/" + type + "/Multiplier") != -1);
| 739| 739| 
| 740| 740| 	if (!counters.length)
| 741| 741| 		return;

binaries/data/mods/public/simulation/components/Attack.js
| 547| ·»   let·cmpDamage·=·Engine.QueryInterface(SYSTEM_ENTITY,·IID_Damage);
|    | [NORMAL] ESLintBear (no-mixed-spaces-and-tabs):
|    | Mixed spaces and tabs.

binaries/data/mods/public/simulation/components/Attack.js
| 535| »   »   }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Attack.js
| 537| }
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

binaries/data/mods/public/simulation/components/Attack.js
| 651| »   »   cmpTimer.SetTimeout(SYSTEM_ENTITY,·IID_Damage,·"MissileHit",·timeToTarget·*·1000·+·+this.template[type].Delay,·data);
|    | [NORMAL] JSHintBear:
|    | Confusing plusses.

binaries/data/mods/public/simulation/components/Attack.js
| 738| »   »   msg.valueNames.indexOf("Attack/"·+·type·+·"/Multiplier")·!=·-1)
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

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

Stan added a comment.Aug 19 2019, 11:14 AM

In case of duplicate bonuses it does the following given this template:

<Attack>
  <Melee>
    <Damage>
      <Hack>3.0</Hack>
      <Pierce>2.5</Pierce>
    </Damage>
    <MaxRange>4.5</MaxRange>
    <Bonuses>
      <BonusCavMelee>
        <Classes>Cavalry</Classes>
        <Multiplier>3.0</Multiplier>
      </BonusCavMelee>
      <BonusCavMelee2>
        <Classes>Cavalry</Classes>
        <Multiplier>2.5</Multiplier>
      </BonusCavMelee2>
    </Bonuses>
  </Melee>
</Attack>

binaries/data/mods/public/gui/reference/common/draw.js
17 ↗(On Diff #9386)

Okay. It seems they also apply to splash

In case of duplicate bonuses it does the following given this template:

Thanks for answering, that's what I wanted to know! I suppose it's too complicated to combine the 3.0× and 2.5× into a single 7.5×?

Stan added a comment.Aug 19 2019, 11:33 AM
In D1707#91216, @Nescio wrote:

In case of duplicate bonuses it does the following given this template:

Thanks for answering, that's what I wanted to know! I suppose it's too complicated to combine the 3.0× and 2.5× into a single 7.5×?

Does it work that way ? I figured it would take one of them, not both ?

Does it work that way ? I figured it would take one of them, not both ?

Why would it take only one? I don't know for sure, but I always assumed all bonus attacks were taken into account. E.g. if a siege engine would have three bonus attacks, 2× vs Defensive, 2× vs StoneWall, and 2× vs Gates, shouldn't it do 8× vs city wall gates (because those have all three classes)?

Stan added a comment.Aug 19 2019, 11:57 AM
In D1707#91223, @Nescio wrote:

Does it work that way ? I figured it would take one of them, not both ?

Why would it take only one?

Because here it's Cavalry and Cavalry ?

I don't know for sure, but I always assumed all bonus attacks were taken into account. E.g. if a siege engine would have three bonus attacks, 2× vs Defensive, 2× vs StoneWall, and 2× vs Gates, shouldn't it do 8× vs city wall gates (because those have all three classes)?

Do you have some time to try ?

Do you have some time to try ?

To test I changed (in an A23 mod) the battering attack to:

<Attack>
  <Melee>
    <Bonuses>
      <Defensive>
        <Classes>Defensive</Classes>
        <Multiplier>2.0</Multiplier>
      </Defensive>
      <StoneWall>
        <Classes>StoneWall</Classes>
        <Multiplier>2.0</Multiplier>
      </StoneWall>
      <Gates>
        <Classes>Gates</Classes>
        <Multiplier>2.0</Multiplier>
      </Gates>
    </Bonuses>
    <Crush>100.0</Crush>
    <MaxRange>7</MaxRange>
    <PreferredClasses datatype="tokens">Gates</PreferredClasses>
    <PrepareTime>1500</PrepareTime>
    <RepeatTime>5000</RepeatTime>
  </Melee>
</Attack>

and launched a “Fortress” random map. Result: rams inflict 8× damage vs city wall gates and 4× vs other city wall pieces.
Then I changed the attack again, to:

<Attack>
  <Melee>
    <Bonuses>
      <Defensive>
        <Classes>Defensive</Classes>
        <Multiplier>2.0</Multiplier>
      </Defensive>
      <Structures>
        <Classes>Defensive</Classes>
        <Multiplier>2.0</Multiplier>
      </Structures>
    </Bonuses>
    <Crush>100.0</Crush>
    <MaxRange>7</MaxRange>
    <PreferredClasses datatype="tokens">Gates</PreferredClasses>
    <PrepareTime>1500</PrepareTime>
    <RepeatTime>5000</RepeatTime>
  </Melee>
</Attack>

Result: rams inflict 4× damage, as expected.

Stan added a comment.Aug 19 2019, 7:49 PM

Thanks for testing. Then indeed it would be nice to group them if it's not too complex I'll do it.

https://www.consolelog.io/group-by-in-javascript/

Stan added a comment.Aug 19 2019, 9:03 PM

It's kinda more complex because there can be multiple classes. So I'm not sure I can pull this off...

In D1707#91288, @Stan wrote:

It's kinda more complex because there can be multiple classes. So I'm not sure I can pull this off...

Is it really necessary? It would bring confusion with:

<Attack>
  <Melee>
    <Bonuses>
      <Defensive>
        <Classes>Defensive</Classes>
        <Multiplier>2.0</Multiplier>
      </Defensive>
      <StoneWall>
        <Classes>StoneWall</Classes>
        <Multiplier>2.0</Multiplier>
      </StoneWall>
      <Gates>
        <Classes>Gates</Classes>
        <Multiplier>2.0</Multiplier>
      </Gates>
    </Bonuses>
(,,,)
  </Melee>
</Attack>

these kind of bonuses, if it would read (2x vs Defensive, 4x vs StoneWall, 8x vs Gates), for if one knows (which is logical IMHO) that Gates is a subclass of StoneWall and both are subclasses of Defensive one could easily assume that a ram does 2x4x8 damage vs Gates?

Wouldn't it be more logical to have the counter shown directly after the attack type? E.g. C-q attack: x hack, Rate: y second(s) (3x vs Classes)?

I still think this could be viable.
I'll explain: I want to quickly check the attack of my soldiers and perhaps any bonuses it has. If I only need to read one line (namely the melee-attack (or ranged)) and the bonuses are already on that line, I've got the information I want. If it is on a seperate line, I have to read the melee-attack line and scan all subsequent lines (now only Capture, but still) until I arrive at the melee bonus line (if there is any). When secondary attacks are implemented, I even have to correlate the name of the bonus(es) to the attack I wanted to check.
This is just a suggestion, if there is any reason not to do this, I'm okay with that :)

Stan added a comment.Aug 29 2019, 10:33 AM

Wouldn't it be more logical to have the counter shown directly after the attack type? E.g. C-q attack: x hack, Rate: y second(s) (3x vs Classes)?

I still think this could be viable.
I'll explain: I want to quickly check the attack of my soldiers and perhaps any bonuses it has. If I only need to read one line (namely the melee-attack (or ranged)) and the bonuses are already on that line, I've got the information I want. If it is on a seperate line, I have to read the melee-attack line and scan all subsequent lines (now only Capture, but still) until I arrive at the melee bonus line (if there is any). When secondary attacks are implemented, I even have to correlate the name of the bonus(es) to the attack I wanted to check.
This is just a suggestion, if there is any reason not to do this, I'm okay with that :)

Makes sense to me would you like to commandeer?

In D1707#92783, @Stan wrote:

Makes sense to me would you like to commandeer?

Wouldn't you mind? It'll take some time though, before I can work this. I'll flag it for myself and update when possible. Probably I should wait for D2138.

Stan added a comment.Aug 29 2019, 5:16 PM
In D1707#92783, @Stan wrote:

Makes sense to me would you like to commandeer?

Wouldn't you mind? It'll take some time though, before I can work this. I'll flag it for myself and update when possible. Probably I should wait for D2138.

Nah no problem, I actually took it from @Feldfeld as long as everyone gets credits it's fine :)

In D1707#92805, @Stan wrote:

Nah no problem, I actually took it from @Feldfeld as long as everyone gets credits it's fine :)

Yeah, I saw that, but (s)he is a *bit* less active than you ;)

Stan added a comment.Aug 29 2019, 5:38 PM

All that matters is that it gets committed and the best quality possible :) The medium is not really important :D

wraitii commandeered this revision.Sep 1 2019, 11:45 AM
wraitii updated this revision to Diff 9572.
wraitii edited reviewers, added: Stan; removed: wraitii.

Rebase on top of D2092, which passed attack bonuses as parts of attack effects.

I've profiled this on Combat-Demo Huge, by selecting the maximum amount of units, it takes 100ms the first time, and then an average of 5ms per Gui update.

This is not negligible, but it's right in that region where maybe it doesn't actually need to be cached ?

Anybody else - feel free to commandeer this.

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/31/display/redirect

wraitii added inline comments.Sep 1 2019, 11:50 AM
binaries/data/mods/public/gui/common/tooltips.js
870

Accidentally left this in

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 389| 389| function getRepairTimeTooltip(entState)
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 392|+		"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 390| 390| {
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393|    |-			"details": entState.repairable.numBuilders
|    | 393|+		"details": entState.repairable.numBuilders
| 394| 394| 		}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 391| 391| 	return sprintf(translate("%(label)s %(details)s"), {
| 392| 392| 			"label": headerFont(translate("Number of repairers:")),
| 393| 393| 			"details": entState.repairable.numBuilders
| 394|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 394|+	}) + "\n" + (entState.repairable.numBuilders ?
| 395| 395| 		sprintf(translatePlural(
| 396| 396| 			"Add another worker to speed up the repairs by %(second)s second.",
| 397| 397| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 411| 411| function getBuildTimeTooltip(entState)
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414|    |-			"label": headerFont(translate("Number of builders:")),
|    | 414|+		"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 412| 412| {
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415|    |-			"details": entState.foundation.numBuilders
|    | 415|+		"details": entState.foundation.numBuilders
| 416| 416| 		}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 413| 413| 	return sprintf(translate("%(label)s %(details)s"), {
| 414| 414| 			"label": headerFont(translate("Number of builders:")),
| 415| 415| 			"details": entState.foundation.numBuilders
| 416|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 416|+	}) + "\n" + (entState.foundation.numBuilders ?
| 417| 417| 		sprintf(translatePlural(
| 418| 418| 			"Add another worker to speed up the construction by %(second)s second.",
| 419| 419| 			"Add another worker to speed up the construction by %(second)s seconds.",
|    | [NORMAL] ESLintBear (space-before-function-paren):
|    | Unexpected space before function parentheses.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/session/selection_panels.js
| 763| 763| 				addResearchToQueue(data.item.researchFacilityId, t);
| 764| 764| 			})(tech);
| 765| 765| 
| 766|    |-			button.onPressRight = (t => function () {
|    | 766|+			button.onPressRight = (t => function() {
| 767| 767| 				showTemplateDetails(
| 768| 768| 					t,
| 769| 769| 					GetTemplateData(data.unitEntStates.find(state => state.id == data.item.researchFacilityId).template).nativeCiv);

binaries/data/mods/public/gui/session/selection_panels.js
|  48| »   »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
|  59| »   »   switch·(data.item)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.

binaries/data/mods/public/gui/session/selection_panels.js
| 731| »   »   »   »   »   »   switch·(entity.check)
|    | [NORMAL] ESLintBear (default-case):
|    | Expected a default case.
Executing section cli...

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

Nescio removed a subscriber: Nescio.Sep 1 2019, 11:53 AM

Given the work at D2138 / D2218, which will add even more stuff to the attack tooltips, I think we have to consider redoing them entirely in context, sometimes after this patch.
It would probably be beneficial to take more whitespace for clarity, such that the tooltip would read something like:

Melee Attack:
   Damage: 5 Hack, 3 Pierce
   Rate: 0.8s
   Bonuses: 3x against Infantry, 4x against Whatever
Capture Attack:
   Capture: 4
   Rate: 0.8s
   Bonuses: 2x against Buildings

This might then accommodate icons instead of words, which would perhaps be more readable.
We could also write this shorter as such:

Attack:
   Melee: 5 Hack + 3 Pierce Damage, 5 Capture, Rate 0.3s Counters: Infantry (3x), Cavalry (2x)
   Capture: 3 Capture, Rate 0.8s, Counters: Building (1.5x)

Maybe we need shorter versions for the in-game tooltips. Bit of a conundrum.

Stan added a comment.Sep 1 2019, 12:06 PM

Didn't you like remove half the code and FeldFeld name from the credits ? Also you forgot the Kushite template and some other as well...

In D1707#93246, @Stan wrote:

Didn't you like remove half the code

Yes, it wasn't necessary after D2092

and FeldFeld name from the credits ?

FeldFeld name is still in there?

Also you forgot the Kushite template and some other as well...

I'm pretty sure I rebased from the latest diff.

Stan added a comment.Sep 1 2019, 12:41 PM

I guess the diff comparator failed then :D

Freagarach updated this revision to Diff 9632.Sep 5 2019, 9:23 AM
Freagarach retitled this revision from Hard-counter tooltips to Generalise hard-counter tooltips..
Freagarach edited the summary of this revision. (Show Details)
Freagarach edited the test plan for this revision. (Show Details)

Ugly-coded prove of concept for Melee Attack: X hack, Rate: Y second(s) (Zx vs Classes):


Code will be improved after D2138.

Freagarach commandeered this revision.Sep 5 2019, 9:23 AM
Freagarach added a reviewer: wraitii.
Vulcan added a comment.Sep 5 2019, 9:25 AM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/76/display/redirect

Vulcan added a comment.Sep 5 2019, 9:29 AM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 412| 412| function getRepairTimeTooltip(entState)
| 413| 413| {
| 414| 414| 	return sprintf(translate("%(label)s %(details)s"), {
| 415|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 415|+		"label": headerFont(translate("Number of repairers:")),
| 416| 416| 			"details": entState.repairable.numBuilders
| 417| 417| 		}) + "\n" + (entState.repairable.numBuilders ?
| 418| 418| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 413| 413| {
| 414| 414| 	return sprintf(translate("%(label)s %(details)s"), {
| 415| 415| 			"label": headerFont(translate("Number of repairers:")),
| 416|    |-			"details": entState.repairable.numBuilders
|    | 416|+		"details": entState.repairable.numBuilders
| 417| 417| 		}) + "\n" + (entState.repairable.numBuilders ?
| 418| 418| 		sprintf(translatePlural(
| 419| 419| 			"Add another worker to speed up the repairs by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 414| 414| 	return sprintf(translate("%(label)s %(details)s"), {
| 415| 415| 			"label": headerFont(translate("Number of repairers:")),
| 416| 416| 			"details": entState.repairable.numBuilders
| 417|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 417|+	}) + "\n" + (entState.repairable.numBuilders ?
| 418| 418| 		sprintf(translatePlural(
| 419| 419| 			"Add another worker to speed up the repairs by %(second)s second.",
| 420| 420| 			"Add another worker to speed up the repairs by %(second)s seconds.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 434| 434| function getBuildTimeTooltip(entState)
| 435| 435| {
| 436| 436| 	return sprintf(translate("%(label)s %(details)s"), {
| 437|    |-			"label": headerFont(translate("Number of builders:")),
|    | 437|+		"label": headerFont(translate("Number of builders:")),
| 438| 438| 			"details": entState.foundation.numBuilders
| 439| 439| 		}) + "\n" + (entState.foundation.numBuilders ?
| 440| 440| 		sprintf(translatePlural(
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 2 tabs but found 3.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 435| 435| {
| 436| 436| 	return sprintf(translate("%(label)s %(details)s"), {
| 437| 437| 			"label": headerFont(translate("Number of builders:")),
| 438|    |-			"details": entState.foundation.numBuilders
|    | 438|+		"details": entState.foundation.numBuilders
| 439| 439| 		}) + "\n" + (entState.foundation.numBuilders ?
| 440| 440| 		sprintf(translatePlural(
| 441| 441| 			"Add another worker to speed up the construction by %(second)s second.",
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 1 tab but found 2.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 436| 436| 	return sprintf(translate("%(label)s %(details)s"), {
| 437| 437| 			"label": headerFont(translate("Number of builders:")),
| 438| 438| 			"details": entState.foundation.numBuilders
| 439|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 439|+	}) + "\n" + (entState.foundation.numBuilders ?
| 440| 440| 		sprintf(translatePlural(
| 441| 441| 			"Add another worker to speed up the construction by %(second)s second.",
| 442| 442| 			"Add another worker to speed up the construction by %(second)s seconds.",
Executing section cli...

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

Freagarach updated this revision to Diff 9665.Sep 8 2019, 1:03 PM
Freagarach edited the summary of this revision. (Show Details)
Freagarach edited the test plan for this revision. (Show Details)

Rebased after D2138/rP22866.

Owners added a subscriber: Restricted Owners Package.Sep 8 2019, 1:03 PM
Vulcan added a comment.Sep 8 2019, 1:05 PM

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

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/96/display/redirect

Vulcan added a comment.Sep 8 2019, 1:08 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (operator-linebreak):
|    | '||' should be placed at the end of the line.
|----|    | /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/globalscripts/Templates.js
|  87|  87| 		// If the elements are still strings, split them by space or by '+'
|  88|  88| 		if (typeof sublist == "string")
|  89|  89| 			sublist = sublist.split(/[+\s]+/);
|  90|    |-		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1)
|  91|    |-		                    || (c[0] != "!" && classes.indexOf(c) != -1)))
|    |  90|+		if (sublist.every(c => (c[0] == "!" && classes.indexOf(c.substr(1)) == -1) ||
|    |  91|+		                    (c[0] != "!" && classes.indexOf(c) != -1)))
|  92|  92| 			return true;
|  93|  93| 	}
|  94|  94| 

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
Executing section cli...

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