vgstation-coders / vgstation13

Butts
GNU Affero General Public License v3.0
265 stars 544 forks source link

Area lightswitch toggling is not consistent #35446

Open boy2mantwicethefam opened 10 months ago

boy2mantwicethefam commented 10 months ago

The areas do not have a consistent designation, for example "crew quarters" includes the bar, the theater, the captain's office, any map where a head of staff has their own little bedroom, the courtroom, the bathrooms, the holodeck controls, Wheelstation's vacant kiosk and so on.

Also, why do lights start off by default instead of on?

At this rate it might actually be an improvement to get rid of roundstart lightswitching altogether given the kinks like having to delay the round to run it, leave it to the pre-roundstart ghosts to toggle the lights off in departments instead of players.

xXxEMBERGHOSTSQUADDIExXx commented 10 months ago

personally i like the idea of areas being treated on an individual basis and not lumping them into departments for these purposes per se but that didnt seem to be popular one solution to what you described is to define departments in a way other than using typepaths

whether or not they start on and get turned off, or they start off and get turned on, the effect is the same in terms of performance the latter is better because generally there will be less occupied areas than unoccupied areas so less iterating is required. and it has the nice property that if no players are in the game at roundstart, nothing needs to be flipped (the opposite case would require every lightswitch to be flipped) and we can operate on small lists that only grow as needed versus large lists that we subtract from.

rather than considering it a roundstart delay therefore some kind of aberration i think its useful to consider it part of the loading process. like the object subsystem takes however many seconds to initialize, and the lights take whatever fraction of a second they currently do (can ctrl+f the logs for "turned the lights on") in that way of thinking, all is right with the world

boy2mantwicethefam commented 10 months ago

When there are less players there is less strain on roundstart overall. The "lights off then on" implementation means it's less straining on lowpop, when there are fewer players, and more straining on highpop, when there are more players and thus more setup. Unfortunately the light switching is positioned at roundstart, after the jobs have been assigned but when players are waiting for the round to properly start. I don't think it feels nice when the round takes about 6-7 extra seconds at a decent population to start. You aren't the only coder to recommend throwing it under the rug in some way as either "this is not a problem" or "just make some graphics to hide the loading". I also don't like that this is after a long string of updates that already attempted to make roundstart faster without a very noticeable improvement.

xXxEMBERGHOSTSQUADDIExXx commented 10 months ago

check the logs, it's pretty fast now We want to shave cpu off where we can but ideally not at the expense of removing content

Also in practice there is a 300 second wait period between rounds. A few more seconds of loading to dress the players etc. is only a drop in the bucket relative to that. It's not really a problem that is actually causing issues for the playerbase in practice. It's moreso something that gets blown out of proportion by coders fixating on it as they are forcing roundstart many times in a row for testing, and also because of the memetic seeding via adacovsks past controversial focus on micro optimizations of roundstart

I do agree it should be as fast as feasible but we don't need to pare gameplay and aesthetic things away for this very minor thing. Instead we can focus on optimization where appropriate.

Also maybe add some detailed time logging to all of the steps of the roundstart process so we can focus on what should be optimized more

The way the fade-in animation pr was framed also added confusion over this topic

boy2mantwicethefam commented 10 months ago

The 5 minute setup is a deliberate delay to let more people join the round before it starts, the wait that happens afterwards can be shaved off. The wait that happened before also got shaved off, pipes got a much more optimized initialize() There are two other roundstart things that need more optimization: the job ban check and record creation. The former goes through a database of thousands and thousands of job bans in a very unoptimized way, others have already planned to fix it but I'm not sure anyone will actually work on it. The latter is record injection due to 4 icons being generated for every player, tried to fix it but it threw up bugs on the live server and I made a revert PR for it, supposedly the new BYOND version will have an in-built version that can generate icons out of appearance instead. What I don't like is that the light stuff deliberately delays the round just to give some more time to render stuff without looking ugly. We could have achieved the same thing with no time delay, just with all lights being on rather than some being off.

xXxEMBERGHOSTSQUADDIExXx commented 10 months ago

From the player POV, 'roundstart' is not so much when a certain proc triggers, moreso when they see their character and can start moving

I think we should have everything in its nicely loaded state (lights on, nobody naked, etc) on the first tick that the player sees their character. If we have that working nicely there isn't really a substantiative 'issue' per se; anything beyond that is mainly the realm of optimization.

xXxEMBERGHOSTSQUADDIExXx commented 10 months ago

@boy2mantwicethefam re. the job bans thing, the main issue is jobban_isbanned() right?

xXxEMBERGHOSTSQUADDIExXx commented 10 months ago

idea for reworking the department thing:

area/ var/department

define DEPT_AREAS_SCIENCE list(/area/science/telesci, /area/science/rdoffice, ...)

area/science/telesci department = DEPT_AREAS_SCIENCE