Transparent pockets? (Items in pockets visible from afar?)

[Brambor]

Continuing discussion opened in #73400 about transparent pockets.

[Brambor]

          > I don't recall how much of my recommendation on that first pr ever got implemented.

We need to track:

* have you looked in this container before? Then show the contents.  If not,

* Is this container transparent? Then show the contents. If not,

* Is this container sealed? Then we should check if there's some kind of label and display that. If not,

* Don't show the contents and display "contents hidden" in `V`.

From that list only transparency got implemented. Everything else is still hidden.

Is this making it so that you can identify the contents of a backpack from across the room using V, because it's not sealed? If so, I'm very much not in favour of that

I agree with that. The question is if you can tell what's inside a pocket when it's not within reach to actually check the contents.
And on top of that, a change like that should be its own PR, it's kinda off topic for this one. Your implementation should simply work with whatever the known contents give you. Fixing/improving the things you get from that is a different thing.

Originally posted by @mqrause in #73400 (comment)

[Brambor]

Looking back, a reason why I was so quick to change transparent() is the doc comment, which contradicts implementation.

item_pocket.h

// open pockets are always considered transparent  //// "open" probably means "unclosable" or "bucket-like", as @mqrause explains bellow
bool transparent() const;
(...)
// the pocket will spill its contents if placed in another container  //// so this is unclosable container, rather than open container
bool open_container = false;
(...)
// the contents of the pocket are visible
bool transparent = false;

item_pocket.cpp

bool item_pocket::sealed() const
{
    if( !sealable() ) {
        return false;
    } else {
        return _sealed;
    }
}
(...)
bool item_pocket::transparent() const
{
    return data->transparent || ( data->open_container && !sealed() );  //// should definitely be _sealed instead of sealed()
}

[Brambor]

@mqrause and @I-am-Erk, lets continue the discussion here.

[mqrause]

An "open" pocket is something like a bucket. Meaning there's nothing holding the contents inside it besides gravity. Originally it was only used for "this container spills its contents if you try to put it into another container", but it got extended to "you can see the contents from a distance". It is explicitly not the opposite of "sealed". Although I'm not sure anymore how an open pocket could be sealed and why I added that check. But I assume there was a reason.

[Brambor]

Give me a sec, I am writing the same thing, sorry to call (ping) you before I had this done :(

[Brambor]

Can you review my code assessment made above? #73445 (comment)

My conclusion: change sealed() to _sealed and fix comments

[mqrause]

Yeah, the comments can definitely be improved. I'm not sure what using _sealed would achieve, though? It seems that would just enable weird edge cases where _sealed is true for whatever reason despite the pocket not actually being sealable.

[Brambor]

Yeah, the comments can definitely be improved. I'm not sure what using _sealed would achieve, though?

If the container isn't sealable, the transparent() returns false. But it should return true since it is not _sealed. Currently, if a bucket isn't sealable (which I would expect), it is considered not transparent, which is weird.

It seems that would just enable weird edge cases where _sealed is true for whatever reason despite the pocket not actually being sealable.

That sounds like a separate issue. If it happens, it is a different bug. But I think this is currently bugged in for the above-mentioned reason.

[Brambor]

Closely related:

Cataclysm-DDA/src/item_contents.h

Lines 121 to 123 in 53a0a73

	/** returns a list of pointers to all visible or remembered content items that are not mods */
	std::list<item *> all_known_contents();
	std::list<const item *> all_known_contents() const;

My understanding of the comment differs from what I understand the method does, The method all_known_contents returns only top level (not nested) items,

So the comment should be changed and the function should be named all_top_known_contents to be consistent with surrounding functions. Or the implementation should change, to call pockets recursively:

Cataclysm-DDA/src/item_contents.cpp

Lines 1737 to 1742 in 53a0a73

	std::list<item *> item_contents::all_known_contents()
	{
	return all_items_top( []( const item_pocket & pocket ) {
	return pocket.is_standard_type() && pocket.transparent();
	} );
	}

[mqrause]

If the container isn't sealable, the transparent() returns false. But it should return true since it is not _sealed. Currently, if a bucket isn't sealable (which I would expect), it is considered not transparent, which is weird.

I think you're misreading that. If it's not sealable it's not sealed. So an unsealable bucket is transparent. Unless you can show me it's broken ingame.

Closely related:

My understanding of the comment differs from what I understand the method does, The method all_known_contents returns only top level (not nested) items,

So the comment should be changed and the function should be named all_top_known_contents to be consistent with surrounding functions.

That's correct and I agree. It's used in recursive contexts, so the implementation works as intended and I wouldn't change it, unless you want to do more/different stuff with it.

[Brambor]

Unless you can show me it's broken ingame.

Fair, I didn't find it. So let's leave it.

[Brambor]

Pockets that aren't transparent should also not leak info about their weight (can have: Weight: at least 0.05 kg). A person cannot see that across the room.

[Brambor]

I am letting go for now. But it is a problem that right now nobody really knows how it should be. It is not easy to figure out and it is not trivial to implement.

[github-actions]

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.