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

Moved temperature to shared #31249

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Jezithyr
Copy link
Contributor

@Jezithyr Jezithyr commented Aug 21, 2024

About the PR

Moved Temperature component, ThermalRegulator component and most of the implementation of temperature, thermal regulation and heater systems to Content.Shared. This will allow shared content to interact with TemperatureComponent without requiring a ServerSide implementation. This does not implement predicted temperature, it only networks the component states.

Why / Balance

No visible changes for players. This was done to make working with temperature in shared systems easier.

Technical details

Simply moved temperature, thermal regulation and heater systems to shared along with their associated components. I also networked the components to make sure that the data is accessible on the client as well. I did not implement prediction since temperature interacts with atmospherics which is a purely server-side system and would make prediction more difficult. Values on temperature related components are calculated on the server and then networked to the clients.

Media

Requirements

  • I have read and I am following the Pull Request Guidelines. I understand that not doing so may get my pr closed at maintainer’s discretion
  • I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

Namespace changes:

ThermalRegulatorComponent: Content.Server.Body.Systems -> Content.Shared.Body.Systems
ChangeTemperatureOnCollideComponent: Content.Server.Temperature -> Content.Shared.Temperature
ContainerTemperatureDamageThresholdsComponent: Content.Server.Temperature -> Content.Shared.Temperature
EntityHeaterComponent: Content.Server.Temperature -> Content.Shared.Temperature
InternalTemperatureComponent: Content.Server.Temperature -> Content.Shared.Temperature
TemperatureComponent: Content.Server.Temperature -> Content.Shared.Temperature
TemperatureProtectionComponent: Content.Server.Temperature -> Content.Shared.Temperature

Obsolete methods (will be removed in the future):

public void ForceChangeTemperature(EntityUid target, float temp, TemperatureComponent? temperature = null)
public float GetHeatCapacity(EntityUid uid, TemperatureComponent? comp = null, PhysicsComponent? physics = null)
public void ChangeHeat(EntityUid uid, float heatAmount, bool ignoreHeatResistance = false, TemperatureComponent? temperature = null)

Changelog

@github-actions github-actions bot added the Status: Needs Review This PR requires new reviews before it can be merged. label Aug 21, 2024
@Jezithyr Jezithyr mentioned this pull request Aug 21, 2024
1 task
@slarticodefast
Copy link
Member

I think this one is a duplicate of #30511
Not sure if it includes all the same files though.

@deltanedas
Copy link
Contributor

crusher dagger disgustingly op

@Jezithyr
Copy link
Contributor Author

I think this one is a duplicate of #30511 Not sure if it includes all the same files though.

I totally forgot about that one 😅
That said, this implementation puts less networking load on the server (Temperature components aren't being dirtied every tick only when they actually get changed)

Comment on lines +55 to +56
heater.Comp2.Load = SettingPower(setting, heater.Comp1!.Power);
Appearance.SetData(heater, EntityHeaterVisuals.Setting, setting);
Copy link
Contributor

Choose a reason for hiding this comment

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

Sussy null suppression because this can cause nullrefs and isn't guaranteed to exist.

Comment on lines +10 to 11
[RegisterComponent, Access(typeof(SharedEntityHeaterSystem))]
public sealed partial class EntityHeaterComponent : Component
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs NetworkedComponent also it's being dirtied but there's nothing actually being synced.

Comment on lines +14 to 15
[RegisterComponent, Access(typeof(SharedTemperatureSystem))]
public sealed partial class InternalTemperatureComponent : Component
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs NetworkedComponent

Comment on lines +21 to +24
[DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
public float CurrentTemperature = Atmospherics.T20C;

[DataField, ViewVariables(VVAccess.ReadWrite)]
[DataField, ViewVariables(VVAccess.ReadWrite), AutoNetworkedField]
Copy link
Contributor

Choose a reason for hiding this comment

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

VVRW unnecessary on datafields.

Comment on lines +90 to +115

private void UpdateDamage(float frameTime)
{
_accumulatedFrametime += frameTime;

if (_accumulatedFrametime < UpdateInterval)
return;
_accumulatedFrametime -= UpdateInterval;

if (!ShouldUpdateDamage.Any())
return;

foreach (var comp in ShouldUpdateDamage)
{
MetaDataComponent? metaData = null;

var uid = comp.Owner;
if (Deleted(uid, metaData) || Paused(uid, metaData))
continue;

ChangeDamage(uid, comp);
}

ShouldUpdateDamage.Clear();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Was going to comment this breaks persistence but it was already like this so.

Comment on lines +314 to +317
{
var temperatureQuery = GetEntityQuery<TemperatureComponent>();
var transformQuery = GetEntityQuery<TransformComponent>();
var thresholdsQuery = GetEntityQuery<ContainerTemperatureDamageThresholdsComponent>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can move these to the system.

Comment on lines +332 to +333
RecursiveThresholdUpdate(uid, GetEntityQuery<TemperatureComponent>(), GetEntityQuery<TransformComponent>(),
GetEntityQuery<ContainerTemperatureDamageThresholdsComponent>());
Copy link
Contributor

Choose a reason for hiding this comment

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

Same just use it on the system.

@github-actions github-actions bot added Merge Conflict This PR currently has conflicts that need to be addressed. labels Aug 25, 2024
Copy link
Contributor

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

1 similar comment
Copy link
Contributor

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

@metalgearsloth metalgearsloth added Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged. and removed Status: Needs Review This PR requires new reviews before it can be merged. labels Aug 25, 2024
@Mishics
Copy link

Mishics commented Oct 15, 2024

Hey, when will you continue working? Or someone on vacation... Just wondering, nothing more

@a-person5660
Copy link

are you still working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Merge Conflict This PR currently has conflicts that need to be addressed. Status: Awaiting Changes This PR needs its reviews addressed or changes to be made in order to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants