[WIP]advanced_inv refactor

[akozhevn]

Summary

SUMMARY: Infrastructure "Refactor advanced inventroy"

Purpose of change

These changes make advanced inventroy extendable, so child classes can be created

Describe the solution

Describe alternatives you've considered

Additional context

I will break this into multiple commits, but keep it as a reference to what the goal is (approximately)
List of PRs so far:

• Advanced inventory split Advanced inventory split #34630
• Advanced inv area data Advanced inv area data #35050
• Advanced inv save settings Advanced inv save settings #35239

The following changes are not final, but reflect the implementation.

If you saw Eat menu: #33304
These are mostly unchanged:
comestible_inv_base -> advanced_inv_base
comestible_inv_pane -> advanced_inv_pane
comestible_inv_listitem -> advanced_inv_listitem
comestible_inv_area -> advanced_inv_area

All the special advanced inventroy functions are here:
inventroy_transfer

Fixes and changes: old behavior -> new

• Item count always shows items on the ground, insetead of vehicle. This also allows to put more than MAX_ITEMS into vehicle -> fixed to show vehicle items when appropriate
• Following steps: aim to [d]ragged location, close menu, release, open menu - will produce weird behavior where you can press [d] (even though it shows as invalid) -> [d]ragged behaves like a shortcut to location(+in_vehicle) instead of separate location
• Pressing the same location key repeatedly would swap it with other pane. That's likely a bug, because it doesn't behave that way when location has vehicle. -> Pressing location key while pane is already at that location does nothing
• Moving item to [w]ear moves them to inventory -> actually try to wear them, do nothing on failure
• Moving item to [w]ear from [a]ll produces error message -> behaves same as from any other source
• Added A+I+W - when you need ALL of the items
• Remove sort by damage - it doesn't work
• Remove [C]ontainer - tl;dr: useless functionality, high maintenance cost

long explanation:
What does it actually do?

• I believe the only thing you can do with it is reload a container with liquid

What's bad for user?

• hard to understand (I didn't know what it was, for like first 100 hours of play)
• doesn't filter items you can put in
• can't unload (even though it looks like you can);
• can't swap contents between containers (even though it looks like you can);

What's bad for dev?

• It's a huge hack - every location in the menu represents location in the world, but this represents an item and it's contents. To make up the difference there's a check and separate handling all throughout the code. Meaning more cases and chances for errors in any future dev.

What else?

• I believe [r]eload handles this functionality much better, so there's no need for it.
• If you really want the [c]ontainer-like behavior, I would rather add shortcut for [r]eloading (which is basically same thing but better)

[reed501]

• Pressing location key while pane is already at that location does nothing

Just to clarify: If there are items on the ground on the same tile as vehicle storage (i.e. on the ground under the vehicle because the vehicle tile ran out of space) will you still swap between the two?

[KorGgenT]

You need to break up this PR. It's too many lines of change for one PR.

[akozhevn]

• Pressing location key while pane is already at that location does nothing

Just to clarify: If there are items on the ground on the same tile as vehicle storage (i.e. on the ground under the vehicle because the vehicle tile ran out of space) will you still swap between the two?

No, you would have to press V. I believe it's same in the current implementation.

[KorGgenT]

• Pressing location key while pane is already at that location does nothing

Just to clarify: If there are items on the ground on the same tile as vehicle storage (i.e. on the ground under the vehicle because the vehicle tile ran out of space) will you still swap between the two?

No, you would have to press V. I believe it's same in the current implementation.

no, V is referring to the vehicle you're grabbing. he's talking about when you're looking at a space that has a vehicle on it, you can access the ground and the vehicle both: (screenshot to follow)

[akozhevn]

You need to break up this PR. It's too many lines of change for one PR.

How would I even go about it?
At the bare minimum, I want to split the 2 files into 8. Even assuming all the hacks and shared variables magically resolve themselves, that would produce same-ish size commit; right?

[akozhevn]

• Pressing location key while pane is already at that location does nothing

Just to clarify: If there are items on the ground on the same tile as vehicle storage (i.e. on the ground under the vehicle because the vehicle tile ran out of space) will you still swap between the two?

No, you would have to press V. I believe it's same in the current implementation.

no, V is referring to the vehicle you're grabbing. he's talking about when you're looking at a space that has a vehicle on it, you can access the ground and the vehicle both: (screenshot to follow)

Pretty sure you are referring to [D]ragged.
If you point left pane and right pane to the same location it will point first to vehicle and second to the ground (same as now). And if you repeatedly press location button with vehicle in it, nothing will happen (also same as now).

[KorGgenT]

You need to break up this PR. It's too many lines of change for one PR.

How would I even go about it?
At the bare minimum, I want to split the 2 files into 8. Even assuming all the hacks and shared variables magically resolve themselves, that would produce same-ish size commit; right?

Well, i'd start out with a PR that splits the file up with no code changes. if you split files and make changes all at once, the whole thing needs to be reviewed very carefully, but if you just do file splits someone can write a little script to see that the code is all the same.

[esotericist]

Have not attempted to review the actual code changes (as korg says, they're large enough to essentially be unreviewable), but just your description raises a couple of concerns:

Following steps: aim to [d]ragged location, close menu, release, open menu - will produce weird behavior where you can press [d] (even though it shows as invalid) -> [d]ragged behaves like a shortcut to location(+in_vehicle) instead of separate location

This runs the risk of being a significant functionality regression: the d location being a discrete location means if you select that as your target, close aim, then reposition so the dragged vehicle is in a different orientation from you, it's still selected when you reopen aim. If d is just used as a shortcut to location+vehicle container, then I would expect you'd have to press d for your target every single time you open aim without some really special handling. This would greatly increase the time it takes to loot a large area with a shopping cart.

Pressing the same location key repeatedly would swap it with other pane. That's likely a bug, because it doesn't behave that way when location has vehicle. -> Pressing location key while pane is already at that location does nothing

This is definitely a significant functionality regression: Swapping panes is deliberate, not a bug. this lets you swap which pane a location is in with a single keypress. I rely on this functionality constantly in my gameplay.

[akozhevn]

@esotericist

1. Good point, but should be an easy fix. Something like if the last location you opened is [D], the next time you open menu it automatically goes to new [D].

2. You can still swap, you just need to press the other pane location.

The problem I run into is let say I want to move items from [1] to [6]. I open menu press 1 tab 6 tab - at this point I expect things to be set up correctly, but actually pressing 1 doesn't guarantee to move pane to [1]. So instead of like 0.2 seconds it takes 5+ to set everything up.

What is the exact situation where you feel pressing active pane location is substantially better than pressing other pane location or tab?

The bug part I refer to is because the behavior is inconsistent. Let say [1] has vehicle and [9] doesn't. Pressing 9 repeatedly will now swap locations and pressing 1 repeatedly will not.

[esotericist]

1. Good point, but should be an easy fix. Something like if the last location you opened is [D], the next time you open menu it automatically goes to new [D].

As long as the functionality is preserved.

2. You can still swap, you just need to press the other pane location.

If there's a bug in one use case with a piece of otherwise useful functionality, the first resort should be to fix the functionality, not remove it.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

[ZhilkinSerg]

Feel free to reopen if you want working on it again.