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.

Sign up for GitHub

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

Jump to bottom

Removed Interior Shuttle Walls #34261

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Removed Interior Shuttle Walls #34261

wants to merge 6 commits into from

Conversation

ps3moira
Copy link
Contributor

@ps3moira ps3moira commented Jan 7, 2025 •
edited
Loading

About the PR

Removed interior shuttle walls.

Why / Balance

They don't align with the regular shuttle walls sprite, or the diagonal or window sprites either. They also are significantly cheaper to produce and 4 steps less to deconstruct compared to regular shuttle walls with only a 200 hp difference. It's also cutting bloat.

Media

image

Changelog
🆑

  • remove: Removed interior shuttle walls

@github-actions github-actions bot added Changes: No C# Changes: Requires no C# knowledge to review or fix this item. Changes: Sprites Changes: Might require knowledge of spriting or visual design. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/M Denotes a PR that changes 100-999 lines. labels Jan 7, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 7, 2025 •
edited
Loading

RSI Diff Bot; head commit c84a7a5 merging into dc67a5a
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:

Resources/Textures/Structures/Walls/shuttleinterior.rsi

State Old New Status
full Removed
state0 Removed
state1 Removed
state2 Removed
state3 Removed
state4 Removed
state5 Removed
state6 Removed
state7 Removed

Edit: diff updated after c84a7a5

@K-Dynamic
Copy link
Contributor

Can you do migration.yml for shuttle walls

@IamVelcroboy
Copy link
Contributor

This shouldn't be a removal. If the issue is alignment it should be a sprite fix.

@ps3moira
Copy link
Contributor Author

ps3moira commented Jan 7, 2025 •
edited
Loading

This shouldn't be a removal. If the issue is alignment it should be a sprite fix.

That means the interior shuttle wall would have to be made to match the regular shuttle walls, which would be redundant. The alternative to that would be making diagonal and window interior walls which is bloaty for a wall that's not used often for shuttles or shuttle stations.

@UbaserB
Copy link
Member

UbaserB commented Jan 7, 2025

This shouldn't be a removal. If the issue is alignment it should be a sprite fix.

i don't really feel the need to have multiple versions anyway so removing it is fine

@IamVelcroboy
Copy link
Contributor

This shouldn't be a removal. If the issue is alignment it should be a sprite fix.

That means the interior shuttle wall would have to be made to match the regular shuttle walls, which would be redundant. The alternative to that would be making diagonal and window interior walls which feels bloaty.

Why is that bloat? If it's useful and fits the theme, it's... useful.

@IamVelcroboy
Copy link
Contributor

This shouldn't be a removal. If the issue is alignment it should be a sprite fix.

i don't really feel the need to have multiple versions anyway so removing it is fine

I mentioned in Discord but these are useful for shuttle stations in place of grey walls and for shuttle construction uses.

@ps3moira ps3moira marked this pull request as ready for review January 7, 2025 05:27
@NoElkaTheGod
Copy link
Contributor

I don't think they need to be removed, they're pretty useful for, you know, interior shuttle walls. Nerfing their HP would probably be better.

@K-Dynamic
Copy link
Contributor

K-Dynamic commented Jan 7, 2025 •
edited
Loading

I don't really feel either way but I think the screenshot is a bit unfair, especially if interior shuttle walls are used as pillars or interior rooms that don't connect to shuttle walls.

We could try making them not connect like how shuttle walls and shuttle windows don't.

@beck-thompson beck-thompson added P3: Standard Priority: Default priority for repository items. S: Needs Review Status: Requires additional reviews before being fully accepted DB: Beginner Friendly Difficulty: Great for beginners. Unambiguous in scope, and explains how to achieve the result. A: General Interactions Area: General in-game interactions that don't relate to another area. T: Content Removal Type: Removal of content or code that is not related to code clean-up. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Jan 7, 2025
@TiniestShark
Copy link
Contributor

I'd disagree that this is 'bloat' personally. At best their materials or HP should be reworked since they work perfectly well as a counterpart to regular/reinforced walls. They also have a much smoother design compared to the reinforced ones which gives it a nice aesthetic difference in comparison

kosticia
kosticia reviewed Jan 11, 2025
View reviewed changes
@slarticodefast slarticodefast added S: Awaiting Changes Status: Changes are required before another review can happen and removed S: Needs Review Status: Requires additional reviews before being fully accepted labels Jan 15, 2025
@github-actions github-actions bot added S: Needs Review Status: Requires additional reviews before being fully accepted and removed S: Awaiting Changes Status: Changes are required before another review can happen labels Jan 18, 2025
slarticodefast
slarticodefast approved these changes Jan 18, 2025
View reviewed changes
Copy link
Member

@slarticodefast slarticodefast left a comment

Choose a reason for hiding this comment

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

Code looks good. Whether or not to remove it I'll leave up to the art and map maintainers.

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Jan 31, 2025
@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Feb 11, 2025
@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Feb 20, 2025
@github-actions GitHub Actions
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kosticia kosticia kosticia left review comments

@Emisse Emisse Emisse approved these changes

@slarticodefast slarticodefast slarticodefast approved these changes

Assignees
No one assigned
Labels
A: General Interactions Area: General in-game interactions that don't relate to another area. Changes: No C# Changes: Requires no C# knowledge to review or fix this item. Changes: Sprites Changes: Might require knowledge of spriting or visual design. DB: Beginner Friendly Difficulty: Great for beginners. Unambiguous in scope, and explains how to achieve the result. P3: Standard Priority: Default priority for repository items. S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted S: Needs Review Status: Requires additional reviews before being fully accepted size/M Denotes a PR that changes 100-999 lines. T: Content Removal Type: Removal of content or code that is not related to code clean-up.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.