xyzz / acquisition

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

Misc_improvements #497

Closed DaneelTrevize closed 5 years ago

DaneelTrevize commented 5 years ago

Correct the unifying label width mechanism, for "Map Tier" being longer than "R. Level".

Zana's new signature mod is very verbosely worded. The QCompleter mods dropdown forces the min-width of the search UI to the longest mod string in the list. So I replaced a few Master mods with* prefixed shorter names. More experience with Qt would be required to possibly find a way to alter the popup's width behaviour via QAbstractItemView, and/or styling these placeholder mod lines (e.g. with italics). An example of the new minimum size of the mods dropdown can be seen here.
Fix use of disable_login bool for certain Login DisplayErrors.
Add support for "% Increased Attack and Cast speed" mod (found on jewels), increased spell base crit chance, and % maximum life.
Subjectively improved the order of mods in the dropdown filter.
Tidy some declaration orders, NULL pointers, unused parameters to reduce warnings.

DaneelTrevize commented 5 years ago

Again, haven't touched C++ in ~15 years, mostly just went with what the suggested Qt Creator and googling recent C++11 discussions pointed me to, in order to reduce the warnings.

I guess I now modify the branch, and then bump this PR with a comment when I think it's ready for another review?

xyzz commented 5 years ago

Yeah that sounds fine.

ericsium commented 5 years ago

You can make another commit and then squash the new commit in with a previous and re-edit the comment if you want. Once you update the branch on github servers it will re-qualify the change and we'll see the update in this thread.

You can do this using git 'rebase' command. It allows you to manipulate the last X checkins on a branch, re-order them, update comments, squash them together etc... It's a good thing to get familar with.

For example, you have 2 commits on this branch, if you checked in a third commit you could run 'git rebase -i HEAD~3'.

If you're rebased a local branch you need to push to server with 'git push -f' to force the push. Once that's done your pull will be automatically re-qualified.

DaneelTrevize commented 5 years ago

Ok, I think I'm done squashing and deduplicating commit comments, review PR again please.

Personally I am against editing history and losing change granularity, especially for remotely pushed changes, but done as requested, I think.

DaneelTrevize commented 5 years ago

Going to recreate this PR with a manually rebased branch, because of 498 being closed/consistent policy of rewriting commit history.