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

Hammerhead Atmos and Hangar Rebuild #2387

Merged
merged 21 commits into from
Aug 11, 2023

Conversation

SerynEngi
Copy link
Contributor

@SerynEngi SerynEngi commented Apr 9, 2023

About The Pull Request

This PR applies a number of mapping fixes to the hammerhead. The only one pictured is the hangar, as atmos changes affect the whole ship.

Why It's Good For The Game

Fix man good. See above.

Testing Photographs and Procedure

HammerHangar Updated

Changelog

🆑
fix: Added Cargo Accept/Reject stamps, Xenobio Console, and tiny-fans to the HOS office doors on the Hammerhead
fix: Removed heavy firelocks from the Hammerhead's Hangar
fix: Placed supply piping properly on layer 2 and the scrubber network on layer 4 on the Hammerhead
fix: Minor atmos adjustments throughout the map on the Hammerhead
/:cl:

@SerynEngi SerynEngi requested a review from a team as a code owner April 9, 2023 03:30
@SerynEngi
Copy link
Contributor Author

FAQ
Mostly leaving it here so the PR itself is clean.

Q: Why didn't you just reopen the other PR? Or the one after that?
A: Scope change. I also fixed atmos in this one, though the hangar could arguably be considered an atmos issue, and Github jank hit the other one.

Q: Why haven't you added forward-firing primary weaponry?
A: Take it up with the core dev team. Get them to buy off on it and I'll do so.

Q: I noticed bug X, will you fix it?
A: If you comment it here, and its not a coder issue, probably.

Q: I dont like the way that X is designed, will you fix it?
A: Not in this PR. Outside of the hangar and atmos piping, I only fixed a couple wiring issues.

@Kmc2000
Copy link
Contributor

Kmc2000 commented Apr 12, 2023

Not sure what the current standard is for launch tubes, but any reason you've removed the atmos piping from them here?

@SerynEngi
Copy link
Contributor Author

Not sure what the current standard is for launch tubes, but any reason you've removed the atmos piping from them here?

The short version is lack of ATC players, and that launch tubes don't really function without ATC.

The longer version is that the original version utilized the ATC as the one who would control this, and independently controlled the blast doors so that the Pilots could not depressurize the hangar.

In order to make this functional in the same way without ATC, I would need to build it into multi-door airlocks, which I avoid on the principle of how much these break if we're doing anything particularly heavy (such as PVP or events).

On a related note, I would kill for code that activates blast-doors as the buttons do, only with airlocks, as I think it likely that it would break less often (being one action rather than a series of actions that have to run correctly in order).

@Bokkiewokkie
Copy link
Contributor

Not sure what the current standard is for launch tubes, but any reason you've removed the atmos piping from them here?

The short version is lack of ATC players, and that launch tubes don't really function without ATC.

The longer version is that the original version utilized the ATC as the one who would control this, and independently controlled the blast doors so that the Pilots could not depressurize the hangar.

In order to make this functional in the same way without ATC, I would need to build it into multi-door airlocks, which I avoid on the principle of how much these break if we're doing anything particularly heavy (such as PVP or events).

On a related note, I would kill for code that activates blast-doors as the buttons do, only with airlocks, as I think it likely that it would break less often (being one action rather than a series of actions that have to run correctly in order).

The code to operate airlocks remotely like blast doors does actually exist already, we could also make airlock controllers remotely accessible with a few lines of fighter code, like how the buttons work.

Besides that I still don't understand the complete removal of the cycling system, even if there is no ATC or AI to operate it actively, it can still be turned off by the pilots themselves if it's not used. (iirc the air alarms there are unlocked by default)
The system helps a lot with saving air and preventing damage from pressure changes when used properly, and losing it for a system which will always force there to be a massive pressure difference seems bad.

@SerynEngi
Copy link
Contributor Author

I will preface this with- I'm entirely fine with re-adding the air system, it wouldn't take me long and I'd even give it more air and pipe it so it actually works efficiently, if you still disagree with my reasoning.

Besides that I still don't understand the complete removal of the cycling system, even if there is no ATC or AI to operate it actively, it can still be turned off by the pilots themselves if it's not used.

I removed it because I didn't predict it actually being used. Pilots tend to disregard atmos differences unless it affects them in the airlock system, normally preferring a vacuum. I set the hangar Air Alarm to all access so that they could elect to turn off the hangar atmosphere for that reason.

The code to operate airlocks remotely like blast doors does actually exist already,

That'd be neat, how does one make that work on mapping? Legitimate question, as I don't know how. Also, if such exists, why do we not use it for all airlocks with multiple doors?

we could also make airlock controllers remotely accessible with a few lines of fighter code, like how the buttons work.

I am all for this, but I am not a coder and have absolutely no idea how I would make this work. I also know that the function to activate buttons from your fighter is currently bugged and does not work.

Bokkiewokkie
Bokkiewokkie previously approved these changes Jun 16, 2023
Copy link
Contributor

@Bokkiewokkie Bokkiewokkie left a comment

Choose a reason for hiding this comment

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

Mapping approved, if you're wondering why it's not testmerged that will be because pf conflicts with faxes

@github-actions
Copy link

github-actions bot commented Jul 7, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Bokkiewokkie Bokkiewokkie merged commit 5530fee into BeeStation:master Aug 11, 2023
9 checks passed
@SerynEngi SerynEngi deleted the Hammerderp branch July 21, 2024 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants