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

dedicated unload loot button #59596

Merged
merged 6 commits into from
Aug 1, 2022

Conversation

bombasticSlacks
Copy link
Contributor

@bombasticSlacks bombasticSlacks commented Jul 25, 2022

Summary

None

Purpose of change

Should be a way to strip corpses and unload stuff without otherwise sorting

Describe the solution

Added a new method similar to sort but with the actual moving removed. just tries to unload nearby unload zones.

Moved over to the activity actor system, made some functions that were just used in the old activities available in a namspace for use across the systems.

Describe alternatives you've considered

Testing

Load up game, make an unload zone, kill some zeds, try unloading

Additional context

image

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jul 25, 2022
@mqrause
Copy link
Contributor

mqrause commented Jul 25, 2022

You should implement the activity with the activity actor system. See #40013.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 25, 2022
@bombasticSlacks
Copy link
Contributor Author

You should implement the activity with the activity actor system. See #40013.

in the future I would like to generalize this and further similar activities to that system and split them into chunks (so code isn't replicated), however for now it seems like a lot of work and risks regression when we should be in freeze. This is just a quick assist for the new nested zombie items making the workflow of unloading a bit snappier potentially.

@snipercup
Copy link
Contributor

Why go trough zones for this? Shouldn't this be just a player action? Do you want NPC's to use this function as well and that's why you're using zones? I get there's a player action for unloading a container, but that puts it into your inventory as opposed to your solution where it is dropped onto the ground. I feel like it should be more similar to unload container as a player action.

@mqrause
Copy link
Contributor

mqrause commented Jul 25, 2022

in the future I would like to generalize this and further similar activities to that system and split them into chunks (so code isn't replicated), however for now it seems like a lot of work and risks regression when we should be in freeze. This is just a quick assist for the new nested zombie items making the workflow of unloading a bit snappier potentially.

You can make it a quick assist with an activity actor and save the refactoring for later. There's no real need to add to legacy code here.

@bombasticSlacks
Copy link
Contributor Author

bombasticSlacks commented Jul 25, 2022

Why go trough zones for this

So you can do it en-masse. If you have a huge unload zone you can unload everything in there. It also gets nested containers.

but that puts it into your inventory as opposed to your solution where it is dropped onto the ground

Onto the ground exists as well but is unbound by default "Unload container" in the bindings menu. It's very useful.

@bombasticSlacks
Copy link
Contributor Author

You can make it a quick assist with an activity actor and save the refactoring for later. There's no real need to add to legacy code here.

I apologize but I don't understand what this means.

@mqrause
Copy link
Contributor

mqrause commented Jul 25, 2022

I mean you can make a simple activity actor and pretty much just copy over your do_turn (with some small adjustments probably). It's not any more complicated but uses the modern activity system instead of the legacy one.

@bombasticSlacks bombasticSlacks marked this pull request as draft July 25, 2022 18:59
@bombasticSlacks bombasticSlacks marked this pull request as ready for review July 28, 2022 13:27
@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 28, 2022
@bombasticSlacks bombasticSlacks marked this pull request as draft July 29, 2022 02:34
@bombasticSlacks bombasticSlacks marked this pull request as ready for review July 29, 2022 02:51
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jul 29, 2022
@dseguin dseguin merged commit 99705f8 into CleverRaven:master Aug 1, 2022
@molkero
Copy link
Contributor

molkero commented Aug 1, 2022

src/activity_actor.cpp:6266:14: error: variable 'unload_always' set but not used [-Werror,-Wunused-but-set-variable] bool unload_always = false;

alef pushed a commit to alef/Cataclysm-DDA that referenced this pull request Aug 6, 2022
* Unload loot working

* kind of working

* new mode with interrupts working correctly

* feature parity test

* Clangin'

* clang again
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` [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants