Page MenuHomeWildfire Games
Differential D2138

Generalised Attack effects tooltips after D2092/rP22754.
ClosedPublic
Actions

Authored by Freagarach on Aug 1 2019, 2:43 PM.

Details

Reviewers
wraitii
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP22866: Generalise attack tooltips after D2092/rP22754.
Summary

D2092/rP22754 introduced that any attack can have any effect. However, the tooltips not yet support more than one effect for a attack.
This patch aims to fix that. Note that tooltips for StatusEffects were not present before and will not be hereafter, since a more elaborate way of showing those is needed.

Test Plan

Verify that everything works as intended.

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

There are a very large number of changes, so older changes are hidden. Show Older Changes
This revision now requires changes to proceed.Aug 23 2019, 9:23 AM
Freagarach added a comment.EditedAug 23 2019, 5:56 PM
In D2138#92053, @wraitii wrote:

Thanks for taking a jab at this :)

You're welcome! I'm afraid though that I neither understand it fully nor am convinced yet (probably related). (By the way, if you think: "don't be stupid (no worries, I think that all the time about myself), its easy, let me do it in 5 mins." feel free to commandeer ;) )

In D2138#92053, @wraitii wrote:

These 3 things should have their own tooltip wrapper, not use a common one. Indeed, status effects will look completely different.

Can live with that.

In D2138#92053, @wraitii wrote:

Further, these tooltip functions need to be agnostic from the attack's name / type, since they must apply for death damage, status effects, etc.
Right now we have:

  • attackRateDetails
  • damageTypesToText
  • getAttackTooltip
  • getSplashDamageTooltip

It would make more sense now to have (names indicative)

  • DamageTooltip -> per damage type of <Damage>, gives you the DPS + rate
  • CaptureTooltip
  • StatusEffectsTooltip (which will in turn call the other tooltip helpers)

Why duplicate the attackRateDetails? It is common in all the attack types isn't it?

In D2138#92053, @wraitii wrote:

And then AttackTooltip, calling the other tooltips, with each attack type (which will pre-emptively help with D368)
In general Splash tooltip and Death tooltip should also be trivially composable from the first 3.

So if I understand correctly:

  • AttackTooltip (equally DeathDamage, Splash) calls:
    • attackRateDetails (not for DD and splash)
    • damageTooltip
    • captureTooltip
    • StatusEffectsTooltip; which also calls
      • DamageTooltip
    • rangeTooltip
elexis retitled this revision from Tooltips for D2092/rP22754. to Generalised Attack effects tooltips for D2092/rP22754..Aug 23 2019, 5:58 PM
Freagarach updated this revision to Diff 9461.Aug 23 2019, 9:51 PM
Freagarach retitled this revision from Generalised Attack effects tooltips for D2092/rP22754. to Generalised Attack effects tooltips after D2092/rP22754..

What about this?
In seperate diffs?

  • Support for DeathDamage tooltip.
  • Status effects tooltips.
Vulcan added a comment.Aug 23 2019, 9:55 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 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
| 267| 267| 		return "";
| 268| 268| 
| 269| 269| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 270|    |-			"damage": template.Capture.toFixed(1),
|    | 270|+		"damage": template.Capture.toFixed(1),
| 271| 271| 			"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 272| 272| 		});
| 273| 273| }
|    | [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
| 268| 268| 
| 269| 269| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 270| 270| 			"damage": template.Capture.toFixed(1),
| 271|    |-			"damageType": unitFont(translateWithContext("damage type", "Capture"))
|    | 271|+		"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 272| 272| 		});
| 273| 273| }
| 274| 274| 
|    | [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
| 269| 269| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 270| 270| 			"damage": template.Capture.toFixed(1),
| 271| 271| 			"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 272|    |-		});
|    | 272|+	});
| 273| 273| }
| 274| 274| 
| 275| 275| function statusEffectsTooltip(template)
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 304| 304| 			"details": attackRateDetails(template, type)
| 305| 305| 		});
| 306| 306| 		let effects = [captureTooltip(template.attack[type]),
| 307|    |-				damageTooltip(template.attack[type]),
|    | 307|+			damageTooltip(template.attack[type]),
| 308| 308| 				statusEffectsTooltip(template.attack[type])].filter(effect => effect);
| 309| 309| 		let range = rangeTooltip(template.attack[type]);
| 310| 310| 
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 305| 305| 		});
| 306| 306| 		let effects = [captureTooltip(template.attack[type]),
| 307| 307| 				damageTooltip(template.attack[type]),
| 308|    |-				statusEffectsTooltip(template.attack[type])].filter(effect => effect);
|    | 308|+			statusEffectsTooltip(template.attack[type])].filter(effect => effect);
| 309| 309| 		let range = rangeTooltip(template.attack[type]);
| 310| 310| 
| 311| 311| 		tooltips.push(sprintf(translate("%(attackLabel)s %(details)s, %(range)s, %(rate)s"), {
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 331| 331| 			continue;
| 332| 332| 
| 333| 333| 		let effects = [captureTooltip(splash),
| 334|    |-				damageTooltip(splash),
|    | 334|+			damageTooltip(splash),
| 335| 335| 				statusEffectsTooltip(splash)].filter(effect => effect);
| 336| 336| 
| 337| 337| 		let splashDamageTooltip = sprintf(translate("%(label)s: %(effects)s"), {
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 3 tabs but found 4.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 332| 332| 
| 333| 333| 		let effects = [captureTooltip(splash),
| 334| 334| 				damageTooltip(splash),
| 335|    |-				statusEffectsTooltip(splash)].filter(effect => effect);
|    | 335|+			statusEffectsTooltip(splash)].filter(effect => effect);
| 336| 336| 
| 337| 337| 		let splashDamageTooltip = sprintf(translate("%(label)s: %(effects)s"), {
| 338| 338| 			"label": headerFont(g_SplashDamageTypes[splash.shape]),
|    | [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/461/display/redirect

Harbormaster completed remote builds in B9029: Diff 9461.Aug 23 2019, 9:55 PM
wraitii added a comment.Aug 24 2019, 8:23 AM

That is indeed more what I mean ?
I think you'll agree that it's more flexible.

What I meant about attackRateDetails is that it expects an entity template (and the name of an attack type), which is far more than simply "attack rate" tooltips should need. So I think it needs some changing too to be re-usable for something other than attack types.

wraitii mentioned this in D2218: Status Effects v1 - GUI support and fix following rP22754.Aug 24 2019, 10:40 AM
Freagarach updated this revision to Diff 9477.Aug 24 2019, 12:30 PM

Elaborated the attackRateTooltip, however, I could not split it because it would add more duplicity. IMHO This is a decent alternative.

Vulcan added a comment.Aug 24 2019, 12:34 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 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
| 273| 273| 		return "";
| 274| 274| 
| 275| 275| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 276|    |-			"damage": template.Capture.toFixed(1),
|    | 276|+		"damage": template.Capture.toFixed(1),
| 277| 277| 			"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 278| 278| 		});
| 279| 279| }
|    | [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
| 274| 274| 
| 275| 275| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 276| 276| 			"damage": template.Capture.toFixed(1),
| 277|    |-			"damageType": unitFont(translateWithContext("damage type", "Capture"))
|    | 277|+		"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 278| 278| 		});
| 279| 279| }
| 280| 280| 
|    | [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
| 275| 275| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 276| 276| 			"damage": template.Capture.toFixed(1),
| 277| 277| 			"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 278|    |-		});
|    | 278|+	});
| 279| 279| }
| 280| 280| 
| 281| 281| function statusEffectsTooltip(template)
|    | [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
| 422| 422| function getRepairTimeTooltip(entState)
| 423| 423| {
| 424| 424| 	return sprintf(translate("%(label)s %(details)s"), {
| 425|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 425|+		"label": headerFont(translate("Number of repairers:")),
| 426| 426| 			"details": entState.repairable.numBuilders
| 427| 427| 		}) + "\n" + (entState.repairable.numBuilders ?
| 428| 428| 		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
| 423| 423| {
| 424| 424| 	return sprintf(translate("%(label)s %(details)s"), {
| 425| 425| 			"label": headerFont(translate("Number of repairers:")),
| 426|    |-			"details": entState.repairable.numBuilders
|    | 426|+		"details": entState.repairable.numBuilders
| 427| 427| 		}) + "\n" + (entState.repairable.numBuilders ?
| 428| 428| 		sprintf(translatePlural(
| 429| 429| 			"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
| 424| 424| 	return sprintf(translate("%(label)s %(details)s"), {
| 425| 425| 			"label": headerFont(translate("Number of repairers:")),
| 426| 426| 			"details": entState.repairable.numBuilders
| 427|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 427|+	}) + "\n" + (entState.repairable.numBuilders ?
| 428| 428| 		sprintf(translatePlural(
| 429| 429| 			"Add another worker to speed up the repairs by %(second)s second.",
| 430| 430| 			"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
| 444| 444| function getBuildTimeTooltip(entState)
| 445| 445| {
| 446| 446| 	return sprintf(translate("%(label)s %(details)s"), {
| 447|    |-			"label": headerFont(translate("Number of builders:")),
|    | 447|+		"label": headerFont(translate("Number of builders:")),
| 448| 448| 			"details": entState.foundation.numBuilders
| 449| 449| 		}) + "\n" + (entState.foundation.numBuilders ?
| 450| 450| 		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
| 445| 445| {
| 446| 446| 	return sprintf(translate("%(label)s %(details)s"), {
| 447| 447| 			"label": headerFont(translate("Number of builders:")),
| 448|    |-			"details": entState.foundation.numBuilders
|    | 448|+		"details": entState.foundation.numBuilders
| 449| 449| 		}) + "\n" + (entState.foundation.numBuilders ?
| 450| 450| 		sprintf(translatePlural(
| 451| 451| 			"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
| 446| 446| 	return sprintf(translate("%(label)s %(details)s"), {
| 447| 447| 			"label": headerFont(translate("Number of builders:")),
| 448| 448| 			"details": entState.foundation.numBuilders
| 449|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 449|+	}) + "\n" + (entState.foundation.numBuilders ?
| 450| 450| 		sprintf(translatePlural(
| 451| 451| 			"Add another worker to speed up the construction by %(second)s second.",
| 452| 452| 			"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/471/display/redirect

Harbormaster completed remote builds in B9045: Diff 9477.Aug 24 2019, 12:34 PM
Freagarach added inline comments.Aug 24 2019, 2:20 PM
binaries/data/mods/public/gui/common/tooltips.js
345 ↗(On Diff #9477)

Can be inlined below.

Freagarach mentioned this in D1707: Generalise hard-counter tooltips..Aug 29 2019, 4:45 PM
Freagarach updated this revision to Diff 9535.Aug 30 2019, 8:41 AM

Inlined effects at splash.

Vulcan added a comment.Aug 30 2019, 8:46 AM

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 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
| 273| 273| 		return "";
| 274| 274| 
| 275| 275| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 276|    |-			"damage": template.Capture.toFixed(1),
|    | 276|+		"damage": template.Capture.toFixed(1),
| 277| 277| 			"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 278| 278| 		});
| 279| 279| }
|    | [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
| 274| 274| 
| 275| 275| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 276| 276| 			"damage": template.Capture.toFixed(1),
| 277|    |-			"damageType": unitFont(translateWithContext("damage type", "Capture"))
|    | 277|+		"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 278| 278| 		});
| 279| 279| }
| 280| 280| 
|    | [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
| 275| 275| 	return sprintf(translate("%(damage)s %(damageType)s"), {
| 276| 276| 			"damage": template.Capture.toFixed(1),
| 277| 277| 			"damageType": unitFont(translateWithContext("damage type", "Capture"))
| 278|    |-		});
|    | 278|+	});
| 279| 279| }
| 280| 280| 
| 281| 281| function statusEffectsTooltip(template)
|    | [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
| 420| 420| function getRepairTimeTooltip(entState)
| 421| 421| {
| 422| 422| 	return sprintf(translate("%(label)s %(details)s"), {
| 423|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 423|+		"label": headerFont(translate("Number of repairers:")),
| 424| 424| 			"details": entState.repairable.numBuilders
| 425| 425| 		}) + "\n" + (entState.repairable.numBuilders ?
| 426| 426| 		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
| 421| 421| {
| 422| 422| 	return sprintf(translate("%(label)s %(details)s"), {
| 423| 423| 			"label": headerFont(translate("Number of repairers:")),
| 424|    |-			"details": entState.repairable.numBuilders
|    | 424|+		"details": entState.repairable.numBuilders
| 425| 425| 		}) + "\n" + (entState.repairable.numBuilders ?
| 426| 426| 		sprintf(translatePlural(
| 427| 427| 			"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
| 422| 422| 	return sprintf(translate("%(label)s %(details)s"), {
| 423| 423| 			"label": headerFont(translate("Number of repairers:")),
| 424| 424| 			"details": entState.repairable.numBuilders
| 425|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 425|+	}) + "\n" + (entState.repairable.numBuilders ?
| 426| 426| 		sprintf(translatePlural(
| 427| 427| 			"Add another worker to speed up the repairs by %(second)s second.",
| 428| 428| 			"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
| 442| 442| function getBuildTimeTooltip(entState)
| 443| 443| {
| 444| 444| 	return sprintf(translate("%(label)s %(details)s"), {
| 445|    |-			"label": headerFont(translate("Number of builders:")),
|    | 445|+		"label": headerFont(translate("Number of builders:")),
| 446| 446| 			"details": entState.foundation.numBuilders
| 447| 447| 		}) + "\n" + (entState.foundation.numBuilders ?
| 448| 448| 		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
| 443| 443| {
| 444| 444| 	return sprintf(translate("%(label)s %(details)s"), {
| 445| 445| 			"label": headerFont(translate("Number of builders:")),
| 446|    |-			"details": entState.foundation.numBuilders
|    | 446|+		"details": entState.foundation.numBuilders
| 447| 447| 		}) + "\n" + (entState.foundation.numBuilders ?
| 448| 448| 		sprintf(translatePlural(
| 449| 449| 			"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
| 444| 444| 	return sprintf(translate("%(label)s %(details)s"), {
| 445| 445| 			"label": headerFont(translate("Number of builders:")),
| 446| 446| 			"details": entState.foundation.numBuilders
| 447|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 447|+	}) + "\n" + (entState.foundation.numBuilders ?
| 448| 448| 		sprintf(translatePlural(
| 449| 449| 			"Add another worker to speed up the construction by %(second)s second.",
| 450| 450| 			"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/512/display/redirect

Harbormaster completed remote builds in B9113: Diff 9535.Aug 30 2019, 8:46 AM
Freagarach added inline comments.Aug 30 2019, 9:05 AM
binaries/data/mods/public/gui/common/tooltips.js
310–312 ↗(On Diff #9535)

Back to the g_AttackTypes[type] for performance reasons?
This now has removed hardcoding of the attack types.

wraitii requested changes to this revision.Sep 1 2019, 10:09 AM

This looks pretty good. Changes I'd do:

  • rename 'template' to specify what this template is exactly (e.g. entityTemplate, attackTemplate, damageTemplate...)
  • pass derived templates to the damageTooltip, captureTooltip functions
  • skip status effects for this patch
  • the previous code picked 'Rate' for all units, and 'Interval' for structures, but you're actually picking 'Interval' everywhere. Is there a reason for this? I think if anything it should default to Rate, but I also think you could keep the switching code.
binaries/data/mods/public/gui/common/tooltips.js
182 ↗(On Diff #9535)

You're losing the Rate / Interval switch here... Not sure it's a huge issue though.

258 ↗(On Diff #9535)

I'd prefer template to be already template.Damage here.

287 ↗(On Diff #9535)

Needs to be changed to 'Give Status' - but actually I'd just remove it in this diff, as I have D2218 in the pipes.

294 ↗(On Diff #9535)

I'd prefer you pass template.Damage, ... (see above).

310–312 ↗(On Diff #9535)

We do have performance concerns in GUI scripts, but this is likely fast enough.

This revision now requires changes to proceed.Sep 1 2019, 10:09 AM
Freagarach updated this revision to Diff 9575.Sep 1 2019, 1:12 PM
Freagarach marked 4 inline comments as done.
  • Specify templates.
  • Pass effect templates.
  • Nuke StatusEffects despite it being merely a placeholder (see summary).
  • Inlined SplashShape.
  • Interval -> Rate.
Freagarach added a subscriber: Nescio.Sep 1 2019, 1:12 PM
Freagarach added inline comments.
binaries/data/mods/public/gui/common/tooltips.js
182 ↗(On Diff #9535)

Yeah, I've renamed to Rate for that makes more sense, I do not think we lose information here (@Nescio?), but I can reintroduce the switch if necessary.

Vulcan added a comment.Sep 1 2019, 1:14 PM

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

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

Freagarach added inline comments.Sep 1 2019, 1:16 PM
binaries/data/mods/public/gui/common/tooltips.js
11–20 ↗(On Diff #9575)

Removal of these could be seperated, is that desired?

Vulcan added a comment.Sep 1 2019, 1:17 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [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
| 267| 267| 		return "";
| 268| 268| 
| 269| 269| 	return sprintf(translate("%(amount)s %(name)s"), {
| 270|    |-			"amount": captureTemplate.toFixed(1),
|    | 270|+		"amount": captureTemplate.toFixed(1),
| 271| 271| 			"name": unitFont(translateWithContext("damage type", "Capture"))
| 272| 272| 		});
| 273| 273| }
|    | [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
| 268| 268| 
| 269| 269| 	return sprintf(translate("%(amount)s %(name)s"), {
| 270| 270| 			"amount": captureTemplate.toFixed(1),
| 271|    |-			"name": unitFont(translateWithContext("damage type", "Capture"))
|    | 271|+		"name": unitFont(translateWithContext("damage type", "Capture"))
| 272| 272| 		});
| 273| 273| }
| 274| 274| 
|    | [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
| 269| 269| 	return sprintf(translate("%(amount)s %(name)s"), {
| 270| 270| 			"amount": captureTemplate.toFixed(1),
| 271| 271| 			"name": unitFont(translateWithContext("damage type", "Capture"))
| 272|    |-		});
|    | 272|+	});
| 273| 273| }
| 274| 274| 
| 275| 275| function attackEffectsTooltip(attackTypeTemplate)
|    | [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| function getRepairTimeTooltip(entState)
| 414| 414| {
| 415| 415| 	return sprintf(translate("%(label)s %(details)s"), {
| 416|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 416|+		"label": headerFont(translate("Number of repairers:")),
| 417| 417| 			"details": entState.repairable.numBuilders
| 418| 418| 		}) + "\n" + (entState.repairable.numBuilders ?
| 419| 419| 		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
| 414| 414| {
| 415| 415| 	return sprintf(translate("%(label)s %(details)s"), {
| 416| 416| 			"label": headerFont(translate("Number of repairers:")),
| 417|    |-			"details": entState.repairable.numBuilders
|    | 417|+		"details": entState.repairable.numBuilders
| 418| 418| 		}) + "\n" + (entState.repairable.numBuilders ?
| 419| 419| 		sprintf(translatePlural(
| 420| 420| 			"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
| 415| 415| 	return sprintf(translate("%(label)s %(details)s"), {
| 416| 416| 			"label": headerFont(translate("Number of repairers:")),
| 417| 417| 			"details": entState.repairable.numBuilders
| 418|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 418|+	}) + "\n" + (entState.repairable.numBuilders ?
| 419| 419| 		sprintf(translatePlural(
| 420| 420| 			"Add another worker to speed up the repairs by %(second)s second.",
| 421| 421| 			"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
| 435| 435| function getBuildTimeTooltip(entState)
| 436| 436| {
| 437| 437| 	return sprintf(translate("%(label)s %(details)s"), {
| 438|    |-			"label": headerFont(translate("Number of builders:")),
|    | 438|+		"label": headerFont(translate("Number of builders:")),
| 439| 439| 			"details": entState.foundation.numBuilders
| 440| 440| 		}) + "\n" + (entState.foundation.numBuilders ?
| 441| 441| 		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
| 436| 436| {
| 437| 437| 	return sprintf(translate("%(label)s %(details)s"), {
| 438| 438| 			"label": headerFont(translate("Number of builders:")),
| 439|    |-			"details": entState.foundation.numBuilders
|    | 439|+		"details": entState.foundation.numBuilders
| 440| 440| 		}) + "\n" + (entState.foundation.numBuilders ?
| 441| 441| 		sprintf(translatePlural(
| 442| 442| 			"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
| 437| 437| 	return sprintf(translate("%(label)s %(details)s"), {
| 438| 438| 			"label": headerFont(translate("Number of builders:")),
| 439| 439| 			"details": entState.foundation.numBuilders
| 440|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 440|+	}) + "\n" + (entState.foundation.numBuilders ?
| 441| 441| 		sprintf(translatePlural(
| 442| 442| 			"Add another worker to speed up the construction by %(second)s second.",
| 443| 443| 			"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/541/display/redirect

Harbormaster failed remote builds in B9156: Diff 9575!Sep 1 2019, 1:17 PM
Nescio added a comment.Sep 1 2019, 1:48 PM

What does rate mean here?

Stan added a subscriber: Stan.Sep 1 2019, 1:58 PM
Stan added inline comments.
binaries/data/mods/public/gui/common/tooltips.js
156 ↗(On Diff #9575)

TODO (Full caps I believe) I always wondered if tickets should'nt be created each time and Todos referencing them (But that's another topic)

158 ↗(On Diff #9575)

What if repeat time is 0 ?

163 ↗(On Diff #9575)

Guess it could be +projectile > 1

306 ↗(On Diff #9575)

Could set it to 0 and ignore my comment above maybe

Freagarach added a comment.Sep 1 2019, 2:29 PM
In D2138#93283, @Nescio wrote:

What does rate mean here?

Rate is here the attack rate. Or the rate at which arrows are fired (previously Interval).

Nescio added a comment.Sep 1 2019, 2:38 PM

Again, what is the rate here? Damage per arrow, arrows per time, damage per time, a speed, something else? If it's just <RepeatTime>, then why not call it time?

Freagarach added a comment.Sep 1 2019, 2:45 PM
In D2138#93315, @Nescio wrote:

Again, what is the rate here? Damage per arrow, arrows per time, damage per time, a speed, something else? If it's just <RepeatTime>, then why not call it time?

Ah, it is the repeatTime for melee and 1-projectile attacks and arrows per repeatTime for multi-projectile attacks.

Freagarach added a comment.Sep 1 2019, 3:28 PM

May I suggest we don't really care for the name as in rP14954 they got switched and no-one objected? The split was introduced in rP14618. Related discussion on IRC.

Freagarach marked 4 inline comments as done.Sep 1 2019, 3:49 PM
Freagarach added inline comments.
binaries/data/mods/public/gui/common/tooltips.js
156 ↗(On Diff #9575)

I always wondered if tickets should'nt be created each time and Todos referencing them

Agreed, but it is kinda difficult here, as the ticket has no meaning without this patch being committed. And when this patch is committed I can either open a ticket regarding the issue, or create a diff resolving the issue.

158 ↗(On Diff #9575)

All hell breaks loose. No really, try it.

Freagarach updated this revision to Diff 9583.Sep 1 2019, 3:58 PM
Freagarach marked 2 inline comments as done.
  • Rate -> Interval.
  • if (projectiles && projectiles > 1) -> if (projectiles > 1).
Vulcan added a comment.Sep 1 2019, 3:59 PM

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

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

Vulcan added a comment.Sep 1 2019, 4:02 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [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
| 267| 267| 		return "";
| 268| 268| 
| 269| 269| 	return sprintf(translate("%(amount)s %(name)s"), {
| 270|    |-			"amount": captureTemplate.toFixed(1),
|    | 270|+		"amount": captureTemplate.toFixed(1),
| 271| 271| 			"name": unitFont(translateWithContext("damage type", "Capture"))
| 272| 272| 		});
| 273| 273| }
|    | [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
| 268| 268| 
| 269| 269| 	return sprintf(translate("%(amount)s %(name)s"), {
| 270| 270| 			"amount": captureTemplate.toFixed(1),
| 271|    |-			"name": unitFont(translateWithContext("damage type", "Capture"))
|    | 271|+		"name": unitFont(translateWithContext("damage type", "Capture"))
| 272| 272| 		});
| 273| 273| }
| 274| 274| 
|    | [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
| 269| 269| 	return sprintf(translate("%(amount)s %(name)s"), {
| 270| 270| 			"amount": captureTemplate.toFixed(1),
| 271| 271| 			"name": unitFont(translateWithContext("damage type", "Capture"))
| 272|    |-		});
|    | 272|+	});
| 273| 273| }
| 274| 274| 
| 275| 275| function attackEffectsTooltip(attackTypeTemplate)
|    | [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| function getRepairTimeTooltip(entState)
| 414| 414| {
| 415| 415| 	return sprintf(translate("%(label)s %(details)s"), {
| 416|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 416|+		"label": headerFont(translate("Number of repairers:")),
| 417| 417| 			"details": entState.repairable.numBuilders
| 418| 418| 		}) + "\n" + (entState.repairable.numBuilders ?
| 419| 419| 		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
| 414| 414| {
| 415| 415| 	return sprintf(translate("%(label)s %(details)s"), {
| 416| 416| 			"label": headerFont(translate("Number of repairers:")),
| 417|    |-			"details": entState.repairable.numBuilders
|    | 417|+		"details": entState.repairable.numBuilders
| 418| 418| 		}) + "\n" + (entState.repairable.numBuilders ?
| 419| 419| 		sprintf(translatePlural(
| 420| 420| 			"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
| 415| 415| 	return sprintf(translate("%(label)s %(details)s"), {
| 416| 416| 			"label": headerFont(translate("Number of repairers:")),
| 417| 417| 			"details": entState.repairable.numBuilders
| 418|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 418|+	}) + "\n" + (entState.repairable.numBuilders ?
| 419| 419| 		sprintf(translatePlural(
| 420| 420| 			"Add another worker to speed up the repairs by %(second)s second.",
| 421| 421| 			"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
| 435| 435| function getBuildTimeTooltip(entState)
| 436| 436| {
| 437| 437| 	return sprintf(translate("%(label)s %(details)s"), {
| 438|    |-			"label": headerFont(translate("Number of builders:")),
|    | 438|+		"label": headerFont(translate("Number of builders:")),
| 439| 439| 			"details": entState.foundation.numBuilders
| 440| 440| 		}) + "\n" + (entState.foundation.numBuilders ?
| 441| 441| 		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
| 436| 436| {
| 437| 437| 	return sprintf(translate("%(label)s %(details)s"), {
| 438| 438| 			"label": headerFont(translate("Number of builders:")),
| 439|    |-			"details": entState.foundation.numBuilders
|    | 439|+		"details": entState.foundation.numBuilders
| 440| 440| 		}) + "\n" + (entState.foundation.numBuilders ?
| 441| 441| 		sprintf(translatePlural(
| 442| 442| 			"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
| 437| 437| 	return sprintf(translate("%(label)s %(details)s"), {
| 438| 438| 			"label": headerFont(translate("Number of builders:")),
| 439| 439| 			"details": entState.foundation.numBuilders
| 440|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 440|+	}) + "\n" + (entState.foundation.numBuilders ?
| 441| 441| 		sprintf(translatePlural(
| 442| 442| 			"Add another worker to speed up the construction by %(second)s second.",
| 443| 443| 			"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/546/display/redirect

Harbormaster failed remote builds in B9165: Diff 9583!Sep 1 2019, 4:02 PM
Stan added inline comments.Sep 1 2019, 4:06 PM
binaries/data/mods/public/gui/common/tooltips.js
163 ↗(On Diff #9583)

If it can be undefined you need to add the '+' to cast it to int ( I see the variable below is undefined) else it might error out/have some weird behavior
(Js also have function for casting to int but they are slow so we just use +variable)

156 ↗(On Diff #9575)

Sure though the best would be to fix it now if it isn't complex :D

158 ↗(On Diff #9575)

Ah yeah of course. A 0 repeat time in Timer.js is just a one time timer but here it's different :)

Freagarach marked an inline comment as done.Sep 1 2019, 4:12 PM
Freagarach added inline comments.
binaries/data/mods/public/gui/common/tooltips.js
163 ↗(On Diff #9583)

I tested but not got any strange behavior? Still a good change to do, though.

156 ↗(On Diff #9575)

Well yes, except that I do not think templates have projectile names? They only have projectile actors, which you don't want to show here ;) So that means changing templates, which I think is out of scope (I strayed a bit already).

Nescio added a comment.Sep 1 2019, 6:48 PM

A rate (ratio) implies something per something, e.g.

  • fertility rate is the number of children born per woman
  • incarceration rate is the number of prisoners per 100,000 population
  • interest rate is the money paid over money borrowed

Likewise, 3 arrows per 2 seconds is a rate, but 2 seconds is a time (interval). 0 A.D.'s tooltips are actually incorrect, so perhaps you could solve that:


binaries/data/mods/public/gui/common/tooltips.js
260–262 ↗(On Diff #9583)

Actually the switch is reversed here; it ought to be: if buildingAI then Rate else Interval.

Freagarach planned changes to this revision.Sep 1 2019, 6:59 PM
In D2138##inline-43818, @Nescio wrote:

Actually the switch is reversed here; it ought to be: if buildingAI then Rate else Interval.

I noticed ;)

In D2138#93343, @Freagarach wrote:

May I suggest we don't really care for the name as in rP14954 they got switched and no-one objected? The split was introduced in rP14618. Related discussion on IRC.

Discussion on IRC today.
So there *is* objection and I will revert to the desired behaviour. Thanks @Nescio!

Nescio added a comment.Sep 1 2019, 7:03 PM

So there *is* objection and I will revert to the desired behaviour.

It doesn't really matter how you call it internally in the code, as long as it is done consistently. However, for user-facing text strings it's best to be precise. (I actually only noticed 0 A.D. does it wrong today, because of your patch; thank you.)

Freagarach added a comment.Sep 1 2019, 7:44 PM
In D2138#93461, @Nescio wrote:

So there *is* objection and I will revert to the desired behaviour.

It doesn't really matter how you call it internally in the code, as long as it is done consistently. However, for user-facing text strings it's best to be precise. (I actually only noticed 0 A.D. does it wrong today, because of your patch; thank you.)

I removed the whole differentiation between them and used Interval for both ^^.

Freagarach updated this revision to Diff 9598.Sep 2 2019, 5:43 PM
Freagarach marked 2 inline comments as done.

When multiple projectiles: Rate instead of Interval.

Vulcan added a comment.Sep 2 2019, 5:46 PM

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

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

Vulcan added a comment.Sep 2 2019, 5:49 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
|    |++++| /zpool0/trunk/binaries/data/mods/public/gui/common/tooltips.js
| 163| 163| 
| 164| 164| 	if (+projectiles > 1)
| 165| 165| 	{
| 166|    |-		header = headerFont(translate("Rate:"))
|    | 166|+		header = headerFont(translate("Rate:"));
| 167| 167| 		let projectileString = sprintf(translatePlural("%(projectileCount)s %(projectileName)s", "%(projectileCount)s %(projectileName)s", projectiles), {
| 168| 168| 			"projectileCount": projectiles,
| 169| 169| 			"projectileName": unitFont(translatePlural("arrow", "arrows", projectiles))
|    | [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
| 269| 269| 		return "";
| 270| 270| 
| 271| 271| 	return sprintf(translate("%(amount)s %(name)s"), {
| 272|    |-			"amount": captureTemplate.toFixed(1),
|    | 272|+		"amount": captureTemplate.toFixed(1),
| 273| 273| 			"name": unitFont(translateWithContext("damage type", "Capture"))
| 274| 274| 		});
| 275| 275| }
|    | [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
| 270| 270| 
| 271| 271| 	return sprintf(translate("%(amount)s %(name)s"), {
| 272| 272| 			"amount": captureTemplate.toFixed(1),
| 273|    |-			"name": unitFont(translateWithContext("damage type", "Capture"))
|    | 273|+		"name": unitFont(translateWithContext("damage type", "Capture"))
| 274| 274| 		});
| 275| 275| }
| 276| 276| 
|    | [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
| 271| 271| 	return sprintf(translate("%(amount)s %(name)s"), {
| 272| 272| 			"amount": captureTemplate.toFixed(1),
| 273| 273| 			"name": unitFont(translateWithContext("damage type", "Capture"))
| 274|    |-		});
|    | 274|+	});
| 275| 275| }
| 276| 276| 
| 277| 277| function attackEffectsTooltip(attackTypeTemplate)
|    | [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
| 415| 415| function getRepairTimeTooltip(entState)
| 416| 416| {
| 417| 417| 	return sprintf(translate("%(label)s %(details)s"), {
| 418|    |-			"label": headerFont(translate("Number of repairers:")),
|    | 418|+		"label": headerFont(translate("Number of repairers:")),
| 419| 419| 			"details": entState.repairable.numBuilders
| 420| 420| 		}) + "\n" + (entState.repairable.numBuilders ?
| 421| 421| 		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
| 416| 416| {
| 417| 417| 	return sprintf(translate("%(label)s %(details)s"), {
| 418| 418| 			"label": headerFont(translate("Number of repairers:")),
| 419|    |-			"details": entState.repairable.numBuilders
|    | 419|+		"details": entState.repairable.numBuilders
| 420| 420| 		}) + "\n" + (entState.repairable.numBuilders ?
| 421| 421| 		sprintf(translatePlural(
| 422| 422| 			"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
| 417| 417| 	return sprintf(translate("%(label)s %(details)s"), {
| 418| 418| 			"label": headerFont(translate("Number of repairers:")),
| 419| 419| 			"details": entState.repairable.numBuilders
| 420|    |-		}) + "\n" + (entState.repairable.numBuilders ?
|    | 420|+	}) + "\n" + (entState.repairable.numBuilders ?
| 421| 421| 		sprintf(translatePlural(
| 422| 422| 			"Add another worker to speed up the repairs by %(second)s second.",
| 423| 423| 			"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
| 437| 437| function getBuildTimeTooltip(entState)
| 438| 438| {
| 439| 439| 	return sprintf(translate("%(label)s %(details)s"), {
| 440|    |-			"label": headerFont(translate("Number of builders:")),
|    | 440|+		"label": headerFont(translate("Number of builders:")),
| 441| 441| 			"details": entState.foundation.numBuilders
| 442| 442| 		}) + "\n" + (entState.foundation.numBuilders ?
| 443| 443| 		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
| 438| 438| {
| 439| 439| 	return sprintf(translate("%(label)s %(details)s"), {
| 440| 440| 			"label": headerFont(translate("Number of builders:")),
| 441|    |-			"details": entState.foundation.numBuilders
|    | 441|+		"details": entState.foundation.numBuilders
| 442| 442| 		}) + "\n" + (entState.foundation.numBuilders ?
| 443| 443| 		sprintf(translatePlural(
| 444| 444| 			"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
| 439| 439| 	return sprintf(translate("%(label)s %(details)s"), {
| 440| 440| 			"label": headerFont(translate("Number of builders:")),
| 441| 441| 			"details": entState.foundation.numBuilders
| 442|    |-		}) + "\n" + (entState.foundation.numBuilders ?
|    | 442|+	}) + "\n" + (entState.foundation.numBuilders ?
| 443| 443| 		sprintf(translatePlural(
| 444| 444| 			"Add another worker to speed up the construction by %(second)s second.",
| 445| 445| 			"Add another worker to speed up the construction by %(second)s seconds.",

binaries/data/mods/public/gui/common/tooltips.js
| 166| »   »   header·=·headerFont(translate("Rate:"))
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.
Executing section cli...

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

Harbormaster completed remote builds in B9184: Diff 9598.Sep 2 2019, 5:49 PM
wraitii accepted this revision.Sep 7 2019, 3:58 PM

I think this needs one last change but I'll do it myself before committing as it's fairly straightforward.

Thanks for the patch.

binaries/data/mods/public/gui/common/tooltips.js
154 ↗(On Diff #9598)

pass repeatTime directly here, so that we can use this for status effects.

This revision is now accepted and ready to land.Sep 7 2019, 3:58 PM
wraitii updated this revision to Diff 9655.Sep 7 2019, 4:10 PM

Use 'Details' instead of 'Tooltip' for helper functions (previous convention), group them together (range and attack rate were split from attack by armour-related tooltips).

Also fix linter report.

Vulcan added a comment.Sep 7 2019, 4:16 PM

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

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

Vulcan added a comment.Sep 7 2019, 4:28 PM

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

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

Harbormaster completed remote builds in B9252: Diff 9655.Sep 7 2019, 4:28 PM
wraitii updated this revision to Diff 9656.Sep 7 2019, 4:31 PM

Add ':' back, and make session tooltips at most 400 pixels wide so that this fits on one line.

wraitii added a comment.Sep 7 2019, 4:32 PM

Without the tooltip size change:


With:

Vulcan added a comment.Sep 7 2019, 4:32 PM

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

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

Vulcan added a comment.Sep 7 2019, 4:36 PM

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

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

Harbormaster completed remote builds in B9253: Diff 9656.Sep 7 2019, 4:36 PM
Closed by commit rP22866: Generalise attack tooltips after D2092/rP22754. (authored by wraitii). · Explain WhySep 7 2019, 4:47 PM
This revision was automatically updated to reflect the committed changes.
wraitii added a commit: rP22866: Generalise attack tooltips after D2092/rP22754..
Freagarach added a comment.Sep 8 2019, 8:55 AM
In D2138#94180, @wraitii wrote:

Thanks for the patch.

Thanks for committing this!

Revision Contents
Changeset List

PathSize
ps/
trunk/
binaries/
data/
mods/
public/
gui/
common/
10 lines
235 lines

Diff 9658

View Options

ps/trunk/binaries/data/mods/public/gui/common/setup.xml

Loading...
View Options

ps/trunk/binaries/data/mods/public/gui/common/tooltips.js

Loading...
Wildfire Games · Phabricator