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

Fill interior pockets #53807

Closed
bombasticSlacks opened this issue Dec 27, 2021 · 10 comments
Closed

Fill interior pockets #53807

bombasticSlacks opened this issue Dec 27, 2021 · 10 comments
Labels
<Enhancement / Feature> New features, or enhancements on existing Info / User Interface Game - player communication, menus, etc. Items: Containers Things that hold other things

Comments

@bombasticSlacks
Copy link
Contributor

bombasticSlacks commented Dec 27, 2021

Is your feature request related to a problem? Please describe.

Currently your character will almost never attempt to fill an item inside of another item when picking up by default. This should be something you can enable.

Partially related to #53806 and would almost certainly need #53643 implemented first

Also related to the discussion on #53737 once this issue is resolved a rigid container would go back to being available space if this was enabled.

Solution you would like.

Add a button to the pocket pickup menu for "fill if inside" or something similar disabled by pockets by default.

If this is enabled when an item is picked up and it's looking for the best pocket to put it in, pockets inside pockets (that have been enabled) would be considered. So for example if you want a stocked first aid kit, you could load it up with bandages and stuff. Or if it's early days you can pack items in tight against your cast iron pan (storing them inside it).

Describe alternatives you have considered.

No response

Additional context

No response

@mqrause
Copy link
Contributor

mqrause commented Dec 27, 2021

Currently your character will never attempt to fill an item inside of another item when picking up by default.

This is not true. The best pocket algorithm does consider those pockets and will pick them if certain conditions are true. See item_pocket::better_pocket for details.
There is a lot of room for improvement, though.

@bombasticSlacks
Copy link
Contributor Author

I apologize I'm with family for the holidays and haven't had my dev environment (or I'd just be fixing these things instead of writing issues). I know I'm doing a lot of talking and not a lot of solving but the plan is to do a lot of this in the next few days. You are right that under some circumstances it will pick an interior pocket, Usually by setting the pocket to higher priority (tho I still can't get it to work with rigid containers). However https://github.com/CleverRaven/Cataclysm-DDA/blob/master/src/item_pocket.cpp#L344 basically short circuits it (and before rigid containers test too).

What I'm proposing here would fix this issue without having things go in weird pouches for people unexpectedly. so that line would look something like:

if( nested && !data->load_internal() ) {

where load internal is the boolean for if this feature is on. So if you have this enabled an interior pocket is treated as just as valid as a non internal as opposed to now where it's only tested partially.

@mqrause
Copy link
Contributor

mqrause commented Dec 27, 2021

Yeah, I just wanted to clarify it's already there to be built upon.

@mqrause
Copy link
Contributor

mqrause commented Dec 27, 2021

However https://github.com/CleverRaven/Cataclysm-DDA/blob/master/src/item_pocket.cpp#L344 basically short circuits it (and before rigid containers test too).

I think I need to explain the reasoning there, too. This is meant to bypass pocket properties that don't matter on contained items. Remaining volume for example is meant to choose the smallest possible pocket so you don't block pockets that can hold large items with smaller items that could go elsewhere. This would prefer the pockets of a jeans inside a backpack over the backpack itself if it wasn't skipped. And rigid containers weren't meant to literally get filled when inside another item. Especially open containers like buckets can't even contain items when inside another item. They are simply meant to not use up the space of their pocket in the parent pocket.
The obtain cost check is even more problematic, because, this check only looks at the pocket data and doesn't take nesting into account. So getting a weapon from a holster inside a backpack inside a body bag would be considered faster than getting it directly from the body bag. The actual obtain cost when trying to wield it for example should be correctly calculated of course.

Most of this isn't set in stone of course, But the way to go here is to add more rules above the skip condition, for example by evaluating more complex pocket settings. Simply bypassing the bypass can and will lead to unexpected and unwanted results. And it can already be achieved by increasing the priority anyway.

@bombasticSlacks
Copy link
Contributor Author

I feel like we are going in circles. A 5L bucket filled with 1L of water in a backpack takes 5L of space not 1L. A hard .5L pill bottle with two codein in it doesn't take .02L it takes .5L.

Especially open containers like buckets can't even contain items when inside another item. They are simply meant to not use up the space of their pocket in the parent pocket.

What you are describing here is a non-rigid container. That is what non-rigid pockets do. If rigid pockets also fill their space as their contained volume increases there shouldn't be a distinction between rigid and non rigid containers.

If you aren't aware as well there is a desire to make it so that obtain costs are modified by how cluttered a container is. If this happens having a large rigid container would be good for more than just meta organization. It would in that case reduce the obtain cost of getting the parent container item before looking for the small item ex: finding a pill bottle is easier than finding a loose pill.

That said I am aware of the obtain cost problems #53347

There could be work done on this system outside of what this issue recommends but if you want the functionality you were arguing should be default in #53737 you need to put stuff in the rigid containers your bag can't just magically grow to have the difference in volume. So the point of this is if you're lugging a 5L bucket and you need the space you can toggle it to be treated as a valid pocket for normal evaluation.

You may have also seen I've posted in this issue my interest in implementing #53643 first which would cover most use cases and as I say would be done before I add this toggle which would be a final QOL.

@mqrause
Copy link
Contributor

mqrause commented Dec 28, 2021

I feel like we are going in circles. A 5L bucket filled with 1L of water in a backpack takes 5L of space not 1L. A hard .5L pill bottle with two codein in it doesn't take .02L it takes .5L.

Especially open containers like buckets can't even contain items when inside another item. They are simply meant to not use up the space of their pocket in the parent pocket.

What you are describing here is a non-rigid container. That is what non-rigid pockets do. If rigid pockets also fill their space as their contained volume increases there shouldn't be a distinction between rigid and non rigid containers.

I'm talking about pockets like this:

        "rigid": true,
        "open_container": true,

They cannot contain anything at all when they are inside another item, simply because you have no way to ensure anything inside them will really stay there. But that doesn't turn them into solid inaccessible blocks.

I don't get the impression you actually listen to my arguments, so I'll leave it at that.

That said I am aware of the obtain cost problems #53347

That's a different issue. I'm talking about item_pocket::obtain_cost, which only looks at the pocket data, versus item_location::obtain_cost which is responsible for that issue.

@wapcaplet wapcaplet added <Enhancement / Feature> New features, or enhancements on existing Info / User Interface Game - player communication, menus, etc. Items: Containers Things that hold other things labels Dec 28, 2021
@PatrikLundell
Copy link
Contributor

mqrause probably made it clear already, but buckets belong to the category of open containers that can't contain anything unless they're carried in the hands or standing on the "ground".

Sure, in real life, you can put an open item, like a frying pan, into a backpack and use the space that's "inside" the frying pan for backpack storage (not frying pan storage). I don't know if the current system accounts for that, but if it doesn't, you'd have to "give back" the "internal" space of those items to their containers rather than trying to use them as pockets, but then you get into weird situations where you're somehow managing to use the space inside these open containers to contain things that won't fit there, like e.g. the space of two buckets inside a backpack collectively containing a box that won't fit in either bucket (while, on the other hand, you can usually fit one bucket inside of the other so they mostly overlap in real life, as they're shaped [roughly conical] to permit that).

@mqrause
Copy link
Contributor

mqrause commented Dec 28, 2021

It tries to account for it, but it's currently bugged. item::can_contain will first check if there's enough space in the pocket itself and if not check any contained rigid pockets if they can contain the item. Just the actual insertion is missing that kind of logic, so it won't get inserted even if it would fit in a contained rigid pocket. So if it got fixed, it'd already account for your "weird" example because it doesn't just give back the space.

@bombasticSlacks
Copy link
Contributor Author

Well it would explain why we've been going in circles for days. It wasn't at all clear to me that what you've been talking about are open_containers. I've been trying to fix bugs and issues primarily for toolboxes, prescription bottles and jugs/bottles. I didn't even realize open_container is a flag. On investigation it is used around 40 times in the code and their are over 800 rigid items. So I'll keep this in mind going forward and make sure that my changes don't effect the 5% of cases you are talking about.

@bombasticSlacks
Copy link
Contributor Author

looks like this has been sorted out via the above merged PR. Glad someone got a solution working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Enhancement / Feature> New features, or enhancements on existing Info / User Interface Game - player communication, menus, etc. Items: Containers Things that hold other things
Projects
None yet
Development

No branches or pull requests

4 participants