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

Nested Containers (a.k.a item pockets) #39406

Merged
merged 125 commits into from
May 3, 2020

Conversation

KorGgenT
Copy link
Member

@KorGgenT KorGgenT commented Apr 9, 2020

Summary

SUMMARY: Features "Implements item pockets"

Purpose of change

Resolves #3671

Describe the solution

All right gang buckle up 'cuz the ride's just beginning.

This is a very large PR. I have attempted to cut it up into reasonable commits, but I don't think i can split it into multiple PRs because then it won't compile.

Here's a rundown of what all this project does:

  1. Items have any number of "pockets". (including 0, but those are just not containers)
  2. each pocket can have different attributes associated with it, much like islot_container (which this pr incidentally also removes)
    The current pocket attributes, with their json names, are as follows:
  • "pocket_type" - a pocket type denotes a regular pocket (CONTAINER) vs. more specialized pockets that either do not use the same restrictions or have other special rules. (MAGAZINE, CORPSE)
  • "ammo_restriction" - this pocket can only contain items with an ammotype that matches one of the ammotypes in this set. this also overrides the usage of min_item_volume, max_item_volume, and max_contains_weight.
  • "min_item_volume" - the smallest volume of an item that can be contained in this pocket. think of it like things will fall out, the UI will stop you from trying to do that though.
  • "max_contains_volume" - the total volume available for use in this pocket.
  • "max_contains_weight" - same as above but weight
  • "spoil_multiplier" - a multiplier on rot. sort of a wip, meant to be used maybe insteaed of "preserves" or in tandem with it for containers that reseal.
  • "weight_multiplier" - a multiplier of weight of the contents. only affects how much the containing item weighs, so max_contains_weight uses the full item weights. meant to be used for mod content, i added this because it was like 2 lines of code.
  • "magazine_well" - an amount of space the contents of a nonrigid item ignores before it starts to expand.
  • "moves" - the base number of moves to take out or put in items in this pocket
  • "fire_protection" - imported from the magazine islot, protects the contents from exploding if the item gets destroyed from fire.
  • "watertight" - can contain liquid
  • "gastight" - can contain gas WIP
  • "open_container" - spills items if placed into a pocket. it's like a bucket.
  • "flag_restriction" - items contained must have one of the flags in this list.
  • "rigid" - item doesn't change size if items are placed inside it.
  • "resealable" - meant to be used like "preserves". still a WIP, possibly not something that'll stay in the final version.
  • "holster" - this item can only contain one item or item stack.
  1. Character no longer has an amalgam inventory space. All items are stored in other items, or worn or wielded.
  2. when a player gets an item, it tries to pick the best pocket this item will go into. (see Character::best_pocket(), item_contents::best_pocket(), and item_pocket::better_pocket())
  3. when a player opens the inventory, it displays all worn items, wielded item, and all contained items, and the player can interact with them just as before. Additionally can Drop them with multidrop. This obsoletes the Unload function for everything that is not a magazine. (and guns with internal mags)
  4. When interacting with a container in the inventory, it is possible to micromanage what items are in which pocket, by interacting with them:
    image
    image
    (oops, it says holster item)
    image

These particular screens is a bit of a WIP but needs some UX opinions outside my own. One comment I have about it is that in the "put in" screen it should not allow you to select the item itself or anything it contains. open just opens another inventory with just that item. i'd actually consider removing this one if it's too much, but i know how our playerbase is with hoarding things.
7. Advanced inventory should work just as before. Currently it's a little broken in this PR because of item indices.
8. Crafting should be able to find items inside of other items for the crafting_inventory() stuff. i.e .it should work as before.
9. Generally I want to remove all the logic where you interact with a container as if you're interacting with the item itself, as it confuses the code and now you can have multiple items inside of items. Maybe we can keep it for select things, like checking if there's only 1 type of thing inside it first.

Describe alternatives you've considered

nerf toolboxes

Testing

This project has been through a lot of iterations of testing, and it is not done at that. I've made a number of releases on my own fork that wonderful people have hammered on for me, and thisproject has gone through 3 pretty nasty rebases. I'm expecting some bugs that i've already fixed.

Here's some notes on bugs that i'm aware of pre-rebase:

  • mods don't work, i need to finish the json for those
  • volume display shows incorrect values (adds non-rigid nested container volume)
  • you can't wear an item if you don't have inventory space for it or it's not wielded
    this is related to Character::i_add(). i've had a very short conversation with ralreegorganon about how it needs a better api, but I haven't done much that information.

Additional context

I'm going to need some help putting a bow on this. The main thing i want is some guidance into splitting this into commits/prs that make it mergeable - i already have the general idea that i can split off the JSON (i've done part of that already, but not all of it) and write some json loading code to be a little easier on 3rd party mods. I'll accept help on fixing up the weird bugs that are obvious too, and/or some figuring what certain functions do via discord as i'm going to be hammering away at this for a couple few days to wrap up the additional obvious bugs.

Sponsor me maybe?
image

Special mentions:
@Brian-Otten and @wapcaplet have helped me fill out the json for containers.

@KorGgenT KorGgenT added Game: Mechanics Change Code that changes how major features work [JSON] Changes (can be) made in JSON Items: Magazines Ammo holding items and objects. [C++] Changes (can be) made in C++. Previously named `Code` Items / Item Actions / Item Qualities Items and how they work and interact labels Apr 9, 2020
@KorGgenT KorGgenT added this to the 0.F milestone Apr 9, 2020
@KorGgenT KorGgenT marked this pull request as draft April 9, 2020 11:13
src/advanced_inv.cpp Outdated Show resolved Hide resolved
src/advanced_inv.cpp Outdated Show resolved Hide resolved
src/avatar.cpp Outdated Show resolved Hide resolved
@@ -2095,7 +2095,7 @@ int Character::i_add_to_container( const item &it, const bool unloading )

const itype_id item_type = it.typeId();
auto add_to_container = [&it, &charges]( item & container ) {
auto &contained_ammo = container.contents.front();
item &contained_ammo = container.contents.first_ammo();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can someone sanity check this line for me? it wasn't actually clear if we were looking only into a magazine, or if "ammo" was just being used as a general word for "an item this will have"

src/character.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
src/character.h Outdated Show resolved Hide resolved
// skip food without comestible, like MREs
if( const item *it_food = it.get_food() ) {
if( it_food->get_comestible()->comesttype == "DRINK" ) {
if( !preserves && it_food->goes_bad() && has_near( zone_type_id( "LOOT_PDRINK" ), where, range ) ) {
if( it_food->goes_bad() && has_near( zone_type_id( "LOOT_PDRINK" ), where, range ) ) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: moving "preserves" into other stuff has ramifications for how zones store things.

src/crafting.cpp Outdated Show resolved Hide resolved
src/crafting.h Outdated Show resolved Hide resolved
src/item_contents.cpp Outdated Show resolved Hide resolved
src/item_pocket.cpp Outdated Show resolved Hide resolved
src/item_pocket.cpp Outdated Show resolved Hide resolved
src/item_pocket.cpp Outdated Show resolved Hide resolved
@Inglonias
Copy link
Contributor

while I understand that this is a draft that's not meant to be merged as-is, I'm surprised at how few commits there actually are. Did you squash a bunch of commits together?

@Rivet-the-Zombie
Copy link
Member

Squee?

@kevingranade
Copy link
Member

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

https://discourse.cataclysmdda.org/t/adding-a-mod-to-a-world/23546/13

@romanpogosov-usa
Copy link

Bugs and quality of life decrease this change introduced makes game almost unplayable.

@KorGgenT
Copy link
Member Author

Thank you for your excellent suggestion and detailed explanation on how to improve it.

@notazed
Copy link

notazed commented May 26, 2020

What do you think about this user's suggestion from #3671 (comment) for using a tree list view for inventory management?

Use to scroll, and to expand/collapse individual containers. And use enter to select and move individual items (like the sort armor window), without having to insert or open a window for each item/container?

                                           Volume          Storage
- B [ || backpack                            2.00       4.50/10.00
  - w / || copper spear                      1.50
  -   ) 3 plastic bottles                    1.50        1.50/1.50*
    -   ~ clean water (6)                    1.50
  -   ) 2 plastic bottles                    1.00        0.00/1.00*
  + b [ canvas bag                           0.25*       0.00/1.00
  -   ) tin can                              0.25        0.25/0.25*
    -   % mashed pumpkin (fresh) (cold) (1)  0.25
+ j [ |\ jeans                               2.00        0.25/0.50
- f [ || army jacket                         3.00        1.50/3.50
  - s , hobo stove (35)                      1.50        0.18/0.25*
    -   = tinder (35)                        0.18

+ is a closed container, - is opened

Imo it would make this change much less cumbersome and more intuitive to use.

@KorGgenT
Copy link
Member Author

i think it's a nice stretch goal. i put it in the project, lower priority than pocket favorites.

@kevingranade
Copy link
Member

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/1125

Brambor added a commit to Brambor/Cataclysm-DDA that referenced this pull request Sep 17, 2024
Brambor added a commit to Brambor/Cataclysm-DDA that referenced this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Game: Mechanics Change Code that changes how major features work Items / Item Actions / Item Qualities Items and how they work and interact Items: Magazines Ammo holding items and objects. [JSON] Changes (can be) made in JSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Saner Inventory through Unified, Nestable Containers [$255]