xyzz / acquisition

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

[Enhancement] Support War for the Atlas (Shaper/Elder) item varients #490

Closed DaneelTrevize closed 5 years ago

DaneelTrevize commented 5 years ago

Items can now be generated ingame that are flagged as either Shaper or Elder varients/bases, and the API data also exposes this. Acquisition could be enhanced to display this status, both as a column in the search results window, and in the game-imitating tooltip header bar and background.

Additionally Acquisition could be enhanced to support filtering to items have Shaper/Elder/either base types.

Potentially a pseudo Mods filter line could be added for any & all known Shaper/Elder mods, but this seems excessive, as would listing many of the popular ones in the dropdown.

DaneelTrevize commented 5 years ago

I am able to provide a PR that will initially just add a "War" column to the search results, displaying an S for Shaper items, and E for Elder. It would be the single commit under this branch.

Subsequent code to add a Shaper/Elder/either search element has been partially developed, currently a checkbox to filter for just either, but I'm interested if this is sufficient or if I should investigate how to add a new dropdown to provide 3 choices.

The currently proposed column and checkbox are present in this example (as well as the lack of enhanced tooltip border and background).

I'm not sure if this issue should be reduced in scope to just cover the prepared column change, or kept and want the search UI changes brought into it & the proposed branch.

ericsium commented 5 years ago

Github does not really do dependent commits well. I noticed that it looks like you branched support_war_for_the_atlas_types off of fix_modlist_strings rather than master so it includes unrelated mod changes. You really want to keep these separate so your branches are based off of the current master head and only include changes for requested enhancement. That way pull requests can be reviewed independent of other changes.

After you sync your fork's master branch with the upstream project I'd spend some time coming up with clean branches. You could for example use rebase to delete the unrelated commits on this branch.

I think when I started contributing I spent about a quarter of my time just trying to figure out git and I still don't feel like I have a great handle on it.

ericsium commented 5 years ago

The current changes you've made look straightforward. It's okay to work on search/ui changes as well on this branch.

DaneelTrevize commented 5 years ago

PR #498 addresses these changes.

It's the least-conflicting way I can slice the remaining changes I'd previously prepared as a chain of branches. This one has a hint of the other corrections to be made w.r.t. the dynamically calculated label width for neat UI positioning, and of a new flowlayout instance to be used by several new elements.

DaneelTrevize commented 5 years ago

Is there any official source of the 2D art assets used to regenerate the item header and background look, because Shaper and Elder items have new varients of these. Procurement seems to have a Shaper header asset, but not a complete set. Just source them from the official website via linking an item in a forum thread? I'm not sure I'll get time to get up to speed with how Qt is using embedded assets and HTML to form these tooltip images.

DaneelTrevize commented 5 years ago

(In my dev fork) I've managed to add new 1x1 to 2x4 Shaper and Elder background assets, sourced from the POE website.
For the Shaper background, which acts as a small window onto a much larger tiled canvas, I went with 0,0 x,y offsets for all sizes, having any atlas-indicating background seems far more important than using the exact larger-image offset for where an item is currently stashed (especially as it changed when the item's moved, and isn't the same dynamic/animated background online as in-game anyway).

I've managed to add this transparent background into the GenerateItemIcon function flow (i.e. using QPainter), and the same approach seems the best way to combine the header icons with the Shaper/Elder icon overlays too, rather than the current QLabel border-image stylesheet approach which lacks an obvious way to overlap images.