Simple-Station / Einstein-Engines

A Space Station 14 upstream repository, inspired by Baystation12.
https://einstein.simplestation.org
GNU Affero General Public License v3.0
24 stars 49 forks source link

Fewer Mid-Round Events #486

Closed DEATHB4DEFEAT closed 3 days ago

DEATHB4DEFEAT commented 5 days ago

Description

Ports and improves https://github.com/Simple-Station/Parkstation-Friendly-Chainsaw/pull/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

:cl:

DEATHB4DEFEAT commented 3 days ago

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](https://github.com/Simple-Station/Einstein-Engines/assets/77995199/956537d6-a4c8-4a77-8af1-56e5d24a68e0)

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

DEATHB4DEFEAT commented 3 days ago

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

It did not work.

VMSolidus commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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 commented 3 days ago

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.