More detailed unit info and small improvements

[KionX]

UEF ACU

Cybran SACU

Presets

Partially improved describe presets and enhancements.
Impossible to fully correctly describe presets and enhancements. There is not enough data in the blueprint.

[shalkya]

i will review more afterward

[shalkya]

I think both stealth should be shown listed together (not separated by the sonar value), anyway i'm explaining later why i would prefer radar/sonar stealth to be removed.

Not true, it's sonar stealth & radar stealth. I also find it more informative to have stealth field here.


for these 2, same thing : radar stealth and sonar stealth. Also stealth field is more informative.

this one should have radar stealth and sonar stealth. Also personal stealth is more informative.

Overall i understand why you made radar stealth and sonar stealth and removed personal stealth and stealth field for the "more unit description" feature. But it leads to inaccurate information or lack of information outside of the "more unit description" feature (Also for the unit database). The sonar stealth and the radar stealth does indeed exist. But afaik only 1 unit in the game is using 2 different values for them (the T3 cybran sonar) and there is high chances that balance team is going to make these value the same, for practical reason. Because of that I would like you to change the work you did for stealth, and keep it as before.

i wouldn't show "T3 Support Armoured Command Unit (Combatant Preset)' (which is pretty long) but only "SACU (Combatant Preset)". I think you should use for sacu preset, the name of the preset (eg the thing you removed).

[shalkya]

I know what you tried to mean with "less is better". But it isn't very explicit. I would advise you to change to "% of damage taken"
and then you display "Normal : 100%" "Overcharge : 25%" "Deathnuke : 3.2%" "CZAR beam : 25%" "Othe Tactical Bomb : 10%"

Maybe it would be even better to do "% of resistance"
"Normal : 0 %" "Overcharge : 75%" "Deathnuke "96.8%" "CZAR beam : 75%" "Othe Tactical Bomb : 90%"

Btw you really need to change "Othe Tactical Bomb" to "Ahwassa Bomb". Nobody will understand what "Othe Tactical Bomb" is.

[shalkya]

it's radar stealth and sonar stealth
i still feels displaying "personal radar stealth" or "personal sonar stealth" is a bit better. In the same way it works for shield.


it's radar stealth field and sonar stealth field

the sacu (preset or not) have wrong information.
1st : They all display the same information
2nd : Some have ability displayed, that they shouldn't have. Here the "cloacking" is only for cloack preset, and shouldn't appear for others sacu.
3rd : Some ability aren't shown. For example the Anti-air preset, should have the "anti-air" ability. It doesn't have it right now.
Edit : dps aren't updated either. It's mostly wrong for all preset. This need to be fixed, or hide information (if you can't fix) and show only the explanation text. We shouldn't display wrong information.

the "anti-shield" dps isn't shown, while it's the most important information.

[shalkya]

the death nuke damage is wrong. Should be around 2.5k and range/splash is wrong too.
it lacks other upgrades explanation. Could at least display the gun, since you display the other offensive upgrades.
the "Nuclear warhead" information is wrong. Wrong damage and splash.

Overall the ACU information display useless info (tele damage for example) while ignoring the upgrade survivability upgrades (nano, shields etc etc)

[KionX]

They all display the same information
Because the blueprint is the same.
Here the "cloacking" is only for cloack preset
Because the blueprint is the same and cloak exists.
Anti-air preset, should have the "anti-air" ability
This remains as it was. This ability can be added to the blueprint.
dps aren't updated either
It is difficult to apply the enhancements effects before creating a unit.

These are problems by design. It`s all special cases.
There is no longer any place where you can add descriptions.
If all special cases are taken into account, the code will turn into fucked shit.
I did not touch the simple list of abilities. It shows only abilities what is in the blueprints.

[KionX]

Impossible to correctly describe presets. There is not enough data in the blueprint.

[shalkya]

no stealth information on harms

can't build ML and megalith anymore.

errors :

WARNING: Error running HandleEvent script in CScriptObject at 1a3dc140: ...rs\pierre-yves\documents\github\fa\lua\maui\text.lua(172): bad argument #1 to `gfind' (string expected, got nil)
         stack traceback:
         	[C]: in function `gfind'
         	...rs\pierre-yves\documents\github\fa\lua\maui\text.lua(172): in function <...rs\pierre-yves\documents\github\fa\lua\maui\text.lua:151>
         	...s\documents\github\fa\lua\ui\game\unitviewdetail.lua(427): in function `WrapAndPlaceText'
         	...s\documents\github\fa\lua\ui\game\unitviewdetail.lua(687): in function `Show'
         	...ves\documents\github\fa\lua\ui\game\construction.lua(1159): in function `OnRolloverEvent'
         	...\pierre-yves\documents\github\fa\lua\maui\button.lua(62): in function <...\pierre-yves\documents\github\fa\lua\maui\button.lua:50>
WARNING: Error running HandleEvent script in CScriptObject at 1a3dc140: ...rs\pierre-yves\documents\github\fa\lua\maui\text.lua(172): bad argument #1 to `gfind' (string expected, got nil)

When hovering over Harms icon, when selecting it

WARNING: Error running HandleEvent script in CScriptObject at 225fe880: ...s\documents\github\fa\lua\ui\game\unitviewdetail.lua(411): bad argument #4 to `format' (number expected, got no value)
         stack traceback:
         	[C]: in function `format'
         	...s\documents\github\fa\lua\ui\game\unitviewdetail.lua(411): in function `?'
         	...s\documents\github\fa\lua\ui\game\unitviewdetail.lua(449): in function `WrapAndPlaceText'
         	...s\documents\github\fa\lua\ui\game\unitviewdetail.lua(687): in function `Show'
         	...ves\documents\github\fa\lua\ui\game\construction.lua(1157): in function `OnRolloverEvent'
         	...\pierre-yves\documents\github\fa\lua\maui\button.lua(62): in function <...\pierre-yves\documents\github\fa\lua\maui\button.lua:50>

When hovering over ML or Mega icon when selecting it

Can't get the info about Harms, ML and mega when hovering over their icon

[KionX]

no stealth information on harms
xrb2308 doesn't have stealth. Then it is replaced with xrb2309.

[shalkya]

alright so pretty much all good.
Last thing i would like is the death damage / crash damage of experimental (land and air). I think it's important and it's for several units.

[KionX]

Side effect - showing teleport ACU as weapon.
There is no suitable criterion for hiding.
Perhaps some more DummyWeapon will be displayed.

[shalkya]

yeah well it's not that bad.
Zthuue shows 15 dps and lobo shows 12 dps. I think it's due to them splitting in several bomb (a volley of multiple projectile). I think it's pretty bad if we display wrong dps, can you do something about that ?
in comparison aeon T1 arty is 100 dps, and cybran 38 dps; which are accurate dps.

Edit : same of Salvation which shows 70 dps
or uef mml which show 30 dps, when it actually shoot 3 missiles in a row. (60 or more dps for other mml)
novax have 3 dps
sera T2 pd has 13 dps

[KionX]

I did not find a way to improve calculate the DPS.

[KionX]

Reasons:
Unknown how many projectiles are in the salvo.
Unknown how many projectiles are in the queue.
Unknown how many parts the projectiles are divided into.

Maybe this can be corrected through the blueprint.

[shalkya]

so rapidly i looked into the unit databases :
for the normal unit database : https://github.com/FAForever/UnitDB
"T1 uef arty : DPS 12" WRONG
"T1 sera arty : DPS 16" WRONG
"T2 uef MML : DPS 60" TRUE
"salvation : DPS 71" WRONG
"novax : DPS 243" TRUE
"T2 sera PD : DPS 151" TRUE

for spooky unit database : https://github.com/spooky/unitdb
"T1 uef arty : DPS 60.24" TRUE
"T1 sera arty : DPS 77.59" TRUE
"T2 uef MML : DPS 60" TRUE
"salvation : DPS 70.97" WRONG
"novax : DPS 243" TRUE
"T2 sera PD : DPS 151.25" TRUE

You should look into their code to see how they calculate dps so you can improve the calculation for the displayed stat in the game. It seems that spooky can calculate T1 arty dps while normal database can't. Both can't calculate salvation dps.

[shalkya]

now other unit have wrong dps :
T2 uef torpedo launcher : 600 dps instead of expected 150 dps
UEF sam : 2000 dps instead of expected 333 dps
T1 cybran static AA : 140 dps instead of expected 70 dps
T2 cybran static flak : 300 dps instead of expected 150 dps
cybran sam : 1333 dps instead of expected 333 dps
harms : 2250 dps instead of expected 375 dps
T1 sera static AA : 133 dps instead of expected 67 dps
T2 sera torpedo launcher : 450 dps instead of expected 150 dps
T2 sera static flak : 285 instead of expected 140 dps
sera sam : 666 dps instead of expected 333 dps
T2 aeon torpedo launcher : 600 dps instead of expected 150 dps
aeon sam : 666 dps instead of expected 333 dps

[shalkya]

i guess you can remove dps for kamikaze (doesn't make sense), after that it should be good to merge.
Also the text for uef battleship is a little bit too big in comparison to the "box"/"window" for it.

[shalkya]

fca0950 did you only add the projectile files, or did you edit them at the same time ?

[KionX]

Edited a bit. I forgot to edit it as a separate commit.
2 lines in the BP. ~4 lines in the script to support new parameters.

[shalkya]

I understand you remade all the commits for sanity and to fix the commit mixing addition of files and modification of the same files, and i thank you for it.
But please, be careful !
92e3faa#diff-99a5c94d2d8bb8ecc67e191acf0f2ed2L699-R701
You are reverting the change made to ythota death weapon ! (https://github.com/FAForever/fa/pull/2934/files#diff-99a5c94d2d8bb8ecc67e191acf0f2ed2)
I will keep looking in the files to spot any other revert, but can you also do the same, and prevent revert to sneak into your pr in the future ?
I am not sure what caused it, but most likely you have some habits using git they may have caused it. For example, it's most likely unrelated i guess, but the pr should not need to be closed and reopened. You already told me it was automatic, but this isn't supposed to happen, even if you intend to force push.

[KionX]

I checked the PR again and corrected all unnecessary changes.
Also first post updated.

[shalkya]

thanks i'll look into it. Ah yeah i didn't see that you fixed that already in the commit "Reverting unnecessary changes".