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

Bulk loading items saves time cost for generic "pick up(g)" process #69244

Merged
merged 5 commits into from
Nov 12, 2023

Conversation

lispcoc
Copy link
Contributor

@lispcoc lispcoc commented Nov 10, 2023

Summary

Balance "Bulk loading items saves time cost for generic pick up(g) process"

Purpose of change

Follow up #68214

Describe the solution

Apply same solution as #68214 to "pick up" action
Additionally, in #68214 there was an issue where the cost of moving large items was underestimated.
This time I will address this issue to some extent by placing a limit on the capacity that can be loaded in bulk. Although not implemented in this PR, we plan to backport it later to the Insert action as well.

Describe alternatives you've considered

I think it would be better to review the conventional "pick up" processing and redirect it to "insert" processing, but I gave up because the "pick up" processing is very complicated.
Please let me know if there is a better way

Testing

Pick up items from adjacent tiles

1000 salts (5 L)
Before: 30:00
After: 00:54

100 apples (23 L)
Before: 3:11
After: 3:11

Additional context

@github-actions github-actions bot added Game: Balance Balancing of (existing) in-game features. [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Nov 10, 2023
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Nov 10, 2023
@GuardianDll
Copy link
Member

i have a huge doubts one can pick up a hundreds apples in 13 seconds

@lispcoc
Copy link
Contributor Author

lispcoc commented Nov 11, 2023

i have a huge doubts one can pick up a hundreds apples in 13 seconds

I also had my doubts about the time taken to pick up larger items.
This comes from the following cost formula defined in item_handling_cost() in character.cpp

cost = min( 200, item_volume / 20_ml )

Changing the 20_ml coefficient can solve the problem that the effect of volume on time is underestimated, but item_handling_cost() is also used for things such as the cost of taking out weapons, so this is not recommended.

My interpretation of this PR is that time can be saved by ignoring the overhead of moving to an adjacent tile or opening a container for subsequent items.
As an alternative approach, I think that the underestimation of bulky items could be overcome to some extent by making the net bonus of ignoring this overhead stop working each time a certain volume is exceeded.

Below is an example

Limit volume 1000 ml 500 ml 250 ml 200 ml
1000 salts (5 L) 0:20 0:29 0:47 0:54
100 apples (23 L) 0:50 1:15 1:43 3:11

@lispcoc lispcoc marked this pull request as ready for review November 12, 2023 04:34
@Maleclypse Maleclypse merged commit 52b0144 into CleverRaven:master Nov 12, 2023
@lispcoc lispcoc deleted the bulk_pickup branch November 12, 2023 15:38
Maleclypse pushed a commit to Maleclypse/Cataclysm-DDA that referenced this pull request Nov 16, 2023
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 BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Game: Balance Balancing of (existing) in-game features. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants