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

u_bulk_trade_accept doesn't respect item containers. #68456

Closed
MNG-cataclysm opened this issue Oct 2, 2023 · 5 comments · Fixed by #72759 or #73703
Closed

u_bulk_trade_accept doesn't respect item containers. #68456

MNG-cataclysm opened this issue Oct 2, 2023 · 5 comments · Fixed by #72759 or #73703
Labels
(S2 - Confirmed) Bug that's been confirmed to exist

Comments

@MNG-cataclysm
Copy link
Contributor

MNG-cataclysm commented Oct 2, 2023

Describe the bug

When using u_bulk_trade_accept to sell items, specifically canned ones, the containers that they are not respected by the trade option and will leave the player with a bunch of empty containers. If done with sealed canned goods, you will be left with a bunch of sealed, empty cans.

Attach save file

N/A

Steps to reproduce

  1. Use any u_bulk_trade_accept code that takes canned goods in a repeat response, I used Eddie's trade interaction.
  2. Spawn in contained canned goods, I used corn.
  3. Trade them all and be left with a bunch of empty cans.

Expected behavior

I expect that if I sell canned goods or sealed things like that, that I will also sell the can instead of magically removing the food and leaving me with empty, sealed cans.

Screenshots

Before selling my corn.
inventory
After selling my corn.
buggy inventory

Versions and configuration

-OS: Windows 11
-Game Version: cdda-windows-tiles-sounds-x64-2023-09-26-1218
-Graphics Version: Tiles
-Ingame language: English
-Mods loaded: [ dda, no_npc_food, personal_portal_storms, no_fungal_growth ]

Additional context

None.

@MNG-cataclysm MNG-cataclysm added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label Oct 2, 2023
@MNG-cataclysm MNG-cataclysm changed the title 'u_bulk_trade_accept' doesn't respect item containers. u_bulk_trade_accept doesn't respect item containers. Oct 2, 2023
@harakka
Copy link
Member

harakka commented Oct 8, 2023

/confirm
For sealed goods and liquids it's clear enough that the container should go with the goods, but is there some more universal criteria that can be used?

@github-actions github-actions bot added (S2 - Confirmed) Bug that's been confirmed to exist and removed (S1 - Need confirmation) Report waiting on confirmation of reproducibility labels Oct 8, 2023
@github-project-automation github-project-automation bot moved this to Proposed Release Blockers in 0.H Release Tracker Nov 13, 2023
@RenechCDDA
Copy link
Member

RenechCDDA commented Nov 15, 2023

For sealed goods and liquids it's clear enough that the container should go with the goods, but is there some more universal criteria that can be used?

Anything with an automatic whitelist containing that item. So if you bring in some boxes of sugar that you just looted, turning in the sugar turns in the boxes as well. But if (for some bizarre reason) you whitelisted sugar in your hiking backpack and had it in your hiking backpack, your backpack is not forfeit.

edit: OK it seems that automatic and manual whitelists are not differentiated (fair enough). Still, one-pocket item containing one whitelisted item should be pretty good. If we want to be really sure, you can throw in a yes/no query which says "You will also trade over the following containers: [blah]".

@katemonster33
Copy link
Contributor

Came up with #72759 to work on this but could use some feedback. Right now it just looks for sealed cans containing the item but I see we've had the discussion on whitelisted pockets, maybe @RenechCDDA has an opinion. My worry is that the user might unintentionally give up a vessel they didn't intend to if we open up the logic too much. Like if they've setup their own whitelist in like a cup, they might not have intended to give up the cup.

I think limiting the logic to sealed containers makes the most sense, plus, in my opinion it follows that an organization wouldn't care much about taking food donations in unsealed containers. Like, if Eddie is asking for canned corn, why would he accept corn in an old sock from some rando?

@MNG-cataclysm
Copy link
Contributor Author

Sweet, thank you for doing all this! @katemonster33

@I-am-Erk I-am-Erk moved this from Proposed Release Blockers to Accepted Release Blockers in 0.H Release Tracker Apr 2, 2024
@github-project-automation github-project-automation bot moved this from Accepted Release Blockers to Completed in 0.H Release Tracker Apr 4, 2024
@RenechCDDA
Copy link
Member

#72759 doesn't appear to have fully resolved this. Larger tin cans will be opened and emptied, while small ones are correctly removed. Liquids are also taken right out of their (unsealed) containers and in some cases wielded by the receiving party. Obviously, you should not be able to wield cooking oil (16).

To help with making sure we clean up all the instances, I've prepared some saves for testing. In both cases they are right next to the person you need to talk to.

BULK_TRADE-trimmed.tar.gz
Has a small tin can with turn-in item, a large tin can with turn-in item, and gallon jug with liquid turn-in item

BULK_DONATE-trimmed.tar.gz
Has 50 smoked meat, should only lose 1 when donating

@RenechCDDA RenechCDDA reopened this Apr 30, 2024
@github-project-automation github-project-automation bot moved this from Completed to Accepted Release Blockers in 0.H Release Tracker Apr 30, 2024
@github-project-automation github-project-automation bot moved this from Accepted Release Blockers to Completed in 0.H Release Tracker May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(S2 - Confirmed) Bug that's been confirmed to exist
Projects
Status: Completed
4 participants