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

Subsysteming Attempt 2...? #3912

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

MarkSuckerberg
Copy link
Member

About The Pull Request

This is cursed, and I apologise to tmt for doing this like this

My attempt at reviving #2087

Modifies /datum/map_generator code somewhat, allowing for the actual work of generation to be moved to a subsystem. This subsystem then processes a single map generator at a time. This prevents multiple concurrent map generations from lagging the server immensely by eating entire ticks or waking before the MC. Virtual level clears are implemented as a "map generator" that clears every turf passed to them: they were causing the other half of the lag. These map generation jobs are added at a lower priority than planet generation, as there's nothing especially urgent about them.

To communicate map generation progress to players, the spawn_dynamic_encounter proc may now take an additional arg -- the ship requesting the dock. This ship's helms will announce updates on the generation progress every 3 seconds or so according to the number of datums and their progress which remain ahead of the relevant datum in the map generation queue. This should make it a little more obvious that progress is actually being made.

Once again tries to move map generation and deletion to a subsystem. This time with some interesting workarounds to make SSair fire every tick to make sure it does run, as well as running as a background subsystem so it can use up any spare processing time to keep working.

Also adds an element for registering signals from a movable's current vlevel, updated whenever that changes. I... really don't know how we're going to handle stuff without vlevels anymore. I need to either go full in on making sure nothing can exist outside of one somehow, or just chicken out and make there be a default fallback vlevel for stuff. And a signal that gets sent on vlevels when they're starting to clear so we can get spawners to stop and such. Hopefully I can get that all hammered out. Or maybe I won't and we'll live with that one painful sleepless movable deletion loop.

todo:

  • Actually test this on a live game to see if my changes actually fixed anything
  • See if I can make the template placement phase actually use MC_CHECK_TICK somehow. probably make it return false when incomplete and provide a wrapper that does a while() loop until it's done, idk
  • Find out if I can remove ALL of the stragglers that are left by my "improvement" to movable clearing code. also probably move that into the subsystem somehow
  • Handle docking progress more gracefully, make it more accurate, and make it UI
  • Find out if using can_fire is actually bad (I'm not a big SS_HIBERNATE fan)

Why It's Good For The Game

Big source of lag, reduced to atoms. Also gives people a better representation of how long dock should take.

Changelog

🆑
tweak: Overmap docks to outposts or encounters which require map generation will now announce their progress periodically via the helm.
refactor: Map generation is now queued via a subsystem, reducing lag by preventing them from hogging CPU time.
/:cl:

@github-actions github-actions bot added DME Edit Code change Watch something violently break. labels Dec 16, 2024
@thgvr
Copy link
Member

thgvr commented Dec 16, 2024

I'm worried it'll still take 10 minutes

@MarkSuckerberg
Copy link
Member Author

nah I changed it around so it actually runs every tick* now instead of only being in the background. in testing it never got over a minute for generation even when constantly generating and deleting planets

I could be way off the mark though so shrug. just wanted to test a silly idea I had

@thgvr
Copy link
Member

thgvr commented Dec 16, 2024

Previous attempt was also fine stress testing locally we'll see I guess

@MarkSuckerberg
Copy link
Member Author

yup. that's why it needs testing on live

I'm fully prepared to scrap it a second time if it doesn't work so don't worry, but I'm also fully prepared to tweak it as much as I can

@github-actions github-actions bot added the Merge Conflict Use Git Hooks, you're welcome. label Jan 20, 2025
Copy link
Contributor

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

@Imaginos16
Copy link
Member

PRAYING that it works this time, this would be AMAZING to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code change Watch something violently break. DME Edit Merge Conflict Use Git Hooks, you're welcome.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants