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

Requisitions elevator lighting fix #5204

Merged
merged 2 commits into from Dec 18, 2023
Merged

Requisitions elevator lighting fix #5204

merged 2 commits into from Dec 18, 2023

Conversation

ghost
Copy link

@ghost ghost commented Dec 12, 2023

About the pull request

This PR fixes the elevator lighting by changing the way the turf cloning logic works to use the pre-existing ChangeTurf function, which will result in the static lighting properties being copied properly. By doing this change, we can disable the base lighting properties from the shuttle area and have the static lighting system apply to the elevator turf in requisition which is consistent with how the room lighting already works.

Explain why it's good for the game

The existing elevator has many visual effect issues, requiring other forms of lighting to compensate for the problem, such as placing movable lights (flashlight), or other non-static lighting sources. Without these work arounds the elevator will be completely black after the first time the space turfs are cloned.

Testing Photographs and Procedure

Screenshots & Videos

Before

before.mp4

After

after.mp4

Changelog

🆑
fix: Requisitions elevator lighting fix
/:cl:

@github-actions github-actions bot added the Fix Fix one bug, make ten more label Dec 12, 2023
Copy link
Member

@fira fira left a comment

Choose a reason for hiding this comment

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

I've been looking for it for months and i never realized there were turfs instanciations here!!!!! I feel so stupid!!!! AGHHH thanks so much

@fira fira added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Dec 12, 2023
Copy link
Contributor

@Birdtalon Birdtalon left a comment

Choose a reason for hiding this comment

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

No issues noted from testmerges.

@Birdtalon Birdtalon added this pull request to the merge queue Dec 18, 2023
Merged via the queue into cmss13-devs:master with commit 59381f0 Dec 18, 2023
28 checks passed
cm13-github added a commit that referenced this pull request Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix Fix one bug, make ten more Testmerge Candidate we'll test this while you're asleep and the server has 10 players
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants