Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Favorite System v1.0 #29287

Merged
merged 5 commits into from Apr 9, 2019

Conversation

Projects
None yet
5 participants
@cafeoh
Copy link
Contributor

commented Apr 5, 2019

Summary

SUMMARY: Features "Adds a fully functional item favoriting system"

Back with more better features and 70% cleaner code!

gif
gif
(Thanks to @anothersimulacrum for his save by the way! ❤️ )

Purpose of change

Allows to favorite items from every menu including the advanced item menu (but not from the Compare and Multidrop menus). In addition to selecting a single item via the inventory menu (Enter -> f) there's now a new default keybinding to * that favorites the full stack under the cursor.

Two menu behaviours were added to make this actually useful (beyond just the visual aspect) :

  • Category selection in the drop menu, on successive selection, first selects all non favorite item, then all favorite items, then deselects. This allows you, for example, after returning from a profitable food hoarding run to drop everything except your jerky and V8.
  • Advanced inventory works similarly when using the drop all (,) key from your inventory, where it only first empties out all non favorite item on the first press, and then drop all the rest of your favourite gear (with a warning).

Describe the solution

Changed item stacking, item selector, and other UI stuff.

Future additions

  • Implement the , key in the multidrop menu (might be redundant with the advanced inventory, but it'd be pretty straightforward if people are interested 🤷‍♂️ )
  • When dropping items out of necessity (dropping your faithful duffel bag for example), make non favorite items drop first. Haven't looked into it, should be pretty easy though.
  • Add favoriting from drop menu (To be honest that'd be a pain for a few reasons, as favoriting items can cause stacks to merge and requires to reload all items for safety, therefore requiring loops outside input handling, partial refresh/caching items and just a lot of nastiness)
  • Find a better word for "favoriting"? I don't know it's starting to lose all its meaning to me. Favoriting. Favoriting, favoriting. Yeah I hate it. 😡
@Wokko1

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

Has any other PR received that many upvotes in so little time?

@cafeoh cafeoh force-pushed the cafeoh:favorite_v1 branch from e02d872 to d17cd28 Apr 5, 2019

@mlangsdorf
Copy link
Contributor

left a comment

my knowledge of the inventory system is just enough to get me in trouble, but nothing in this looks obviously bad.

@ifreund
Copy link
Contributor

left a comment

Code looks great to me, haven't compiled or tested though. Nice work!

@@ -20,7 +20,7 @@
#include "vpart_reference.h"

const invlet_wrapper
inv_chars( "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!\"#&()*+.:;=@[\\]^_{|}" );
inv_chars( "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!\"#&()+.:;=@[\\]^_{|}" );

This comment has been minimized.

Copy link
@ifreund

ifreund Apr 7, 2019

Contributor

I'm curious as to how loading old saves with * used as an invlet is handled. (I have no idea what would happen, it may not need any special handling)

This comment has been minimized.

Copy link
@cafeoh

cafeoh Apr 7, 2019

Author Contributor

I tested it, and basically the items just keep the invlet. As usual, invlets take precedence over key bindings, so it means you won't be able to use the * bind until you unassign the item, but I don't think it's a big deal. I thought about cleaning the invlets manually (making sure they match), but I'm not convinced it'd really be worth it to have that piece of code laying around.

I wanted to improve handling of invlet attribution later so it's not as hardcoded (there's no verification at the moment), but that's for another PR.

@kevingranade kevingranade merged commit 7a3ce39 into CleverRaven:master Apr 9, 2019

0 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
gorgon-ghprb Build triggered for merge commit.
Details
@kevingranade

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

This pull request has been mentioned on Cataclysm: Dark Days Ahead. There might be relevant details there:

https://discourse.cataclysmdda.org/t/latest-experimental-features/5582/1106

@cafeoh cafeoh deleted the cafeoh:favorite_v1 branch Apr 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.