Decide on format for fit statistics that is copied to clipboard

Gochim commented 4 years ago

Task at hand

Revisited pull request #1733 by @m-sasha, which allows to copy the fit stats to the clipboard. But we need to decide on format. @DarkFenX, @blitzmann and any people interested, could you take a look at it?

Actual behaviour. Currently look like this:

notepad++_2019-10-21_08-45-01

Posting as a picture since GitHub somewhat breaks formatting if pasted as is. I'll post as plain text at the end of the issue

Expected behaviour

It seem decent to me, but perhaps too much into? Maybe have two options available? Like limit default info to dps numbers, total ehp, total active tank, speed. And have a second option "Expanded info" with all info that is in actual behavior? But won't it be to much rework since we don't know how often-used this feature will be?

Paste of fit stats as is

All reps (Hyperion)

DPS: 700 (Weapon: 383, Drone: 317, Volley: 2509)

      TOTAL      EM   THERM     KIN     EXP

EHP 75.5k 82.3k 75.7k 80.1k 73.7k Shield 14.8k 12% 30% 48% 56% Armor 34.3k 78% 72% 72% 61% Hull 26.4k 60% 60% 60% 60%

REPS TOTAL SELF SUST REMOTE Shield 147 81.6 0 65 Armor 1.65k 1.59k 0 65.3 Hull 38.5 6.5 0 32 Total 1.84k 1.67k 0 162

Speed: 144 m/s Signature: 1.21k m Capacitor: 9k GJ (Lasts 42s) Targeting range: 107 km Scan resolution: 138 mm Sensor strength: 27.6

DarkFenX commented 4 years ago

Before going into specifics, i think we should clarify few things on a much higher level :P

Some stats of a fit are affected by external factors. Command bursts, projected effects. Do we export stats with them or without them?
Some of stats depend on extra data which you control via pyfa's UI. Resistances, active tank, ehp - they all can change depending on selected incoming damage pattern (resists because of RAH). DPS/RR stats can vary on spoolup setting (which can be set globally and on a per-module basis), damage stats can change based on selected target resistance profile and "factor in reload" setting (among others). Which settings should we use to export those? Currently used, or some "normal" ones?

m-sasha commented 4 years ago

IIRC, my idea was that it's basically a replacement for sending Pyfa screenshots. So it copies the stats currently shown in the UI.

DarkFenX commented 4 years ago

In this case it might also include some extra data like active incoming damage pattern, but then it might become too bulky. I have no hard opinions on this, so let's wait for @Gochim's response

Gochim commented 4 years ago

I viewed this data also as an "as is" data, so basically what you see on the right side of the fitting window - you get into the clipboard. Otherwise we become way too technical...

DarkFenX commented 4 years ago

In this case i think it makes little sense to provide EHP/tank data for different damage types. It should be just one value against currently selected incoming damage profile. Same for tanking types - what value does it add for active tank (and to a lesser extent, for ehp buffer)? Does it matter if fit tanks 2k with shield or armor?

Less obvious question is split between self peak / self sustainable / remote (which might sound confusing, is it RRs received or RRs provided?). Is it something we have to include? What kind of useful info sustainable tank provides? Why do we have to split between self and remote? (i am asking these questions not because I am opposed to the idea btw)

If user exports some hyperion fit which receives reps from 2 guardians, might as well just say in chat "these are stats of hyperion with 2 guardians repping it" - he will have to do it anyway to make it clear what stats are for.

As a side-note, I am quite busy with work stuff atm, so will provide my suggestion to stats format later.

m-sasha commented 4 years ago

@DarkFenX Again, this is an alternative to sending pyfa screenshots in chat channels. The information in the text is the information in the pyfa fit stats UI. All your questions, if valid, apply equally to the fit stats UI.

Gochim commented 4 years ago

I think there are two ways to look at it. One is like @DarkFenX suggests - simple and short summary of the fit capabilities. @m-sasha suggests that we just literally copy-paste the content of the right side of the fit.

Both actually sound good to me tbh :)

m-sasha commented 4 years ago

It’s slightly better than literal, as I try to compact the output by removing useless stats (0 damage etc.) Or maybe I added that after creating the PR, I don’t remember.

DarkFenX commented 4 years ago

My bad, did not realize that it's actually resistances which are shown in tank table, I thought it is EHP numbers against different resistances (if you skim through it with weak eyesight, k is similar to %).

As for pyfa screenshot vs exported stats - I guess it is based on my understanding on how you use those stats. During theorycrafting for any AT i passed wild amount of ship stats to various chats. While I use pyfa to analyze/optimize various fits and combinations (for which seeing all this data is almost a must), I used stats to present either final result for others to judge it, or some kind of comparison between multiple fits so that other participants could choose variant they liked the most. For this, extra stats probably make little sense.

As example of the first (presenting final result) I actually had a tool based on the new Eos engine which calculated most of those things for me, here is how it looked for some random ship combination (headers are initially collapsed): link. As you can see, it did not export just "current view" of stats, it exported everything i wanted to see about ships (e.g. tanks vs different damage types in different modes of operation).

So, if you use ship stats differently, then yes you might need different info. If you think that this is just replacement for pyfa screenshot - obviously this format is quite close to perfect.

Gochim commented 4 years ago

We can actually make two types of exports. Something like "Fit summary" (short version) and "Fit statistics" (the current one). Or will it be too much?

DarkFenX commented 4 years ago

No, i think fit summary alone will do. I am not insisting on anything, I was trying to understand what you wanted to achieve with this format.

m-sasha commented 4 years ago

The goal with this was actually exactly to improve communication wrt fits for AT prep. I wanted a simple tool, however, and this is it. Because it is textual, you can easily edit out the parts you don’t want, after copying it out of pyfa. Otherwise, I would need to implement UI to ask the user what parts he wants in the output.

DarkFenX commented 4 years ago

Another question - do you want to cater format to monowidth fonts? (judging by table in the screenshot) It won't look as good with any variable-width fonts.

m-sasha commented 4 years ago

That’s a good question. I would prefer not to, but a format that doesn’t rely on columns aligning would be much less compact or intuitive. A dependence on monospace fonts is fine for any modern forum, I guess, but maybe not for the majority of chat channels.

Gochim commented 4 years ago

@DarkFenX , @m-sasha , I suggest we add it in current format and then look at the feedback. So it won't stall needlessly...

If there are no major problems that you see that needs addressed right now, I'll create a pull request on it shortly.

DarkFenX commented 4 years ago

Yeah sorry. Got carried away by non-development stuff. Don't be shy and poke me in slack if i go MIA.

I agree to the format, however i want to propose variable-width font format (and have it as format option):

Stat: Total | EM | Therm | Kin | Exp EHP: 75.5k | 82.3k | 75.7k | 80.1k | 73.7k Shield: 14.8k | 12% | 30% | 48% | 56% Armor: 34.3k | 78% | 72% | 72% | 61% Hull: 26.4k | 60% | 60% | 60% | 60%

or

Stat | Total | EM | Therm | Kin | Exp EHP | 75.5k | 82.3k | 75.7k | 80.1k | 73.7k Shield | 14.8k | 12% | 30% | 48% | 56% Armor | 34.3k | 78% | 72% | 72% | 61% Hull | 26.4k | 60% | 60% | 60% | 60%

What do you think about it?

Also, if it is replacement for pyfa screenshots, do you like Total as 1st column or last column more (in UI it is last, here it is 1st)?

m-sasha commented 4 years ago

I think that's both ugly and confusing, because when columns don't align, the header doesn't project its meaning on the column.

If you assume people know the standard EM-Th-Kin-Exp order, then there's no need for the | separators, just use enough whitespace to separate them visually: EHP: 75.5k (82.3k 75.7k 80.1k 73.7k)

If you assume people don't know it, you have to prefix every value with its meaning: EHP: 75.5k (EM: 82.3k, Th: 75.7k, Kin: 80.1k, Exp: 73.7k)

m-sasha commented 4 years ago

@Gochim before you do anything, let me see if I can push additional improvements to the PR. I added some things after creating it. I'll try to do it before the weekend.

Gochim commented 4 years ago

@Gochim before you do anything, let me see if I can push additional improvements to the PR. I added some things after creating it. I'll try to do it before the weekend.

@m-sasha That would not be a very good idea. When pulling your changes I had to manually merge decent amount of code and then fix several issues to make it work. If you want to make improvements, lets first accept pull request with my changes, then make your improvement based on it.

m-sasha commented 4 years ago

Sounds like a plan.

Gochim commented 4 years ago

Also, if it is replacement for pyfa screenshots, do you like Total as 1st column or last column more (in UI it is last, here it is 1st)?

Regarding total I'd go "like in PYFA". Consistency all the way :)

I agree to the format, however i want to propose variable-width font format (and have it as format option):

Heated discussion I see :) I suggest that we check how it looks when copy/pasting to EVE. If it looks good with column aligning - we keep it this way. If not - then good idea would be to go either with separators, or reformat into strings.

DarkFenX commented 4 years ago

Nah, if it doesn't look good then discard variable-width format altogether i think. It's just format which I am using in variable-width environments when comparing different fits, but use-case is different here.

DarkFenX commented 4 years ago

Also regarding implementation/format if you have any disagreements, could take discussion and coordination to slack. Without such coordination I believe it's better to let one person finish his job and then discuss it to make necessary adjustments, or after pull request has been accepted - make another one with changes you want to do.

Gochim commented 4 years ago

Tried all 3 variants in eve. And to be honest, all 3 look good in a wide chat window (though each had it's own small issues). But when I checked narrow chat windows. the best one looking is a format like below: EHP: 75.5k (EM: 82.3k, Th: 75.7k, Kin: 80.1k, Exp: 73.7k) It is actually the only one to keep good readability. So that's the one I'm going to go with for now.

Later, if needed, we can expand on export options if needed...

m-sasha commented 4 years ago

The use case I was aiming for was AT discussion which happens mostly on forums, and a little bit in chats (but not in EVE), so I'd rather have the monospaced variant as the primary format.

Gochim commented 4 years ago

Monospaced doesn't align properly on Github, it doesn't align properly on EVE online and on E-UNI forums. So I'll go with the most generally readable variant from above that you also suggested. :) Though I'll keep current format in the code as a separate method, so if you want - you could include it as an option along with your planned improvements.

m-sasha commented 4 years ago

HAWKS vs BSOD (Typhoon Fleet Issue)

DPS: 904 (Volley: 7827)

          TOTAL      EM   THERM     KIN     EXP
EHP       83.4k    102k   81.1k   77.9k   81.5k
Shield    35.8k     71%     64%     63%     69%
Armor     19.6k     66%     45%     36%     24%
Hull        28k     60%     60%     60%     60%

Self shield repair: 591 EHP/s (Sustained: 199 EHP/s)

Speed: 172 m/s
Signature: 336 m
Capacitor: 7.25k GJ (Lasts 35s)
Targeting range: 177 km
Scan resolution: 187 mm
Sensor strength: 57.9

EVE forums has this option, and I'm pretty sure E-UNI too.

DarkFenX commented 4 years ago

Currently implemented format seems to be variable-width format:

100MN HM cheap (Loki)

DPS: 553 (Weapon: 553, Drone: 0, Volley: 1660)

EHP: 59.6k (Em: 82.2k, Th: 57.5k, Kin: 53.6k, Exp: 61.1k
Shield: 47.9k (Em: 75%, Th: 70%, Kin: 70%, Exp: 75%
Armor: 9.26k (Em: 83%, Th: 59%, Kin: 36%, Exp: 24%
Hull: 2.49k (Em: 60%, Th: 60%, Kin: 60%, Exp: 60%

Speed: 3201 m/s
Signature: 169 m
Capacitor: 1.62k GJ (Lasts 2m30s)
Targeting range: 81.2 km
Scan resolution: 300 mm
Sensor strength: 20.4

(there are some minor issues which i will fix a bit later) Is it something you intended to have?

DarkFenX commented 4 years ago

Fixed the parenthesis issue. Got a few more questions, mostly about title-style capitalization. How appropriate is it here? Few different examples from abobe:

Weapon detailed stats in parenthesis:

DPS: 553 (Weapon: 553, Drone: 0, Volley: 1660)

Resist types:

EHP: 59.6k (Em: 82.2k, Th: 57.5k, Kin: 53.6k, Exp: 61.1k

Capacitor extra info in parenthesis:

Capacitor: 1.62k GJ (Lasts 2m30s)
Capacitor: 570 GJ (Stable at 86%)

Also is it better to show 2m30s, 2m 30s or 2:30?

I think i will make a release as-is now. But if we want to make any extra changes soonish - I am confident we will have to publish another release soon (as CCP fucked up Vengeance's speed nerf).

Gochim commented 4 years ago

(there are some minor issues which i will fix a bit later) Is it something you intended to have?

Yes, as a direct result of the discussion above. I used the format that is the most generally readable in EVE and couple of forum I checked without additional steps. I left the old format in a working, but commented section of the code. So if additional enhancements will be implemented - we could consider adding a selector to the dialog to allow user to choose the type of export.

I think i will make a release as-is now. But if we want to make any extra changes soonish - I am confident we will have to publish another release soon (as CCP fucked up Vengeance's speed nerf).

I don't plan to make any additional work on this feature in the near time, so either way is fine with me.

DarkFenX commented 4 years ago

Ok I am closing this one then. If anyone thinks we should adjust anything, just post further comments in here or create new ticket.

pyfa-org / Pyfa