Item contents refactor (holster refactor 1 of 2)

[KorGgenT]

Summary

SUMMARY: Infrastructure "create item_pocket and item_contents class definitions"

Purpose of change

this is the first of two PRs to refactor holster to use the new item_pockets, which are part of the greater project "nested container system"
refer to #3671 for plenty-o-discussion

Describe the solution

My main idea behind the nested container system revolves around the idea of a "pocket", which is a small space that an item can have one or more of to contain items. A pocket has several pieces of metadata to define what kind of items are allowed to be placed in them:

• rigid, boolean; determines whether the pocket expands or not.
• type, enum; determines what kind of pocket this is. meant to be used to designate things later on, like magazines. right now there's two types i'm actually using: LEGACY_CONTAINER and CONTAINER. LEGACY_CONTAINER is basically the old way of putting items into items; they just lump them all together, like item mods, contents, liquids, magazines, etc. CONTAINER will be the main "storage" type; you can generally freely put things in and take things out of this type.
• max_contains_volume, volume: the max capacity in volume this pocket can contain.
• max_contains_weight, mass: the max capacity in weight this pocket can contain.
• min_item_volume, volume: the minimum volume of item that can be placed into this pocket. lots of holsters have minimums.
• spoil_multiplier, float: the multiplier for how quickly the contained item spoils. not used just yet, but planned to in the near future
• weight_multiplier, float: the multiplier for how much weight the contained items actually weigh. not likely to be used for vanilla much, but definitely planned to be used for magiclysm. should already work in json.
• moves, int; how many base moves it takes to take out and put items into this pocket
• fire_protection, bool: determines whether items inside will explode or not if the container this pocket is attached to is caught on fire
• watertight, bool: can contain liquids
• gastight, bool: can contain gas
• open_container, bool: this pocket will spill its contents if placed into an inventory
• flag_restriction, [ string ]: the flags the items must have in order to go into the pocket. if this is empty there is no restriction

an item's contents will be replaced by the item_contents class.
The data structure is such that an item_contents has a std::list<item_pocket> and an item_pocket has a std::list
in most applications, the item_pocket functions should never need to be accessed outside of item_contents functions, as this class is directly responsible for walking through the item_pocket.
Much of the functions as such are named the same, but the item_contents functions are usually a for loop.

Testing

tested together with the other PRs as a whole, for several days and via several helpful volunteers.

[jbytheway]

Consider adding a new test to tests/iteminfo_test.cpp for your new iteminfo features.

[BevapDin]

Where in the code are you new classes actually used? I don't see a change to items::contents, I don't see any changes to the itype (to store the new pocket data), or to item (to store pocket contents).

I also don't see any JSON definitions for the pocket data.

In other words: this adds a bunch of dead code.

[KorGgenT]

Where in the code are you new classes actually used? I don't see a change to items::contents, I don't see any changes to the itype (to store the new pocket data), or to item (to store pocket contents).

I also don't see any JSON definitions for the pocket data.

In other words: this adds a bunch of dead code.

#36660
broken up in order to reduce the line count

[BevapDin]

But we can't really test this PR as the code added here is never run. I suggest you only add code that is actually used within a PR. For example the next PR (step 2 of 3) uses item_pocket and item_contents, so add those classes in that PR. But it never uses pocket_data, so you don't need to add it there.

[KorGgenT]

I see... I simply did not want to have a pr with 2k lines of change, but i suppose i can combine this one with that one?

[KorGgenT]

note to self: do a sanity check on wherever i'm calling item_contents::all_items() to make sure the item doesn't actually need its pointer when it's used

[KorGgenT]

sanity check done

[KorGgenT]

note: there's some kind of quantum holster bug i need to fix still
edit: fixed

[codemime]

This PR is not a refactor: it's a sketch which adds loads of code for the sake of a minor part of the new feature. The idea behind pockets is interesting by itself, but it's an optional nice-to-have thing, not the essence of the unified nestable containers. I mean those can be made fully functional and smoothly playable with no pockets at first (or even at all). My concern is that the proposed implementation featuring pockets is somewhat cumbersome in terms that it would hinder the actually hard parts of the task instead of helping them.

I'd suggest abandoning it for now and focusing on evolving the proper container-containee relationship, improving the existing code as you go. Then you could address the major tasks outlined by Kevin in #3671 (comment) to get the new system up and running, making it so that pockets could fit into it nicely.

[KorGgenT]

I don't really understand, but i guess i can close this pr until i have it all done i guess, like i did magiclysm? I have some specific plans and my own concerns now that this PR has been in the works for a long time, and I can outline them if it helps any:

• item::contents as a list is a public member and is accessed everywhere. It really needs some protection in order for the various rules we have currently to be applied generally without having special rules in various parts of the code. an idea i had for pockets is that they'd have exactly that code, so you can for example do item::put_in() or even item.contents.put_in()
• the above means that if item::contents is kept as-is, and i don't do the pocket stuff, it'll make a lot more work for something i've planned for a very long time to bake in anyway. specifically extra migration code in savegame_json and extra loading code that might even get a little messy.
• i'm thinking maybe the hard part you're referring to is the packing problem? by cumbersome, do you mean it'll take more nested loops? I'll admit I haven't looked into that particular problem yet, as I've been focusing on learning the inventory display code.
• i'm thinking pockets will help smooth over certain cases where it only makes sense for one item to be inside one, like liquids and magazines. additionally, this means I expect that this will make the multimag project fairly straightforward, though again i haven't exactly gotten too familiar with the magazine code.

[codemime]

I don't really understand, but i guess i can close this pr until i have it all done i guess, like i did magiclysm?

I'd be advocating for the step-by-step approach. Even this PR (more than 2000 new lines of executable code) is well beyond being comfortably reviewable (Kevin once suggested 200 as a good upper limit).

item::contents as a list is a public member and is accessed everywhere. It really needs some protection in order for the various rules we have currently to be applied generally without having special rules in various parts of the code. an idea i had for pockets is that they'd have exactly that code, so you can for example do item::put_in() or even item.contents.put_in()

This looks like an excellent first step. item::put_in is better because it allows more control from item (like updating caches, triggering events etc).

the above means that if item::contents is kept as-is, and i don't do the pocket stuff, it'll make a lot more work for something i've planned for a very long time to bake in anyway. specifically extra migration code in savegame_json and extra loading code that might even get a little messy.

I don't particularly see why. If you have, say, item::put_in(item), it's easy to change it to item::put_in(item, pocket). Encapsulation is the key here.

i'm thinking maybe the hard part you're referring to is the packing problem?

Yes. It's one of the tricky parts I guess (especially considering proper stacking of items and different handling costs). The packing algorithm should be reasonably fast in case of vast amounts of items. At the same time, it should almost never make the player "fix" it (in other words, it must never yield unacceptable results).

as I've been focusing on learning the inventory display code.

That's the other tricky part, yes. I assume inventing convenient UI for this won't be trivial.

by cumbersome, do you mean it'll take more nested loops?

The container handling code in master can be improved in many ways to help arranging the new feature. Without extensive refactoring it's nearly impossible to add new code there in a clean way.

i'm thinking pockets will help smooth over certain cases where it only makes sense for one item to be inside one, like liquids and magazines. additionally, this means I expect that this will make the multimag project fairly straightforward, though again i haven't exactly gotten too familiar with the magazine code.

I suppose there should be certain constraints, of course (like if a container contains liquid/powder, it can't contain anything else, and vice versa), but in the generic case, items should be able to nest indefinitely.

[KorGgenT]

all right, that makes me feel much better about what i decided to do when you posted the first time. I decided that i'm just going to need to hammer away at it in a branch and make a checklist for things i need to pr at a time to try to keep it as small as i can. i will continue to work with everyone on the discord and see how many prs i get out of it