The Petra hardcounters implementation seems to come from rP13225.
There is also a countersClasses, but I guess that is not affected qualitatively, unless that // TODO: refine using the multiplier is implemented, in which case the ApplyValueModifications part would be done too if it is done here.
You cited the function that does apply the ValueModifications, I would add the function comment in the two lines above just for clarity:

helper function to return a template value, optionally adjusting for tech.
TODO: there's no support for "_string" values here.

Both come from rP14588. The commit message says Still need to fix the entity.js file to handle properly some things as this uses raw templates values..
So if the function does what the comments claim to do, then the function would be functional if and only if the ._string format never occurs for the passed data in this place, is that actually the case? Or rather when could it be the case?

/ps/trunk/binaries/data/mods/public/simulation/components/tests/test_Attack.js
187	(s, t, e) -> (source, target, type)
/ps/trunk/binaries/data/mods/public/simulation/helpers/DamageBonus.js
4	`entity_id_t` isn't a JS type

Shall I make another diff then ?

In rP22346#34393, @Stan wrote:

Shall I make another diff then ?

We can't know that without knowing the answer to the question when the TODO case can be triggered.

In rP22346#34394, @elexis wrote:

In rP22346#34393, @Stan wrote:

Shall I make another diff then ?

We can't know that without knowing the answer to the question when the TODO case can be triggered.

Well obviously we can, since this isn't the only change you requested :)

In rP22346#34395, @Stan wrote:

Well obviously we can, since this isn't the only change you requested :)

Did I actually request a change or just ask a question? (jonathanfrakes.jpg)

Stan added inline comments.Jun 25 2019, 4:40 PM

/ps/trunk/binaries/data/mods/public/simulation/components/tests/test_Attack.js
187	Is that a question ? :P
/ps/trunk/binaries/data/mods/public/simulation/helpers/DamageBonus.js
4	Is that a question ? :P

elexis added inline comments.Jun 25 2019, 5:23 PM

/ps/trunk/binaries/data/mods/public/simulation/helpers/DamageBonus.js
4	Where I report a defect without clicking on request changes or concern, it's something that can be applied to the next patch someone works on where the same applies (it's not the first time either that there were non-JS types used here, so it's likely that such a new version with the same issue can appear in the near future) What is much more important is that the code isn't broken, i.e. the ._string question mentioned, or whatever else might be broken and undiscovered.

I don't understand your concern about AI?

countersClasses will return that it counters even if the multiplier is 1 - which arguably needs improving, but this didn't break it. The comment remains valid.
I don't see why you refer to _string, there never was a _string thing for attack bonuses?

In rP22346#34401, @wraitii wrote:

I don't understand your concern about AI?

(It's for a reason that I did not click on "raise concern" and repeat that difference in the last two comments.)

countersClasses will return that it counters even if the multiplier is 1 - which arguably needs improving, but this didn't break it. The comment remains valid.

That sounds reasonable to change then, for those volunteering in pursuing this feature (hence not clicking on raise concern nor stating that this is irrelevant).

I don't see why you refer to _string, there never was a _string thing for attack bonuses?

Well that's precisely my question, why is the TODO there if it's irrelevant, or differently phrased: _When_ does it become relevant?

In rP22346#34406, @elexis wrote:

I don't see why you refer to _string, there never was a _string thing for attack bonuses?

Well that's precisely my question, why is the TODO there if it's irrelevant, or differently phrased: _When_ does it become relevant?

I might be saying something wrong, but I believe _string is for token values and auras/techs can't modify those (see D270). It probably should be handled in D270 though.

elexis added inline comments.Jul 14 2019, 1:07 PM

/ps/trunk/binaries/data/mods/public/simulation/helpers/DamageBonus.js
27	(This function is called once upon causeDamage, I wonder if the result of the function could be cached and updated upon tech/aura change. Perhaps not since it depends on both source and target entity and that may be too many combinations to make cache hits significant. Perhaps it could be stored globally once on a per-template basis. I guess one can try to go for the worst performance parts first, but it's good to account for that in advance when introducing new code.)