search

xyzz / acquisition

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

[Enhancement] Support Chaos Damage as fully as Physical and Elemental types #491

Closed DaneelTrevize closed 5 years ago

DaneelTrevize commented 5 years ago

The game has changed, for several years now Chaos Damage is at least as common for players to build around dealing as other damage types.
Acquisition can be enhanced to:
display the flat added chaos damage of weapons;
calculate and display the chaos DPS of weapons, and the true total DPS of them;
offer a pair of search fields for min and max Chaos DPS; offer Mods filters for Added Chaos Damage to attacks, and to Spells, and for increased Chaos Damage %.

DaneelTrevize commented 5 years ago

PR #495 addresses these changes.

Most of these proposed changes can be see here.

P.S. Is merging upstream and resolving conflicts as done here favoured, or would attempting to rebase my outstanding/future branches against an updated fork master be better?

ericsium commented 5 years ago

Either rebasing your branches or merging with -ff_only are effectively the same thing. If you merge without -ff_only sometimes git seems to add in a special mereg commit which are best avoided. I'd advise you to try to avoid having to deal with any merge conflicts in git. You can generally do this unless you have multiple pulls that depend on either other. In that case I'd submit the first, wait for it to be mainlined, then submit your dependent pull (which no longer has dependencies) once you're rebased or merged in the mainline change.

Again though I don't claim to be especially knowledgeable with git, as long as the pulls only include the commits of interest and git says there are no merge conflicts then there usually isn't an issue with the pull.