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

feat: Remove z-level restriction for zone activities. #5234

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vitalyo7
Copy link
Contributor

Checklist

Required

Purpose of change

Note: : had to resubmit PR due to issues with my fork, should be good now.
Update the code to ignore z-level differences when working with zones. This should make it easier to setup loot zones across the z-levels for easier loot managements, i.e. basements, roofs, apartments and LMOE. It was a bit annoying to carry the loot up/down stairs.

Describe the solution

Solution is to remove all checks for z-level, both in the GUI and code that interacts with zones.

Describe alternatives you've considered

None. I think zones are currently the best way to manage loot, and just need to tweak them to work properly.

Testing

Additional context

This is half-tested. I compiled the code, did some zone setup in my LMOE bunker and asked my main char to sort out the loot. Seem(s) like it worked. However more testing will likely be required. Volunteers appreciated.

@github-actions github-actions bot added the src changes related to source code. label Aug 22, 2024
Copy link
Contributor

autofix-ci bot commented Aug 22, 2024

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

@scarf005 scarf005 changed the title feat: Removing z-level restriction for zone activities. feat: Remove z-level restriction for zone activities. Aug 23, 2024
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we remove commented out code?

@vitalyo7
Copy link
Contributor Author

will do

@RoyalFox2140
Copy link
Collaborator

I really want to see this PR happen, when it needs testing I might be able to open it up (depending on when im upgrading to linux which is very very soon.)

@scarf005 scarf005 self-requested a review August 23, 2024 01:04
@scarf005
Copy link
Member

the changes look nice, will review after i get home

@RoyalFox2140
Copy link
Collaborator

image
I don't know if this works well enough. I keep getting my sorting cancelled. I can't automatically drop protein rations in the basement but it did correctly move first aid kits up. I thought that it was because I couldn't actually see them due to being in lockers but look at this? There's a protein ration in the basement and I can't actually sort because I can't go up the stairs automatically which also means I can't sort on my own Z level. I really can't make a decision on if this is fine because it works some of the time or if this is a blocker for the PR.

image
image

@RoyalFox2140
Copy link
Collaborator

After testing it a fair bit more the pathfinding is extremely random on where and why it gets stuck but it's always because it can't navigate stairs. It just sometimes does and sometimes doesn't.

Maybe @KheirFerrum knows why the pathfinding breaks.

@KheirFerrum
Copy link
Collaborator

No bloody clue. I would also remind the OP that z-level processing is optional, so make sure your checks reflect that. If z-levels are turned off it likely won't matter what you try to feed the function, it will simply assume the current z-level.

@RoyalFox2140
Copy link
Collaborator

No bloody clue. I would also remind the OP that z-level processing is optional, so make sure your checks reflect that. If z-levels are turned off it likely won't matter what you try to feed the function, it will simply assume the current z-level.

Z-Levels are mandatory behavior now after one of Zlor's PR's. 3D vision however is optional I'm not sure how this affects the PR but I had 3D vision on when I was failing to path find.

@vitalyo7
Copy link
Contributor Author

I have a feeling path finding has some max route length that maybe causing the cancellation. Will see if I can make more changes.
Thanks for helping find the edge cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants