xyzz / acquisition

http://get.acquisition.today/
GNU General Public License v3.0
271 stars 100 forks source link

[Suggestion] Export item to clipboard #448

Open OSwoboda opened 6 years ago

OSwoboda commented 6 years ago

Would be nice if an item could be exported to clipboard to be able to import it to path of building for example.

DaneelTrevize commented 5 years ago

Anyone know the current intended use of the Tooltip Text tab?
I find it's set to Qt::FocusPolicy NoFocus, so it can't even be used as a place to click, Ctrl+A, Ctrl+C, paste into PoB's Create custom... item form and massage from there.

Changing it to StrongFocus enables clicking, but it would need a few tweaks to be able to be imported without edits: Split the name and base item type onto different lines; Ditch everything else before the mods lists;
Remove extra newlines from badly handled new hybrid mods such +armour/+ES;

The resulting text imports, and generates the following central attributes (for a body armour):

Quality: 20
Sockets: ...
LevelReq: ...
Implicits: 0

Which seems a very reasonable place for users to continue, saving a lot of finding correct/equivalent mod combos via PoB's Craft Item form, especially without having to cross-reference with in-game & the new Alt view, to check for hybrid mods.

As for UX, if this text tooltip tab were to be repurposed for this use-case, I'm thinking a button to "Export to Clipboard in Path of Building format" might be better, and then I'm wondering why the last tab is Hide..?
It doesn't hide the whole tooltip pane (with socketed image above and stash map below), thought that can be done by dragging the vertical divider sufficiently right. And the "Tooltip" first tab has an "Upload to Imgur" button already on it, so perhaps our Export and hide/collapse UI elements would be best placed below that and on the collapsing area? And replace the tab header with a show button?

xyzz commented 5 years ago

Yeah, the purpose is so that it displays same information as the fancy tooltip view but takes less space. The "hide" tab is to hide the tooltip, making the right pane take even less space.

I think instead of changing the tooltip a better approach would be to add a new button to the right column, "copy to clipboard" (and maybe support shortcuts like ctrl+c), which copies pob-compatible data (so just like you described, but instead of changing the text tooltip it's completely separate).

DaneelTrevize commented 5 years ago

The "hide" tab is to hide the tooltip, making the right pane take even less space.

Ok, but I don't experience that:
Before, with the area sized just smaller than the element, triggering the scroll bar.
After, with no change to the invisible width and now wasting space.

I think that displaying the PoB-format text that it would generate would help people understand what a button would do, similar to how the fancy tooltip does above the Imgur button. I'm no UX expert but this seems reasonable.

xyzz commented 5 years ago

Yeah, unfortunately the layout code leaves much to be desired, I think it still works in some cases when you have a longer item name or mods. If you're worried about discoverability, you can just name the button "copy to pob".

DaneelTrevize commented 5 years ago

Note to self: PoB creates player-input custom items via a call to the method ItemsTabClass:CreateDisplayItemFromRaw(), which eventually calls ItemClass:ParseRaw(), as found here.
This seems to provide the strings and regex to target generating matches for, but not an overall data structure or schema to validate against.

DaneelTrevize commented 5 years ago

To generate the PoB format for a given item, we first need to know if it's a wearable item (including flasks) that POB would even accept for "Create custom".

This lead down a rabbit hole where flasks aren't currently correctly categorised by Acquisition, because their 2D art isn't generated under the same CDN path format as other items, possibly because of the POE website displaying flask charges within the image. Which leads me to question why we aren't using the "category" object in an item's JSON, rather than regex over the icon path..?
Incomplete proof of concept, but other than wanting a mapping of backend to pretty strings, and a few conditional statements for nesting certain subcategories as is done currently (i.e. not for gems & jewels, thought that's questionable), why not do it this way via the JSON objects hierarchy rather than CDN paths?

xyzz commented 5 years ago

It's using the path because when that feature was implemented the category field wasn't exposed via json; it's a good idea to change it now though.

DaneelTrevize commented 5 years ago

Additional observation: flasks' icon paths contain Base64 encoded JSON with their 2D art paths and thus sub-type/category. E.g. WzksNCx7ImYiOiJBcnRcLzJESXRlbXNcL0ZsYXNrc1wvbGlmZWZsYXNrMTEiLCJzcCI6MC42MDg1LCJsZXZlbCI6MX1d

[9,4,{"f":"Art\/2DItems\/Flasks\/lifeflask11","sp":0.6085,"level":1}]

I [haven't found](https://gchq.github.io/CyberChef/#recipe=Analyse_hash()From_Hex('Auto'/disabled)&input=NzU4MDlmNGQ3YQ) what the subsequent hex code portion of the icon path is checksumming/doing. /75809f4d7a\/Item.png

I think we'd still need to use something like the 2D path parsing for flasks and maps to determine sub-types (and it looks like POE 3.5 will bring new map art).

DaneelTrevize commented 5 years ago

Proposed new implementation of categories, and thus Wearable, is available for review under PR #507 , so the work on the PoB export format itself can continue distinct from these preparatory steps.

DaneelTrevize commented 5 years ago

POB export functionality is found in PR #508

OSwoboda commented 5 years ago

I tested it and there is a major bug: Rare items are imported as uniques and magic items can't be imported because its tried to import them as rares.

DaneelTrevize commented 5 years ago

IIRC the PoB "Create custom..." dialog does not auto-select the correct rarity of item you're creating due to the mod list pasted, instead it just default to the last used one. Please confirm you're not just experiencing this aspect of their UI. And if so, perhaps raise an enhancement request here.

OSwoboda commented 5 years ago

I tested it with c&p from an ingame item:

Rarity: Unique Presence of Chayula Onyx Amulet

Requirements: Level: 60

Item Level: 42

+16 to all Attributes

30% increased Rarity of Items found +60% to Chaos Resistance Cannot be Stunned 20% of Maximum Life Converted to Energy Shield

The dreamer stirs, the world trembles.

and from acquisiton:

Presence of Chayula Onyx Amulet Implicits: 1 +16 to all Attributes 30% increased Rarity of Items found +60% to Chaos Resistance Cannot be Stunned 20% of Maximum Life Converted to Energy Shield

As you can see the information about rarity is missing. This would be ignoring the combobox in the "create custom..." dialog and also would support the option to create an item with just pasting it into the item tab and click add to build instead of "create custom" -> paste -> create -> add to build.

DaneelTrevize commented 5 years ago

Ok, quick testing indicates the "Rarity:" line can be added and immediately affects the PoB dialog's mouseover tooltip, but doesn't change the dropdown. Saving the item at this point will set the rarity, so it will work. Improvement might be added before Betrayal.

The reason this wasn't in the current version of Acquisition is because this feature was developed by finding the minimal version of what PoB presents back (when you subsequently edit a custom item) that works to (re)create the time. i.e. PoB can calculate the level req. itself from the base, so that's a redundant line.
But perhaps instead this feature should be adjusted to generate the same clipboard contents as PoE does itself, for use with any other tools that accept that, rather than a PoB-specific minimised version?

OSwoboda commented 5 years ago

But perhaps instead this feature should be adjusted to generate the same clipboard contents as PoE does itself, for use with any other tools that accept that, rather than a PoB-specific minimised version?

I think that would be the best general solution, using it for pob was just an example in my suggestion.

Thanks for listening to my concerns, looking forward to the improvement!