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

Half Revert #31978 #33160

Merged
merged 5 commits into from
Nov 14, 2024
Merged

Half Revert #31978 #33160

merged 5 commits into from
Nov 14, 2024

Conversation

cohanna
Copy link
Contributor

@cohanna cohanna commented Nov 4, 2024

About the PR

The ability to upgrade reinforced windows to shuttle windows has been removed.

You can still construct shuttle windows from scratch
Partially reverts #31978

Why / Balance

This specific upgrade is largely used to upgrade the windows to brig and command offices which is very powergamey and
not very cool. Itd be one thing if these were meant to be destroyed but its 500 hp is comparable to a reinforced wall which makes it unreasonable for a 2 plasteel upgrade with no tools required.

Revert was requested by @keronshb in comments below

Technical details

removed the upgrade edge on the reinforced window graph
Partially reverts #31978

Media

Requirements

Breaking changes

Changelog

🆑 clinux

  • remove: Normal and reinforced windows can no longer be directly upgraded using rods, plasma, uranium, or plasteel.

@github-actions github-actions bot added the Changes: No C# Changes: Requires no C# knowledge to review or fix this item. label Nov 4, 2024
@thebadman4662
Copy link

Would it not be better to change cost instead rather than remove the feature?

@cohanna
Copy link
Contributor Author

cohanna commented Nov 4, 2024

Would it not be better to change cost instead rather than remove the feature?

I was leaning towards adjusting their health as an alternate over the cost (b/c frankly them having anything over 100hp is silly). I'm more than willing to massively nerf the shuttle walls and windows (as well as increase the cost for construction & upgrading) if that's what the general consensus yields. But since these structural parts are really only used for evac and from what I remember the main reason they have such large heath is to prevent EORG/shuttlebombing (not that it really helps, evac gets spaced and has holes blown in it consistently)
it both doesn't make sense for them to be so easily utilized and since nerfs would get more maintainer pushback than removal, I opted to remove the upgrade path instead.

@cohanna
Copy link
Contributor Author

cohanna commented Nov 4, 2024

In terms of nerfs to the shuttle windows and walls I was thinking this:
Windows-
Hp decreased from 500->100 (this would make reinforced plasma and uranium windows have 50 more hp than a shuttle window while still being non deconstructable via rcd)
Construction cost increased from 2 plasteel -> 4

Walls-
Construction cost increased from 4 total plasteel ->6
(Keep in mind these walls have total reflect but the same hp as reinforced)

What do you guys think?
Upgrades would use the new recipe cost too (so 4 plasteel to upgrade a window)

@IamVelcroboy
Copy link
Contributor

Shuttle structures are intentionally robust. I wouldn't make them weaker. Increase the cost and add to the complexity(extra steps) if you want to decrease the scenarios you mention.

@cohanna
Copy link
Contributor Author

cohanna commented Nov 4, 2024

Shuttle structures are intentionally robust. I wouldn't make them weaker. Increase the cost and add to the complexity(extra steps) if you want to decrease the scenarios you mention.

I fully get why they are so beefy, but an upgrade to a 500hp window with anything that can be ordered from cargo for a reasonable price & quantity is insane at best. You should not be able to upgrade the brig windows to be almost the same hp as a reinforced wall with roundstart materials and no tools.

What kind of steps do you even have in mind? The entire point of the window upgrade system is so that you can quickly upgrade windows. If you make the upgrade process complex then what's the difference between it and just deconstructing the existing window and constructing a shuttle one? The "extra steps" would be to deconstruct the existing window in this case, aka removing the upgrade path (which this PR does).
This PR does not remove the ability to construct shuttle windows, only the ability to upgrade existing windows into shuttle windows. I think the time taken to deconstruct an existing window is costly enough to warrant the shuttle windows high hp.

@IamVelcroboy
Copy link
Contributor

I would say it's fairly reasonable to remove from the upgrade graph as long as they can still be built from scratch. But if it's preferable to leave it in, then look at the time it takes to build from scratch and add a welding step to make up the difference

@cohanna
Copy link
Contributor Author

cohanna commented Nov 4, 2024

But if it's preferable to leave it in, then look at the time it takes to build from scratch and add a welding step to make up the difference

I can agree to that, up to the maintainers to decide between the two though.

@Blackern5000
Copy link
Contributor

Okay, I'm the one who originally added window upgrading, and...

In terms of nerfs to the shuttle windows I was thinking this: Windows- Hp decreased from 500->100 (this would make reinforced plasma and uranium windows have 50 more hp than a shuttle window while still being non deconstructable via rcd)

Plasma/uranium windows are literally cheaper. 2 plasma to build them vs 2 plasteel? And you want the plasma to be better? Huh? You need 1 plasma to make 1 plasteel!
If you want to nerf their HP (which I would not be against), make them no less than 200 HP. Plasteel is NOT cheap.
To go into detail about plasteel costs...

  • Solid plasma costs 2k per 90 pieces, that's 45 full window upgrades, at ~44.4 spesos each.
    Windows upgraded with pure plasma will now have double HP (+75), for ~1.7 HP per speso.
  • Plasteel cost 4.8k per 90 pieces, that's again, 45 window upgrades, but this time at ~106.6 spesos each, providing...
    • A 6.66x upgrade to 500 HP (+425), ~4 HP per speso.
    • A 1.33x upgrade to 100 HP (+25), ~0.23 HP per speso.
    • A 2.66x upgrade to 200 HP (+125), ~1.17 HP per speso.
    • A 3.33x upgrade to 250 HP (+175), ~2.34 HP per speso. Personally, I suggest either this ratio, or the one at the bottom.

Uranium windows have the same HP as plasma windows because they exist to block radiation, they aren't meant to be ultra durable, that's what plasteel is for.
Somewhat off-topic, but... You can't buy uranium, however the material sells for 1.7x more than plasteel (50 per vs 85 per), and assuming the same markup if cargo is buying, you'd be buying uranium windows at 181.2 spesos per upgrade, or a huge

Construction cost increased from 2 plasteel -> 4

There's an already existing construction recipe for shuttle windows that costs 2 plasteel and 2 reinforced glass, the upgrading is just that but you can do it to reinforced windows that are present already. It's the exact same cost.
You COULD increase the cost if you kept the current HP, and it would give you the following.
22.5 upgrades per plasteel crate, ~213.3 spesos per upgrade.

  • A 6.66x upgrade to 500 HP (+425), ~2 HP per speso.

This looks good to me, but you would have to modify the already existing construction to use more plasteel as well. It'd also be mutually exclusive with nerfing the HP, don't change both the cost and the strength.

In all honesty, shuttle windows/walls should use titanium, but we don't have titanium in 14.

@Blackern5000
Copy link
Contributor

But if it's preferable to leave it in, then look at the time it takes to build from scratch and add a welding step to make up the difference

I think a welding step would be a great addition, it would mostly (I know welding masks exist) restrict this upgrade to engineers, who should realistically be the only ones doing this kind of thing.

@cohanna
Copy link
Contributor Author

cohanna commented Nov 4, 2024

Plasma/uranium windows are literally cheaper. 2 plasma to build them vs 2 plasteel? And you want the plasma to be better? Huh? You need 1 plasma to make 1 plasteel!
If you want to nerf their HP (which I would not be against), make them no less than 200 HP. Plasteel is NOT cheap.
Plasma/uranium windows are literally cheaper. 2 plasma to build them vs 2 plasteel? And you want the plasma to be better? Huh? You need 1 plasma to make 1 plasteel!

I can agree to a 200-250hp nerf with a welding step added as well, the original reason I chose 100hp in the first place is that plasteel is often mapped in relative abundance round start (with some maps like cog having like 6 stacks mapped) so while yes, cargo wise plasma and especially uranium is more expensive, plasteel is significantly more accessible.

Uranium windows have the same HP as plasma windows because they exist to block radiation, they aren't meant to be ultra durable, that's what plasteel is for.

I disagree that they aren't meant to be the durable window option. Both plasma and uranium windows block radiation really well and honestly a buff to uranium windows can be argued pretty easily because uranium can't be bought. Plasma windows in particular are often mapped as the "extra durable" window type like its usage for core's ame and burn chambers.

In all honesty, shuttle windows/walls should use titanium, but we don't have titanium in 14.

1000% agree with you there, I was thinking exchanging out the plasteel in the recipe for bananium to make it impossible to construct round start and to also give a use for the material (one day it will be slippery, one day...) but decided that would take much more arguing to get through lol

@keronshb
Copy link
Contributor

keronshb commented Nov 4, 2024

Regarding window upgrades -
I'm in agreement that it should actually be really difficult for command/sec to upgrade windows. This is the 1# reason they weren't included in the RCD and that bypasses that step explicitly. Otherwise you do get command/sec upgrading windows with near indestructible windows every round and that's something we do not want to have gameplay wise.

EDIT:

#31978 was merged without typical maintainer review.
It needs a full revert pending an actual review.

@cohanna
Copy link
Contributor Author

cohanna commented Nov 4, 2024

Otherwise you do get command/sec upgrading windows with near indestructible windows every round and that's something we do not want to have gameplay wise.

This is what prompted the PR lol
Over the past month on lizard windows have increasingly been upgraded to shuttle windows in the bridge/brig/hop office and I had like 4 rounds in a row where the brig was reinforced with this. I'm sure this isn't as much as an issue on mrp servers but it's gotten very annoying on lrp.

@keronshb
Copy link
Contributor

keronshb commented Nov 4, 2024

Otherwise you do get command/sec upgrading windows with near indestructible windows every round and that's something we do not want to have gameplay wise.

This is what prompted the PR lol Over the past month on lizard windows have increasingly been upgraded to shuttle windows in the bridge/brig/hop office and I had like 4 rounds in a row where the brig was reinforced with this. I'm sure this isn't as much as an issue on mrp servers but it's gotten very annoying on lrp.

If you swap this PR to a full revert it'd get my approval. This definitely needed a maintainer review before it was merged.

@cohanna
Copy link
Contributor Author

cohanna commented Nov 4, 2024

If you swap this PR to a full revert it'd get my approval. This definitely needed a maintainer review before it was merged.

Ehhh... Im rather fond of the other window upgrades and really only think the issue lies with the shuttle windows.

@keronshb
Copy link
Contributor

keronshb commented Nov 4, 2024

If you swap this PR to a full revert it'd get my approval. This definitely needed a maintainer review before it was merged.

Ehhh... Im rather fond of the other window upgrades and really only think the issue lies with the shuttle windows.

The main issue is that the original PR accidentally bypassed any official maintainer discussion.
As it is right now, it's also skipping the fact that we excluded this from RCDs for a reason - due to it being really powerful. Which is why we required the actual construction steps for these kind of windows. It's also why things like R-Walls and other specialty type walls aren't in the RCD/upgradable.

@HelpTheSteak
Copy link

I just think they look ugly when next to normal or reinforced walls so I'm on board :godo:

@cohanna
Copy link
Contributor Author

cohanna commented Nov 5, 2024

The main issue is that the original PR accidentally bypassed any official maintainer discussion.

As it is right now, it's also skipping the fact that we excluded this from RCDs for a reason - due to it being really powerful. Which is why we required the actual construction steps for these kind of windows. It's also why things like R-Walls and other specialty type walls aren't in the RCD/upgradable.

I can change this to a revert tomorrow if you really want but since we are on that biweekly release schedule would it not make more sense to have that maintainer discussion now and request changes to this PR? (Instead of reverting, opening a new PR re-adding the old stuff, maintainer discussion there) I'm down to make any requested changes since it's just yaml.

@keronshb
Copy link
Contributor

keronshb commented Nov 7, 2024

The main issue is that the original PR accidentally bypassed any official maintainer discussion.
As it is right now, it's also skipping the fact that we excluded this from RCDs for a reason - due to it being really powerful. Which is why we required the actual construction steps for these kind of windows. It's also why things like R-Walls and other specialty type walls aren't in the RCD/upgradable.

I can change this to a revert tomorrow if you really want but since we are on that biweekly release schedule would it not make more sense to have that maintainer discussion now and request changes to this PR? (Instead of reverting, opening a new PR re-adding the old stuff, maintainer discussion there) I'm down to make any requested changes since it's just yaml.

We already had a lengthy talk about this, was going to make a post later tonight, but if you could swap this to a revert that'd make things easier.

The biggest two reasons for a revert:

  1. It doesn't fit the game.
  • Only thing that really prevents people from spamming windows is materials. This is one of the reasons these window types weren't added to the RCD. To build these stronger windows, one of the biggest balance for them is the time & steps it would take to replace.
  • It removes the need for tools (welding, screwdriver, etc) to repair/remove/replace.
  • It makes space-facing windows a non-issue to deal with. To replace a space facing window you should bring space protection and holofans with you, and air refill as needed.
  1. It skipped the previous maintainer process unintentionally, so there was no discussion behind merging it back then.

@github-actions github-actions bot added the Changes: Sprites Changes: Might require knowledge of spriting or visual design. label Nov 7, 2024
Copy link
Contributor

github-actions bot commented Nov 7, 2024

RSI Diff Bot; head commit 6c7336b merging into 4ba8e35
This PR makes changes to 1 or more RSIs. Here is a summary of all changes:
Edit: diff updated after 6c7336b

@cohanna cohanna changed the title Remove upgrading reinforced windows to shuttle windows Revert #31978 Nov 7, 2024
@cohanna
Copy link
Contributor Author

cohanna commented Nov 7, 2024

Note: these to fixes
fix: Windows have been made to correctly display their damage visuals.
fix: Reinforced plasma windows have been given the correct amount of hp and no longer have 12x the amount they should.
Have been reverted as well. A new PR will need to be made to re-apply these fixes.

@keronshb
Copy link
Contributor

keronshb commented Nov 8, 2024

Could you keep the fix for damage visuals? Reinforced Plasma glass can stay reverted, would need to talk to the other devs about that.

Feel like the fix for the damaged visuals is fine, just not the upgrades.

Copy link
Contributor

@keronshb keronshb left a comment

Choose a reason for hiding this comment

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

Could you keep the fix for damage visuals? Reinforced Plasma glass can stay reverted, would need to talk to the other devs about that.

Feel like the fix for the damaged visuals & plasma is fine, just not the upgrades.

@cohanna tagging and requesting changes for visibility

@cohanna
Copy link
Contributor Author

cohanna commented Nov 12, 2024

@cohanna tagging and requesting changes for visibility

Seen, will implement changes as soon as I get the time to

@cohanna cohanna requested a review from keronshb November 13, 2024 02:54
@github-actions github-actions bot added the S: Needs Review Status: Requires additional reviews before being fully accepted label Nov 13, 2024
@cohanna cohanna changed the title Revert #31978 Half Revert #31978 Nov 13, 2024
@keronshb
Copy link
Contributor

Tests went over fine. This is good to go.

@keronshb keronshb changed the base branch from master to stable November 14, 2024 04:54
@keronshb keronshb merged commit f2b7743 into space-wizards:stable Nov 14, 2024
12 checks passed
@keronshb keronshb mentioned this pull request Nov 14, 2024
@cohanna cohanna deleted the shuttlewindowkill branch December 25, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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: Needs Review Status: Requires additional reviews before being fully accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants