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

Fewer Mid-Round Events #486

Merged
merged 1 commit into from
Jun 25, 2024

Conversation

DEATHB4DEFEAT
Copy link
Member

@DEATHB4DEFEAT DEATHB4DEFEAT commented Jun 24, 2024

Description

Ports and improves Simple-Station/Parkstation-Friendly-Chainsaw#59

Slows rounds down so the station doesn't always have to evacuate in a blaze and can instead leave complete.
Also makes Survival not stupidly destructive and gives a chance for the shift to last a decent time.
Allows servers to configure the times themselves if they want to, so defaults don't matter too much.


Changelog

🆑

  • tweak: Mid-round events will occur much less often

@DEATHB4DEFEAT DEATHB4DEFEAT added Size: 4-Small For small issues/PRs Priority: 3-Medium Needs to be resolved at some point Type: Balancing Balancing some feature Type: Port Brings something to here from another codebase Type: Cleanup Like Rework but small labels Jun 24, 2024
@github-actions github-actions bot added the Changes: C# Changes any cs files label Jun 24, 2024
@SimpleStation14 SimpleStation14 changed the title fewer mid-round events Fewer Mid-Round Events Jun 24, 2024
@github-actions github-actions bot added the Status: Needs Review Someone please review this label Jun 24, 2024
Copy link
Member

@VMSolidus VMSolidus left a comment

Choose a reason for hiding this comment

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

I genuinely appreciate that this lets us CVar configure survival rounds.

Copy link
Member

@VMSolidus VMSolidus left a comment

Choose a reason for hiding this comment

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

Actually, now that I think about it, this system should actually use "Hot Loaded" CVars for the game event timers, which following our repository conventions, would necessitate creating a StationEventSchedulerSystem.CVars.cs file similar to the ones present in AtmosphereSystem.CVars.cs and CPRSystem.CVars.cs. It would be extremely useful for server hosts to be able to on the fly modify the next instance of the event scheduler. Plus it would make all of this code a lot cleaner by removing all of the repeated instances of _config.GetCVar(CCVars.variable).

@DEATHB4DEFEAT
Copy link
Member Author

Actually, now that I think about it, this system should actually use "Hot Loaded" CVars for the game event timers, which following our repository conventions, would necessitate creating a StationEventSchedulerSystem.CVars.cs file similar to the ones present in AtmosphereSystem.CVars.cs and CPRSystem.CVars.cs. It would be extremely useful for server hosts to be able to on the fly modify the next instance of the event scheduler. Plus it would make all of this code a lot cleaner by removing all of the repeated instances of _config.GetCVar(CCVars.variable).

This is horrible, I'm not doing it like this.

image

I do have another similar idea, though, one second.

@DEATHB4DEFEAT
Copy link
Member Author

I do have another similar idea, though, one second.

It did not work.

@VMSolidus
Copy link
Member

I do have another similar idea, though, one second.

It did not work.

I'm not sure why the convention of essentially moving all of the _config.GetCvar stuff to a separate file isn't acceptable? From where I'm standing it looks pretty good? We're essentially just moving all of the Cvar logic to a separate file, so the main bulk of the system's logic becomes a lot more human readable.

@DEATHB4DEFEAT
Copy link
Member Author

I'm not sure why the convention of essentially moving all of the _config.GetCvar stuff to a separate file isn't acceptable? From where I'm standing it looks pretty good? We're essentially just moving all of the Cvar logic to a separate file, so the main bulk of the system's logic becomes a lot more human readable.

There's so much extra stuff for negligible "benefit" that will break easier, and it's very unclear what those variables actually are/do and what you can do with them.

@VMSolidus
Copy link
Member

I'm not sure why the convention of essentially moving all of the _config.GetCvar stuff to a separate file isn't acceptable? From where I'm standing it looks pretty good? We're essentially just moving all of the Cvar logic to a separate file, so the main bulk of the system's logic becomes a lot more human readable.

There's so much extra stuff for negligible "benefit" that will break easier, and it's very unclear what those variables actually are/do and what you can do with them.

Can you help me understand your point of view here? Can you explain in more detail what it is you are suggesting might break if done this way? Can we not simply put documentation on the variables for the EventSchedulerSystem.CVars.cs file?

@DEATHB4DEFEAT
Copy link
Member Author

Can you explain in more detail what it is you are suggesting might break if done this way?

Anything. Adding more redundant things mean it's more likely someone will forget or not know something, and it won't be changed.

Can we not simply put documentation on the variables for the EventSchedulerSystem.CVars.cs file?

No, that's more duplicate stuff.
(Inheritdoc does exist though)

@VMSolidus
Copy link
Member

Can you explain in more detail what it is you are suggesting might break if done this way?

Anything. Adding more redundant things mean it's more likely someone will forget or not know something, and it won't be changed.

Can we not simply put documentation on the variables for the EventSchedulerSystem.CVars.cs file?

No, that's more duplicate stuff. (Inheritdoc does exist though)

Well my underlying problem with this now is that we are dealing with two different standards present in the repository, and no consistent guideline on which to do here. I genuinely could have swore I was under the impression we actually wanted to more consistently apply this specific application of .Component.cs, System.cs, and System.CVars.cs, and that we had discussed this fairly early on in the repository's history.

Does inheritdoc inherit across different system spaces?

@DEATHB4DEFEAT
Copy link
Member Author

Well my underlying problem with this now is that we are dealing with two different standards present in the repository, and no consistent guideline on which to do here.

What you sent me to is not at all standard and will not be.

I genuinely could have swore I was under the impression we actually wanted to more consistently apply this specific application of .Component.cs, System.cs, and System.CVars.cs, and that we had discussed this fairly early on in the repository's history.

We do, we just need to figure out a decent way of doing the CVar part.

Does inheritdoc inherit across different system spaces?

Don't know why it wouldn't.

@DEATHB4DEFEAT DEATHB4DEFEAT requested a review from VMSolidus June 25, 2024 20:39
Copy link
Member

@VMSolidus VMSolidus left a comment

Choose a reason for hiding this comment

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

I will keep this in mind in the future.

@VMSolidus VMSolidus merged commit 1132b05 into Simple-Station:master Jun 25, 2024
18 checks passed
SimpleStation14 added a commit that referenced this pull request Jun 25, 2024
VMSolidus pushed a commit that referenced this pull request Jul 5, 2024
# Description
This restores the basic/survival event rates to how they were before
before #486 (however, it keeps the CVars created in it).

- 5-25 minutes for basic instead of 15-35
- 4-12 minutes for survival instead of 20-45

---

# Why
The PR made it so that survival rounds actually have less events than
extended - that is considering that survival rounds are supposed to be
troublesome and filled with events while extended ones are supposed to
be a chill alternative.

Other forks don't bother with editing CVars every 5 minutes, so this
"opportunity to configure their experience" was never addressed. Based
on the feedback I've received from other players, the change was rather
negative.

From my personal experience, survival rounds became an extended
extended, where during a 2-hour shift you can get a grand total of one
power outage and two mice infestations, and nothing else at all.

---

# Changelog

:cl:
- tweak: Events should now occur as frequently as before. Note: server
owner can configure the frequency on their server manually.

Signed-off-by: Mnemotechnican <[email protected]>
DangerRevolution pushed a commit that referenced this pull request Aug 1, 2024
# Description

Ports
Simple-Station/Parkstation-Friendly-Chainsaw#39

A change made to encourage people to [stop and smell the
roses](https://www.urbandictionary.com/define.php?term=slow+down+and+smell+the+roses),
instead of sprinting everywhere trying to get shit done.
This goes well with #486, so people don't actually *have* to rush places
to try to get things done before the shift ends fatally.
It's weird anyway how we're all constantly sprinting everywhere and have
to *very actively* choose not to (and why would you?).
Increases the default speeds so that walking isn't painfully slow and
sprinting feels more like sprinting in combination with the active
choice to sprint.

Someone needs to PR changing the default sprint or examine buttons, so
people can fight and sprint with this change.
(A lot of other default keybinds suck or conflict too and need to
change)

# Media

Terrible video but whatever


https://github.com/user-attachments/assets/5ff3863d-92c8-4df3-b76b-82874b5e1ae3

# Changelog

:cl:
- tweak: The station's crew hivemind has decided to slow down their
movement and enjoy The Park instead of sprinting everywhere

---------

Signed-off-by: DEATHB4DEFEAT <[email protected]>
Co-authored-by: Pspritechologist <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changes: C# Changes any cs files Priority: 3-Medium Needs to be resolved at some point Size: 4-Small For small issues/PRs Status: Needs Review Someone please review this Type: Balancing Balancing some feature Type: Cleanup Like Rework but small Type: Port Brings something to here from another codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants