-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Model static fires equalizing with the local atmosphere #32760
base: master
Are you sure you want to change the base?
Changes from 11 commits
20b7032
e49e7e3
6798591
42c6498
17601a3
cf43d86
f12048b
3300293
e641878
ff63faa
47d93e9
bce6b81
8451de0
7305c39
9d291bb
ed349f4
e045b71
97d413c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ | |
using Robust.Shared.Physics.Events; | ||
using Robust.Shared.Physics.Systems; | ||
using Robust.Shared.Random; | ||
using Content.Shared.Atmos.Piping.Binary.Components; | ||
|
||
namespace Content.Server.Atmos.EntitySystems | ||
{ | ||
|
@@ -82,6 +83,7 @@ public override void Initialize() | |
SubscribeLocalEvent<ExtinguishOnInteractComponent, ActivateInWorldEvent>(OnExtinguishActivateInWorld); | ||
|
||
SubscribeLocalEvent<IgniteOnHeatDamageComponent, DamageChangedEvent>(OnDamageChanged); | ||
|
||
IProduceWidgets marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
private void OnMeleeHit(EntityUid uid, IgniteOnMeleeHitComponent component, MeleeHitEvent args) | ||
|
@@ -413,7 +415,6 @@ public override void Update(float frameTime) | |
|
||
_timer -= UpdateTime; | ||
|
||
// TODO: This needs cleanup to take off the crust from TemperatureComponent and shit. | ||
var query = EntityQueryEnumerator<FlammableComponent, TransformComponent>(); | ||
while (query.MoveNext(out var uid, out var flammable, out _)) | ||
{ | ||
|
@@ -433,20 +434,51 @@ public override void Update(float frameTime) | |
|
||
if (flammable.FireStacks > 0) | ||
{ | ||
var air = _atmosphereSystem.GetContainingMixture(uid); | ||
var air = _atmosphereSystem.GetContainingMixture(uid, false, true); | ||
IProduceWidgets marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// If we're in an oxygenless environment, put the fire out. | ||
if (air == null || air.GetMoles(Gas.Oxygen) < 1f) | ||
if (flammable.FuelGasMix != null) | ||
{ | ||
Extinguish(uid, flammable); | ||
continue; | ||
// If we're in no atmos extinguish. | ||
if (air == null) | ||
{ | ||
Extinguish(uid, flammable); | ||
continue; | ||
} | ||
|
||
// If our local atmos doesnt have enough of the required gases extinguish. | ||
if (!flammable.FuelGasMix.LEQMoles(air)) | ||
{ | ||
Extinguish(uid, flammable); | ||
continue; | ||
} | ||
} | ||
|
||
var source = EnsureComp<IgnitionSourceComponent>(uid); | ||
_ignitionSourceSystem.SetIgnited((uid, source)); | ||
|
||
if (TryComp(uid, out TemperatureComponent? temp)) | ||
_temperatureSystem.ChangeHeat(uid, 12500 * flammable.FireStacks, false, temp); | ||
{ | ||
// get hot | ||
EnsureComp<AtmosExposedComponent>(uid); // required for the entity to ever cool down. | ||
if (!_physicsQuery.TryComp(uid, out var physics)) | ||
continue; | ||
|
||
var maxDeltaT = flammable.PeakFlameTemperature - temp.CurrentTemperature; | ||
IProduceWidgets marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (maxDeltaT > 0) // we dont want to burn and get colder. | ||
{ | ||
|
||
var tempBump = MathF.Sqrt(maxDeltaT * temp.SpecificHeat * physics.Mass) * flammable.FireStacks; //monotonic function, slows as it reaches maxT | ||
_temperatureSystem.ChangeHeat(uid, tempBump, false, temp); | ||
IProduceWidgets marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a really roundabout way to do I still don't think this is necessarily the right way to do this, because it does not respect energy conservation in terms of energy supplied by the candle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into this a bit more, if we do this (which is something like was originally there): Understanding that, I think we have two alternatives.
Both solutions seem fine to me, I do think the asymtote would be easier to maintain and harder to abuse. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, so superconduction is the thing that makes the station not an ideal enclosed box. The built-in "asymptoting function" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or I guess we can do a hybrid, and have it asymptote with a smaller minimum joule gain. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well hold on. That might be total joules contained in the entity. So then, ~3hr burn time, we would get something like 1,400j/s Thats still pretty high, but not as high. Okay sure, lets try it just with no asymptote, and just see how it feels. |
||
} | ||
|
||
// release gas | ||
if (air != null && flammable.EmissiveGasMix != null) | ||
{ | ||
flammable.EmissiveGasMix.Temperature = air.Temperature; | ||
var releasingGas = flammable.EmissiveGasMix.Clone(); // generate gas for the entity's entire burning lifespan, but we use ReleaseGasTo to handle temperature/pressure for us since those will change for other reasons. | ||
_atmosphereSystem.ReleaseGasTo(releasingGas, air, releasingGas.Pressure); // doing it this way means that we dont need to track the entity damage values or know its destructible or mobstate thresholds. | ||
} // the downside, and a possible later rework, is that you ""lose"" some gas if the candle is burning in an atmosphere where it can't emit due to pressure. | ||
IProduceWidgets marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} // the yml is also slightly cumbersome, because we now specify a "maximum release mix per update (second)" instead of a total gasmix in the entity. | ||
|
||
var ev = new GetFireProtectionEvent(); | ||
// let the thing on fire handle it | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,7 +160,8 @@ private void OnAtmosExposedUpdate(EntityUid uid, TemperatureComponent temperatur | |
var heatCapacity = GetHeatCapacity(uid, temperature); | ||
var heat = temperatureDelta * (airHeatCapacity * heatCapacity / | ||
(airHeatCapacity + heatCapacity)); | ||
ChangeHeat(uid, heat * temperature.AtmosTemperatureTransferEfficiency, temperature: temperature); | ||
ChangeHeat(uid, heat * temperature.AtmosTemperatureTransferEfficiency, temperature: temperature); // move heat to entity | ||
_atmosphere.AddHeat(args.GasMixture, -1 * heat * temperature.AtmosTemperatureTransferEfficiency); // remove heat from air | ||
Comment on lines
+163
to
+164
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Separate PR, because this is a bug fix to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its necessary for temp-emitting fires. |
||
} | ||
|
||
public float GetHeatCapacity(EntityUid uid, TemperatureComponent? comp = null, PhysicsComponent? physics = null) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
- type: entity | ||
name: air candle box | ||
parent: BoxCandle | ||
id: BoxCandleAir | ||
components: | ||
- type: StorageFill | ||
contents: | ||
- id: CandleGasNitrogen | ||
amount: 9 | ||
- id: CandleGasOxygen | ||
amount: 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would need to be in a separate PR if we make this change. I need to spend some time looking at this and see if exciting the mixture is really needed to get temperature to transfer correctly with
AtmosExposed
, but this may have significant detrimental performance impacts.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a nitpick, please name the optional non-default arguments (e.g.
GetContainingMixture((uid, transform), excite: true)
. This makes it a lot more apparent what the argument is, especially if someone is reviewing from a diff/GitHub/anything but Rider.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tested it without exciting it did work, but then the room's gas mix could have different temperatures in different tiles that wouldn't change until something else excited the gas in the room.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the interest of fixing the bug, Ill move the gas emission stuff to another PR, I think for it to work I need burning to have temperature behave reasonably so it makes more sense to start there. I do agree that its kind of soaped into something else in supporting making static fires modeled now.