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

Introduce extension of u_has_item that allows items in the vicinity #74451

Closed
PatrikLundell opened this issue Jun 11, 2024 · 13 comments · Fixed by #77791
Closed

Introduce extension of u_has_item that allows items in the vicinity #74451

PatrikLundell opened this issue Jun 11, 2024 · 13 comments · Fixed by #77791
Assignees
Labels
stale Closed for lack of activity, but still valid. <Suggestion / Discussion> Talk it out before implementing

Comments

@PatrikLundell
Copy link
Contributor

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

While doing testing I encountered the occupied lumbermill and its missions. The manager asked for drive belts, and I debug spawned them, but they weren't recognized as present until I cleared out the inventory and picked them up.
This got bad when the mission was for blankets, as I can't pick up that many blankets (but quilts worked, as they're not monstrously large).

Solution you would like.

The introduction of a new extended directive that looks for items in the vicinity not owned by others, so the items can be e.g. in a vehicle trunk, or hauled. Obviously, missions ought to be updated to use the new directive when reasonable, especially when the bulk of items requested is large.

Describe alternatives you have considered.

A work around would be to use the structure used by the Tacoma farm. The disadvantage with that is that it doesn't seem to support alternative sets of items (blankets, quilts, fur blankets,... for the lumbermill task).

Another alternative would be to extend the syntax used by Tacoma to support alternative sets of items and shift missions to use that syntax.

Additional context

No response

@PatrikLundell PatrikLundell added the <Suggestion / Discussion> Talk it out before implementing label Jun 11, 2024
@GuardianDll
Copy link
Member

missions ought to be updated to use the new directive
It would also require to make an effect that would be able to remove said items from the player or the ground, or transfer their ownership at least

@zachary-kaelan
Copy link
Contributor

Isn't this what u_map_run_item_eocs does? Particularly with manual or manual_multi.

@GuardianDll
Copy link
Member

this one allow to search items on the ground, but it doesn't allow to specify how much of said items is needed to be, doesn't allow to deliver items to NPC (because when this effect is run, we swap our beta talker from NPC we talk to items we search), and it doesn't allow to search items both in character inventory and on the ground

In theory it can be improved to the level where it could solve this, i honestly think only issue 3 might need to be solved, other stuff might be resolved from the json side (like we can just delete said items instead of giving them to NPC?)

Oh, and also MGOAL_CONDITION is a condition, when u_map_run_item_eocs is effect, also need to think the ways to connect two, but at that point one could just move the entire winning condition to EoC and just use finish_mission

Copy link
Contributor

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.

@github-actions github-actions bot added the stale Closed for lack of activity, but still valid. label Jul 20, 2024
@GuardianDll
Copy link
Member

We got u_has_items_sum and u_consume_item_sum since this issue was made, and i even have a code that improves it to allow to use ranges, in master...GuardianDll:Cataclysm-DDA:better_f_consume_item
However, while consuming charges around the character is just a matter of using use_charges() functions, items do not really have such ability, and i am not aware of the way how to make it - my attempt in aforementioned branch failed completely

@PatrikLundell
Copy link
Contributor Author

I would guess the logic behind the operations to find and consume items for crafting ought to be usable, either through the operations themselves or through reuse of their logic.
For crafting it is a two step process: first items are tallied within the reachable range, then a selection of components takes place (via the UI or automatically via logic, in particular when there is only a single possible match), then there is a second pass (after the go ahead has been given to actually perform the task), during which essentially the same logic is applied in a consumption phase where items are removed as they're encountered until the selected count has been removed. Actually this part is a two step process for some categories of items, as desirable targets are removed first, and undesirable ones only in a further pass making up for the lacking number (undesirable items are currently essentially containers with liquids and poisonous food).

@GuardianDll
Copy link
Member

That what Snup (aka mqrause on github) told me and even showed me the code
But as you can see in my branch, it didn't work

@PatrikLundell
Copy link
Contributor Author

I'm trying to look at the code, but I'd like to know where it fails, and, ideally, if you have a suitable save available.

The "where does it fail" question is basically if everything fails, or if it's only the item removal (which it sounds like from your description). If it's the latter case, I would assume the condition.cpp part works.

@GuardianDll
Copy link
Member

GuardianDll commented Oct 13, 2024

You are right, when i say "i failed" i mean "specifically part of code that supposed to remove item from the map doesn't work"

I don't have the save file, but you can test it by running next eoc (that i have in this branch because i used it for tests):

  {
    "type": "effect_on_condition",
    "id": "AAAAAAAAAAAAAAAAAAAAAAAAAAA",
    "effect": [
      { "u_consume_item_sum": [ { "item": "blanket", "amount": 2 }, { "item": "fur_blanket", "amount": 2 } ] },
      { "u_message": "DONE" }
    ]
  },

then spawning blankets around, and activating said eoc via debug - game - activate eoc
You will see that the game would not remove any blankets around you, despite code being compilable

@PatrikLundell
Copy link
Contributor Author

PatrikLundell commented Oct 13, 2024

I've gotten a bit, but not sure if it's all the way.

  • added add_msg in parallel with add_msg_debug to get the output as normal game feedback, for easier examination.
  • assigned the result from the consume_items calls to lists, and then printed the size of the list in the add_msg output.
  • You seem to be very close. Changing "1" to "count_desired" in the calls to consume_items seems to do the trick.

I haven't tested extensively, and you'd probably need some test case with a larger number of items. Also, I've only tested with a single pile.

With this I'm handing it back to you, unless you want me to do more.

@GuardianDll
Copy link
Member

GuardianDll commented Oct 13, 2024

hmm? for master...GuardianDll:Cataclysm-DDA:better_f_consume_item#diff-dbaa22c0fa33ea5e52a18f1ce46db9dcd13311330a9e5130fff6bdf18d67629bR3520 it was intended to use count_desired, i omitted it for test purposes, but for master...GuardianDll:Cataclysm-DDA:better_f_consume_item#diff-dbaa22c0fa33ea5e52a18f1ce46db9dcd13311330a9e5130fff6bdf18d67629bR3544 it is absolutely necessary to remove items one by one, since user has more of this item than quest require, and we need to not overshoot amount to subtract
tho there is surely a more elegant way to do it than remove items one by one

Do you actually mean my code works? i need to examine it once more

@PatrikLundell
Copy link
Contributor Author

I don't really see why you'd want to remove items one by one. It should be sufficient to reduce the desired number with what you just managed to remove.
If, for some reason, you actually need to remove items one by one, you'd need to add a loop to do it.

Anyway, yes, it does indeed seem like you were a hair's width from getting there (I may have missed something, of course).

@GuardianDll GuardianDll self-assigned this Oct 13, 2024
@Procyonae
Copy link
Contributor

I'm tired and only skim read this but MGOAL_FIND_ITEM already does this so it's C++ logic is probably useful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Closed for lack of activity, but still valid. <Suggestion / Discussion> Talk it out before implementing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants