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

Solution to pocket non-transparency #78551

Open
Brambor opened this issue Dec 13, 2024 · 5 comments
Open

Solution to pocket non-transparency #78551

Brambor opened this issue Dec 13, 2024 · 5 comments
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Game: Mechanics Change Code that changes how major features work Map / Mapgen Overmap, Mapgen, Map extras, Map display Mechanic: Pockets <Suggestion / Discussion> Talk it out before implementing

Comments

@Brambor
Copy link
Contributor

Brambor commented Dec 13, 2024

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

You can see a jacket across the room has string in a non-transparent pocket. This has been discussed before:

Solution you would like

remembered_contents

remembered_contents stores exact copies of what the avatar saw at that tile the last time. Saw differs from what is there by items hidden in non-transparent pockets. If the tile contains a slightly damaged jacket with a string in a non-transparent pocket, remembered_contents will contain a slightly damaged jacket without the string.

This alone would be super slow and would double the amount of info we store about items. Which is why we will consider two or three special cases:

  • avatar_knows_contents
  • avatar_knows_from_far
  • The absence of those two is a tile the avatar hasn't seen yet.

avatar_knows_contents

In the map data, store per tile if the avatar knows about the tile contents in avatar_knows_contents. Store in it:

  • true the avatar knows about all items (including their contents)
  • false they don't know about any detail about any item
  • undefined if the avatar didn't see the tile at all, so they know nothing about it.

If the avatar is next to a tile, they see into all the pockets (Except for safes!), so set avatar_knows_contents = true, drop remembered_contents, since the avatar knows what is there.

Possible implementation

We can store it in a tree-like structure:

  1. On an overmap tile level avatar_knows_contents is
    • true if the avatar explored the whole overmap tile
    • false if they didn't see the chunk at all or they don't know about any item that is there
    • undefined otherwise, and go to step 2
  2. Slice the overmap tile by layers (z-level), avatar_knows_contents is
    • true if the avatar knows about all the items at that z-level
    • false if the avatar knows about no items at that level
    • undefined otherwise (like they don't know what is in the jacket across the room), go to step 3
  3. Individual tiles, avatar_knows_contents is
    • true, if the avatar knows about all the items in that tile
    • false if they know about some (but not about like the content of a jacket) -> then remembered_contents needs to be defined

If the value is true or false at level 1, we don't have to (and shouldn't!) store the values on the lower levels (2, 3). Likewise for level 2.

Encoding of JSON map save data:

  • true is "avatar_knows_contents": true
  • false is "avatar_knows_contents": false
  • undefined is the absence of avatar_knows_contents

In cpp, avatar_knows_contents is optional<bool>, if it has_value(), then true and false are obvious. If it does not have a value, it is undefined. Or enum{true, false, undefined}.

Alternatively, store a std::set<tripoint_abs_ms> for when avatar_knows_contents is true. And solve the other options somehow.

We want a function that takes tripoint_abs_ms and returns avatars' knowledge of tile at those coordinates.

In an explored world the question typically is "Does the avatar know about tile X?" and the answer is "Yes, (they know about all tiles in that overmap tiles on that z-level)."


avatar_knows_from_far

We can add another helper bool avatar_knows_from_far. This is like avatar_knows_contents, but they don't know the contents of non-transparent pockets. If true, remembered_contents is not used. When asked about the knowledge of the tile, unlike avatar_knows_contents, we will not return the contents of the pockets.

remembered_contents is set (prior to) when an NPC steals an item from a pocket from a tile that had avatar_knows_from_far. This is important for crafting (explained lower).

Performance and code support

This requires any interaction with any item to first check if the avatar is doing it.

  • If they are, update avatar_knows_contents = true and drop remembered_contents.
  • If they are not and avatar_knows_contents is true or avatar_knows_from_far is true, then save current contents into remembered_contents and set avatar_knows_contents to false. Unless the interaction doesn't result in changing the items.

The problem is this will touch every place where the code changes an item anywhere. Spawning will be ok, we just set avatar_knows_contents to undefined on spawn. Unless the spawning is in avatar_knows_contents == true tile, then set remembered_contents and set avatar_knows_contents to false.

It will also touch the avatar just walking. When they are walking, we need to update all tiles the avatar sees by setting avatar_knows_from_far to true. Set avatar_knows_contents to true for all the tiles the avatar is next to.

V List all items and monsters menu

When viewing a tile with a non-transparent container from afar (tiles with only transparent containers behave as they used to):

  • If avatar_knows_contents is true show the contents like in the current implementation.
    • add Remembered contents: somewhere over there to tell the player these might not be up to date. (The game knows it is up to date, but that is a secret to the player.)
  • If avatar_knows_from_far is true, hide the non-transparent pockets content.
    • add You cannot see inside the pockets maybe even You would see, if you walked closer
  • If remembered_contents is set, show it.
    • add Remembered contents: somewhere over there to tell the player these might not be up to date. (The game knows it is not up to date.)
  • If the tile is not in the avatars' vision. We currently don't show items outside vision and this proposal doesn't change that.
    • This might change in the future. If this issue is implemented, it is a good step towards that. It would partially solve Item Snapshot #72697. But let's focus on the issue at hand!

Crafting

Usually, all reachable tiles in crafting will have avatar_knows_contents true. In that case, crafting will not change.

In case the avatar enters a dark room (avatar vision is 1, crafting reach is 5) they haven't been to before with lots of items and opens a crafting menu

  1. They will not see any items in the crafting menu since avatar_knows_contents is false for all the items.
  2. If they flicker a torch on and off, avatar_knows_from_far will be set to true for each tile and all items will be shown except for those inside non-transparent containers.
  3. If the avatar visits all the tiles, we get to the original case, which doesn't change - see all items.

If the avatar tries to use an item in crafting they remember was there, this will happen:

  1. Let's assume the simulation makes the avatar go to the tile where the items are. It currently does not. This is a separate issue.
  2. The crafting recipe is confirmed by the player; the avatar walks to the tile; discovers the items aren't there; set avatar_knows_contents to true for that tile. Then the crafting GUI no longer shows these items.
  3. popup You remember that %s was there, but it is not there anymore!
    • If you need 1 string and remember 10, visit all the tiles first.
  4. Cancel the crafting. If the avatar walked somewhere, walk back to the original tile.

Describe alternatives you have considered.

I thought it was a per item problem, but it is a per map tile problem:

Per item (partial) solution

Swap and adjust the first 2 steps Erk mentioned to these:

  • Is this container transparent? Then show the contents. If not,
  • have you looked in this container before? Then show the saved cache. If not,

Remembered contents

Whenever the avatar is close to a non transparent container, update what the avatar remembers is inside. Let's call the variable remembered_contents.

We also need bool avatar_saw_contents denoting if the avatar ever saw that item at all. Set to false by default. If we didn't have avatar_saw_contents, then we would update items the avatar never saw, notably all the items that just spawned in a chunk that just generated.

Note: We cannot just do:

bool item::avatar_saw_contents()
{
    return remembered_contents.empty();
}

This doesn't differentiate between a known empty container and a container with unknown contents, which should show "contents hidden" in V (step 4 of Erk's algo).

As an implementation detail to prevent caching what is in the container: save bool avatar_knows_contents. When true, don't use remembered_contents. When false, store remembered_contents - a full copy of what was in the container the last time the avatar saw inside.

Performance and code support

This requires any interaction with any item to first check if the avatar is doing it. If they are, update avatar_knows_contents = true and drop remembered_contents. If they are not and avatar_saw_contents is true and avatar_knows_contents is true, then save current contents into remembered_contents and set avatar_knows_contents to false.

The problem is this will touch every place where code interacts with adding or removing an item from anywhere. Spawning will be ok, we just set avatar_saw_contents to false on spawn.

It will also touch the avatar just walking. When they are walking, we need to update all containers the avatar sees and set avatar_knows_contents to all the items on all tiles the avatar is next to.

V List all items and monsters menu

When viewing a non-transparent container from afar add Remembered contents: somewhere over there to tell the player these might not be up to date. if( avatar_knows_contents ) show the contents like usual. if( !avatar_knows_contents ), show the remembered_contents.

Crafting

Crafting will pull the data from the cache. Usually all items will have avatar_knows_contents true so everything will work as it used to.

In case the player enters a dark room with lots of items and opens a crafting menu, they will not see any items in the crafting menu. They will not see into the containers since avatar_knows_contents is false, so no items within containers will be shown. avatar_knows_contents will double as not knowing about the item at all, so don't show the item either.

Additional context

Ref:

@Brambor Brambor added <Suggestion / Discussion> Talk it out before implementing <Enhancement / Feature> New features, or enhancements on existing Game: Mechanics Change Code that changes how major features work Map / Mapgen Overmap, Mapgen, Map extras, Map display Crafting / Construction / Recipes Includes: Uncrafting / Disassembling [C++] Changes (can be) made in C++. Previously named `Code` Mechanic: Pockets labels Dec 13, 2024
@kevingranade
Copy link
Member

This needs actual use cases driving it, not "I want to be able to remember isolated facts".

If the use cases are mostly in looting activities, I've said many times what I want the solution to be, which is a looting system where you trigger an activity to explore and loot a room and it abstracts the process of discovering the contents and gives you a list of found items to choose from.
If the use case is, "remembering where I found a X that I realized I need", the the solution is a limited amount of memorization or recording of item locations as we discussed in #72697

If the use case is remembering the location of items around your base, that would lead to a very different implementation that remembers a much more limited amount of data in more restricted areas.

What we definitively do not need is a global system where the player remembers details of everything they've ever seen.

@Brambor
Copy link
Contributor Author

Brambor commented Dec 13, 2024

This is not meant to be useful. This is meant as a bug fix for

  • You being able to see items in non-transparent pockets across the room.
  • Crafting can show you items behind a wall if the tile is reachable.
  • Crafting can show you items you didn't see.

This will actually present less information to the player. The game needs to remember what the player saw only so that it doesn't show some things.

Any remembering of items outside the bubble is not presented to the player. I know that is not getting merged which is why I don't want to discuss it here since this issue is not meant to address that at all.

I can see the label enhancement was misleading too.

@Brambor Brambor removed the <Enhancement / Feature> New features, or enhancements on existing label Dec 13, 2024
@Brambor
Copy link
Contributor Author

Brambor commented Dec 13, 2024

Are you still against it @kevingranade?

Breaking the status quo should the avatar just not know what is in the non-transparent barrell across the room, even if they looked inside it?

Or is this issue just not the right answer to the bugs?

@Brambor
Copy link
Contributor Author

Brambor commented Dec 13, 2024

It would show a couple of extra items for tiles in your view that you looked into before:

  • Clothes in a non-transparent closet.
  • Coal in a forge.
  • etc.

@kevingranade
Copy link
Member

Then you need to outline the problem you are addressing in this issue, especially when it's as expansive as this and so closely related to multiple earlier proposals you have made. I followed the references and have some idea of what you're getting at now, but that shouldn't be necessary, and it's still not clear what the scope of the feature is.

For example, when you say "store it in the map data", do you mean locally or forever? Locally is maybe fine, then the player forgets it when they leave the area, forever is just "memorize everything you ever see" in a different package. There's also a MASSIVE problem with all this "avatar saw X one time" stuff stored on the map, because the identity of the avatar can change over time. Again if it's local it's fine, just clear it all if the avatar changes identity and start building it from there.

The use cases as outlined in previous issues also point to a local/transient memory, it's basically:
When I examine a closed container and reveal its contents
then I enumerate all the known items in the area (i.e. with the V -> items menu)
I expect to see the items that were revealed from examining the closed container.
That doesn't imply persistence (which is implied by storing it on the map), it implies modeling transient short term memory of things the player has interacted with recently.

There's also a strong assumption in your text that remembering necessarily means remembering every detail of an item, that's another issue that you look to the use cases to distinguish, do we really need a copy of an item (a rather large object) or do we just remember an itype_id (a single pointer)?

Another thing that use cases do is establish the impact of the problem, which helps when we are deciding whether "It will also touch the avatar just walking. When they are walking, we need to update all tiles the avatar sees by setting avatar_knows_from_far to true" is actually worth the very non-trivial cost of doing it. I strongly doubt that is the case. This proposed element of the feature is also problematic from the point of view of matching the model to how people operate. People are not generally spending a bunch of effort memorizing every item that comes within their field of view, this is not a reasonable ability to give the avatar.

There are a bunch of other issues, but those are the most glaring ones.

My recommendation for following up on this that addresses my strong technical concerns about the proposal is:
add avatar::std::unordered_map<abs_bub_tripoint_ms, std::vector<itype_id>> recently_seen_items;
Add to this when the player interacts with a tile via examining it or even with View Items.
Prune the list on map shift once it's far away.
Don't serialize it.

The smaller the scope, the less problematic it is to go ahead without getting into the weeds with enumerating all your use cases etc,

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` Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Game: Mechanics Change Code that changes how major features work Map / Mapgen Overmap, Mapgen, Map extras, Map display Mechanic: Pockets <Suggestion / Discussion> Talk it out before implementing
Projects
None yet
Development

No branches or pull requests

2 participants