Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adds various ventcrawl QoL changes #14432
Adds various ventcrawl QoL changes #14432
Changes from 12 commits
54c6d1f
ee3792a
cc70211
24d3d30
0255b2a
a3f0b1a
b770285
12707f3
d4dc55a
5f7a725
f56168a
41fcf25
073e751
af08c70
678b62c
1e84fbf
8bbfc52
1eb613d
9b42afb
6ab13ed
a847bc4
7bf7db4
17d5f89
7b25c58
5d66d53
085095a
37b1af3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sources tell me
target=src
may be a problem and may not work well with people in ventsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Target is just for the location of the progress bar and is only visible to the user. It worked fine in testing. Is there any interaction you are concerned about?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure this is meant to be
||
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because vent crawl machinery is the list of vents, scrubbers, and passive vents. Check open on the other hand is a function that checks if the chosen atmospherics machinery has a potential connection that isn't currently filled, for example, a T junction with an open side.
TLDR: This OR lets you enter open pipes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can most likely be made typeless
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be removed since it's checked prior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesnt this make it so you cant climb into single device pipenets? If so you need to document this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went back and checked, you are still able to climb into single device pipe nets.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will make sure other logic won't break
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried this. It completely breaks ventcrawling, by making all nonpipes solid black sprites.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does
remove_ventcrawl
get called when the mob is destroyed? If not, there might be anull
value in the ventcrawlers list.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically yes. Update ventcrawl gets called every time a pipenet is updated, which will remove them. I added it to destroy to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this add duplicates to the
ventcrawler
global since this is also called when a pipe gets destroyed/created?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't as this check will do the following.
First, check if the mob currently has a pipe overlay, if they do. Delete it. Then if they are still currently in a pipe, regenerate it.
Then if they don't have an overlay, check if they are in a pipe, if they are, give them an overlay. Then just to be sure, if they are not in a pipe at this point, delete an overlay if they have one (This is just a sanity check to cover all four cases and probably unneeded)
The four cases are as follows for a nested if
They have an overlay and are in a pipe. Remove and readd.
They have an overlay and are not in a pipe. Just remove.
They don't have an overlay and are in a pipe. Give them an overlay.
They don't have an overlay and are not in a pipe. Try to remove it just to be sure.
Though, I could force a remove before the add in the case of no overlay and in a pipe, if you want to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be slightly more expensive, but would always work. Thoughts on this as a replacement?
/mob/living/update_pipe_vision()
remove_ventcrawl()
if(is_ventcrawling(src))
add_ventcrawl(loc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All gut here. Overlooked some code it seems back when I commented on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here