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

Disassembly menu: highlight filtered components, always show required & components #78737

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Brambor
Copy link
Contributor

@Brambor Brambor commented Dec 23, 2024

Summary

Interface "Disassembly menu: highlight filtered components, always show required & components"

Purpose of change

Part of #73808

Describe the solution

I don't like my solution. I need to set cells to mutable and replace_cell to const so that it compiles. I am listening to any better solution!

  • Add on_filter_change to inventory_selector_preset
  • use it in the disassembly menu to highlight components from the filter e.g. d:copper wire
    • image
      Old image, update it after code changes.

Needs work

  • Now, it shows things you should not see, such as items behind a wall. It should use all_known_items().
  • Numerous TODOs I added across the code. Almost all of them need solving before this is ready.

Describe alternatives you've considered

Expand the capabilities more sustainably by implementing this somewhere else in the call chain. I would like that!

Testing

  1. Open the ( disassembly menu.
  2. Observe: items from further away are shown now.
  3. Observe: each column has its colour.
  4. Observe: New column CAN DISASSEMBLE.
  5. Try to disassemble something far away.
  6. Observe: It doesn't start disassembly, as expected.
  7. Filter for d:copper wire
    • Currently, it doesn't update the cache automatically as it should.
    • Cache can be updated manually by "Toggle language to English" option.
  8. Observe (if cache was updated) components are highlighted.

Additional context

@github-actions github-actions bot added Info / User Interface Game - player communication, menus, etc. [C++] Changes (can be) made in C++. Previously named `Code` labels Dec 23, 2024
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Dec 23, 2024
@Brambor Brambor changed the title Disassembly menu: show distant items, always show required components Disassembly menu: highlight filtered components, show distant items, always show required components Dec 23, 2024
@Procyonae
Copy link
Contributor

Maybe it's just me but I don't see why I'd ever want stuff out of reach showing up on the menu if it can't do anything with it (autopathing to it and then starting the activity). I appreciate you have that listed as a todo but unless you plan on doing it asap I think it'd be better if that part was disabled (or disabled by default and toggleable on) for now.
I haven't read your changes fully but inventory::form_from_map might be worth a look if you missed it unless that's how all_known_items() is built.

@mqrause
Copy link
Contributor

mqrause commented Dec 23, 2024

I expected the point was to extend the range to the same as e.g. crafting. I don't think inventory selector is the correct type of menu to show items from the entire reality bubble. It feels like V menu + appropriate item filter does most of the things you want there already.

@Brambor
Copy link
Contributor Author

Brambor commented Dec 23, 2024

I expected the point was to extend the range to the same as e.g. crafting.

  • I would love such a menu.
  • However, this PR is only packing what can fit into the existing disassembly menu. The disassembly menu can pack some more QoL into it.
  • Acquire graph #69831 should do a lot of calculations I want (items/time) and show sources for items (crafting, disassembly). But when I will do that I don't know.

It feels like V menu + appropriate item filter does most of the things you want there already.

  • I want to show the number of disassembled things at a glance. This is not shown in V menu.
  • But yes, these two menus are similar. Conceptually so many menus just show items in different ways.

I don't think inventory selector is the correct type of menu to show items from the entire reality bubble.

Maybe it's just me but I don't see why I'd ever want stuff out of reach showing up on the menu if it can't do anything with it (autopathing to it and then starting the activity). I appreciate you have that listed as a todo but unless you plan on doing it asap I think it'd be better if that part was disabled (or disabled by default and toggleable on) for now.

  • By showing items further than 1 tile I can plan the disassembly in the disassembly menu, where all the requirements, results, and time are shown at a glance. Decide what I want there, then (when I path to the item) act on it.
  • I would like to implement Travel to in this PR. It is an obvious next step.
  • I wanted to do a toggle to hide distant items regardless of the Travel to implementation, I didn't know how, I just realized there might be a way, just look at other inventory_selector menus lol.

I haven't read your changes fully but inventory::form_from_map might be worth a look if you missed it unless that's how all_known_items() is built.

I didn't look into it properly yet. Thanks for the tip!


The reason I published this PR is to get feedback as I was unhappy with my approach.

Ths is definitely not finished.

Thanks for the feedback!

@mqrause
Copy link
Contributor

mqrause commented Dec 23, 2024

I expected the point was to extend the range to the same as e.g. crafting.

* I would love such a menu.

* However, this PR is only packing what can fit into the existing disassembly menu. The disassembly menu can pack some more QoL into it.

I didn't mean a disassembly menu like the crafting menu, just the range of shown (and interactable) items. For crafting, you can use any components and tools in a 6 tile radius (PICKUP_RANGE is the constant I believe), and I'd absolutely understand using that for disassembly, too, without having to walk anywhere. I just don't think inventory selector is the correct type of menu for more than that.

@Brambor
Copy link
Contributor Author

Brambor commented Dec 23, 2024

I expected the point was to extend the range to the same as e.g. crafting.

* I would love such a menu.

* However, this PR is only packing what can fit into the existing disassembly menu. The disassembly menu can pack some more QoL into it.

I didn't mean a disassembly menu like the crafting menu, just the range of shown (and interactable) items. For crafting, you can use any components and tools in a 6 tile radius (PICKUP_RANGE is the constant I believe), and I'd absolutely understand using that for disassembly, too, without having to walk anywhere. I just don't think inventory selector is the correct type of menu for more than that.

Ah, I get you now. I thought about that, however, wouldn't it be weird if you could essentially destroy any disassemable item distant 5 tiles? If an enemy NPC drops a gun, you could just destroy it, right? Or is this not an issue?

Having that in would remove the need to display if anything is far, the need of travel to it etc. Which might be why you bring it up in this context :D

@IdleSol
Copy link
Contributor

IdleSol commented Dec 23, 2024

I didn't realize there was a search by component. I thought it was just by name.

My personal opinion. You have a menu that's too heavy. And with chunks of empty space.

Keep the old version. But add to it:

  • highlighting of components
  • the ability to select multiple items

Something like:
1

As a second step, and possibly a separate PR, you can increase the radius of view of items. To the level of the radius of finding components for crafting. With the same restrictions (if they are there).

In the third step, you can add a choice between disassembling immediately or moving the selected items to an area to disassemble items. (Provided that zone is nearby).

As for details:

  • There is no point in showing the distance to the item. You either can disassemble it or you can't.
  • Similarly with tools. If you can make it out we show the components. If you can't disassemble that we show the missing tools. But there's no point in showing the components if you lack the tools to take the item apart. (And you can always press E). (Like the screenshot with the electrohack)

@Brambor
Copy link
Contributor Author

Brambor commented Dec 23, 2024

I didn't realize there was a search by component. I thought it was just by name.

Searching needs better hints. I think nearly all menus support it. This would be a valid new issue I support.

Keep the old version. But add to it:

  • the ability to select multiple items

This might be nice. I will see if I fit it in after I finish my half-baked improvements.

As a second step, and possibly a separate PR, you can increase the radius of view of items. To the level of the radius of finding components for crafting. With the same restrictions (if they are there).

Yeah, I concluded so too. I should make a separate PR right now.

In the third step, you can add a choice between disassembling immediately or moving the selected items to an area to disassemble items. (Provided that zone is nearby).

I don't play with that. I would first have to see the value. I understand conceptually, I would need to want it though.

  • Similarly with tools. If you can make it out we show the components. If you can't disassemble that we show the missing tools. But there's no point in showing the components if you lack the tools to take the item apart. (And you can always press `E'). (Like the screenshot with the electrohack)

This is exactly the thing I don't agree with. To me, it is dumb that I cannot see at a glance what I can get from disassembly until I charge my tools or fetch them. It does make the menu wider though... Maybe a toggle between "slim"/"descriptive" would be the best option, if I am allowed to merge the "descriptive" option.

@IdleSol
Copy link
Contributor

IdleSol commented Dec 23, 2024

I don't play with that. I would first have to see the value. I understand conceptually, I would need to want it though.

Several items are disassembled one by one. If you interrupt the operation, then one item will be partially disassembled and the other items will not be marked in any way.

At the moment, if you take a few things apart. The first item is placed on the workbench or taken in hand. The other things remain under the character's feet (disassembly of things through the B menu).

If mass disassembly is available through the disassembly menu. The items will remain in their places within a radius of 6 or 8 squares. To continue the operation, you will again have to search for these items and select them for disassembly.

This is an unnecessary action that can lead to mistakes. (Disassemble more than planned).

The solution may be:

  1. All things in mass disassembly are tagged in some way. But this requires new mechanics. (not sure).
  2. All items get a disassembled status of 0%. This will give the ability to resume disassembling the item via the activation menu or workbench interaction. But they still remain in their places. And you'll have to look for them.
  3. Move the selected items to a new location. This location can be a tile marked as a disassembly area. The character can then start mass disassembly via B. Or have a follower do it.

Notice. If the disassembly menu will move things to a special zone. Then mass disassembly via the menu is not so necessary anymore.

This is exactly a thing I don't agree with. To me, it is dumb that I cannot see at a glance what I can get from disassembly until I charge my tools or fetch them. It does make the menu wider though... Maybe a toggle between "slim"/"descriptive" would be the best option, if I am allowed to merge the "descriptive" option.

With all tools being plugged in, you will most likely never get a dead tool situation. In general, it's more likely to be a situation where "you don't have a tool" than a situation where "you don't have enough charge".

@Brambor
Copy link
Contributor Author

Brambor commented Dec 23, 2024

@Brambor
Copy link
Contributor Author

Brambor commented Dec 23, 2024

If I do (or anybody does) the multi-select I want to see the list of components to be obtained from the disassembly, combined from all to-be-disassembled items, sorted.

Then it would solve the counting problem:

  1. You need 1348 copper wire and 57 electronic scrap.
  2. ( Open disassembly menu.
  3. Select things until you fulfil the copper wire requirement.
  4. Observe: It tells you how much electronic scrap you would get from that.
  5. Select things until the electronic scrap requirement is fulfilled too.
  6. Fiddle with the things if you want.

@Brambor Brambor changed the title Disassembly menu: highlight filtered components, show distant items, always show required components Disassembly menu: highlight filtered components, always show required & components Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants