add a command to enable atmos devices on arbitrary maps#4049
add a command to enable atmos devices on arbitrary maps#4049Alkheemist wants to merge 10 commits intonew-frontiers-14:masterfrom
Conversation
|
reject ECS, return to hashset |
|
Realistically, this is going to be used for two things mainly: Cryo pods on the Rimworld event, and making the scrubbers work on the minicentcomm POI. Both of which are great to me! Though, how would I go about testing that before/after profiling stuff? |
|
Since it's unlikely mamy maps will be set like this, and we're not using special properties of HashSets, it would probably be more optimal to use a normal list. HashSets have to hash the value being checked before knowing if it's in the list, which does have some measure of cost. But since we're unlikely to get more than 10 of these maps on any shift even in generous terms, the O(n) check of a list would probably beat the O(1) check with hash cost. Lists will also play nicer with caching and branch prediction. Granted, we are checking an integer, so that cost will be especially low and probably measured in terms of ns. A SortedSet could also work if we'd want to skip hashing and get O(log(n)) lookups. |
|
rewrote to use ECS since we'd need to do so for persistence reasons later. |
Content.Server/Atmos/EntitySystems/AtmosphereSystem.Commands.cs
Outdated
Show resolved
Hide resolved
apply suggestions from the-hivequeen Co-authored-by: the-hivequeen <49570376+the-hivequeen@users.noreply.github.com>
arimah
left a comment
There was a problem hiding this comment.
I'm sorry, I'm a nitpicky bastard 😢 Looks good, just one TINY change.
Content.Server/Atmos/EntitySystems/AtmosphereSystem.Commands.cs
Outdated
Show resolved
Hide resolved
blackknight954
left a comment
There was a problem hiding this comment.
tested and works as intended, now all we need is working thermal scanners..
About the PR
Adds a command to re-enable atmos on maps.
Why / Balance
Admins can have a little atmos, as a treat
Technical details
Adds a hashset containing maps that atmos should be enabled on to the atmos system and checks if the map is in that hashset when checking whether or not to process atmos devices.
Note that the way AtmosInputCanRunOnMap works means that this is more computationally expensive the more atmos devices you have on maps that are not the main one, so hopefully the additional check isn't going to bog down the server. But before/after profiling should probably be done to see the real impact.
And the hashset seems cleaner to check compared to iterating through a list. The easiest would be a field on the component, but that's in the engine and I don't want to try and mess with that right now.
How to test
Media
dotnet_01_11-24-15.mp4
Requirements
Breaking changes
Changelog
N/A, admin only