Random bleeding on the human #1362 - #1519
Open
rohitkulkarni97 wants to merge 11 commits into
Open
rohitkulkarni97 wants to merge 11 commits into
rohitkulkarni97 wants to merge 11 commits into
Conversation
RE-SS3D#1362 was never a bleeding bug. Oxygen ran on three independent ~1Hz clocks with no shared ordering: the lungs' breath timer, the heart's beat timer, and OxygenConsumerSubSystem's own 1s timer. Whenever consume led deliver by one cycle, a zero-buffer limb was charged the full DamageWithNoOxygen lump while the blood pool still held hundreds of times the body's total need. That Oxy damage feeds RelativeDamage, RelativeDamage drives Bleed(), and the result was spontaneous bleeding and a suffocation death spiral. Deliver and consume now collapse into one ordered, dt-scaled operation, which makes the crossover structurally impossible rather than merely rarer: - OxygenConsumerSubSystem becomes MetabolicSubSystem, a thin ~10Hz scheduler holding IMetabolicController; IOxygenConsumer becomes IMetabolicController. - CirculatoryController.MetabolicTick owns the physiology per entity. Tissues pull from the shared blood pool, rate-limited by cardiac output and blood volume, and the pool is debited exactly once by what was actually taken. - CirculatoryLayer.MetabolicStep replaces ConsumeOxygen and ReceiveOxygen with a continuous dt-scaled draw, grading damage by the unmet fraction rather than applying a binary 25 point cliff. - Heart loses SendOxygen and its connected-parts graph walk, and gains CardiacOutputFactor - BPM/60, so a stopped heart moves no oxygen - and BeatTick, driven by the controller instead of its own Update. - HealthController.AddBodyPart gains a body so internal organs can announce themselves. It previously had no callers at all. Both renamed .meta files keep their original GUIDs, so existing scene references survive. The rename cannot be split out: landing the new .meta files while the old ones still exist would leave two assets claiming the same GUID. Nor can delivery be separated from consumption, since the whole point is that they stop being separate operations - an intermediate commit would compile but leave the body with no oxygen supply at all. Verified over a 3.5 minute run: the perfused set reaches hasHeart=true with count=14, no oxygen damage, no bleeding, clean shutdown. Known gaps, planned separately: severed body parts no longer decay, perfusion no longer stops at parts without a circulatory layer, and HealthController's body part list now includes internal organs, which shifts the lungs' breathing thresholds by roughly 5%.
CirculatoryController.OnStopServer called SubSystems.Get<MetabolicSubSystem>().UnregisterController(this) unguarded. On teardown the scheduler is destroyed before the entities registered with it, so Get logged "Couldn't find subsystem" and returned null, and the call then threw a NullReferenceException on every shutdown. Use the silent SubSystems.TryGet instead. Init still uses Get, because a scheduler missing at startup is a genuine fault and should stay loud. Introduced by the previous commit: develop's equivalent lived in CirculatoryLayer.Cleanlayer, which only runs on detach and never on scene teardown. The exception surfaced only in Unity's Editor.log and never in LogHost.log, so the run looked clean on the Serilog channel alone.
HealthController._bodyPartsOnEntity only ever grew. AddBodyPart had no callers before the oxygen rework, so nothing was added after spawn and nothing was ever removed - lose both legs and the body still computed oxygen demand for them. Its one consumer is Lungs.SetBreathingState, which sums demand across the list to pick a breathing state, so a wrong list moves those thresholds. - The list is now a HashSet, so "no duplicates" is enforced by the type rather than by every caller remembering to check first. A part tracked twice would silently double its share of the body's oxygen demand. - Membership doubles as the record of which parts are subscribed: Add and Remove report whether the part was new or actually present, so tracking and subscription update off one lookup and cannot disagree. - Removal drops the part and unsubscribes before announcing, so a listener reading BodyPartsOnEntity during the event sees the body as it now is. - OnStartServer routes its initial scan through AddBodyPart, so the two entry points cannot drift apart. - BodyPart.Dispose announces each internal organ before disposing it. This is the half that is easy to miss: DestroyBodyPart recurses through part.DestroyBodyPart() and so announces organs, but DetachBodyPart goes via Dispose, which fired no event at all. Detaching a head therefore removed the head and left the brain in the list forever - still counted in breathing demand, never dropped from the perfused set. BodyPartsOnEntity and ComputeIndividualNeeds widen from ReadOnlyCollection to IReadOnlyCollection to accept a set. Deliberate departures from develop, both invisible today because the breathing state field is written and never read: - breathing thresholds now sum over all 14 body parts rather than 10, aligning demand with blood volume, which was already computed over 14 - dismemberment now reduces demand, where before the list never shrank Verified by severing a head: tracked parts and the perfused set both go 14 -> 12, and whole-body demand drops to exactly 54,300 ml x 2.77e-6 = 0.150411.
A body part's oxygen demand is the average of GetOxygenNeeded() across its IOxygenNeeder layers, and CirculatoryLayer's constructor computed it. But that constructor runs from inside TryAddBodyLayer, while the layer list is still being built, so it averages over whatever happened to be added before it. Every part adds a MuscleLayer first and so got a sane value. Brain is the only one that adds its circulatory layer first, so it averaged over an empty list, landed on numberOfConsumers = 0, and set demand to zero. Nothing recomputed it, so it stayed zero for the object's whole life. A part that needs no oxygen can never run a deficit and can never take Oxy damage, so the brain has been silently exempt from the metabolic model - on this branch and on develop alike. Whole-body demand was short by exactly the brain's 1300 ml x 2.77e-6, and there is not one brain OXY DAMAGE line in any log. SetOxygenNeeded becomes public ComputeOxygenNeeded, called from BodyPart.OnStartServer once AddInitialLayers has returned. Fixing it there rather than by reordering Brain's layers means no part, present or future, can opt itself out through the order it happens to add its layers in. OxygenNeeded also becomes a plain auto-property: its old setter discarded the value assigned to it and called SetOxygenNeeded instead, and no caller ever used it. Verified: whole-body demand reads 0.164538 = 59,400 ml x 2.77e-6, the full 14-part volume, where it previously read 0.160937. This does not by itself make a severed head's brain decay - that additionally needs severed parts to be driven by the metabolic scheduler, which is a separate change. When that lands, decapitation becoming fatal will be new behaviour, not restored parity: the brain never decayed on develop either.
The perfused set was a flat scan of every BodyPart under the entity, plus each one's internal organs. develop derived it instead by walking outward from the heart and descending only through parts carrying a circulatory layer, with the rule stated in its own comment: fixing a living foot on a wooden leg won't prevent it from dying. The rework dropped that rule - a living part behind a non-circulatory prosthetic would have been perfused regardless. Walk from the heart's container again, and stop wherever circulation stops. AddIfPerfused returns whether the part was newly added, and the walk descends only on true. That enforces the connectivity rule and doubles as a visited check, so the walk also survives a cyclic topology - body parts are authored as a tree, but nothing in the code requires that. The walk runs from the metabolic tick, not from the add/remove handlers. This matters: a removal is announced inside BodyPart.DetachBodyPart, and the part is not unlinked from its parent until Dispose runs afterwards. Walking from the handler therefore still reaches the departing part and keeps it - severing a head left the head and its brain in the set indefinitely. Deferring by one tick lets the topology settle first, and coalesces the several events one dismemberment raises into a single walk. Deriving the set rather than editing it incrementally also removes the need for _pendingPerfusion and DrainPendingPerfusion, which existed to retry parts announced before their body layers were built. Both deleted. One deliberate divergence from develop: organs are checked for a circulatory layer like any other part. develop added them unconditionally and then dereferenced the layer it had not checked for, which was a crash rather than a behaviour worth keeping. Verified by severing a head: perfused set 14 -> 12 with the head and brain both gone, matching HealthController's own list, and whole-body demand 0.164538 -> 0.150411 - the exact 14-part and 12-part volumes. Zero oxygen damage, zero bleeding. The set now comes out in anatomical order from the torso outward, which is the visible signature that the walk replaced the flat scan. The connectivity rule itself cannot be tested end to end until prosthetics exist; it rests on the recursion being unable to reach a part whose parent carries no circulation.
The health system had no runtime levers. Dismemberment could only be triggered by destroying a part outright, which spawns no floor copy, and heart rate could not be changed at all - Heart.SetBeatFrequency existed with zero callers. Several claims about the circulatory model could therefore only be reasoned about, never observed. detachbodypart severs a part by destroying its bone layer alone. A part detaches while its bone layer is destroyed and the part as a whole is not, and bone maxes out at half the part's own maximum, so damaging bone alone lands in that gap. DestroyBodyPartCommand cannot do this: it damages every layer, so total damage always passes MaxDamage and the part is destroyed rather than detached. setheartrate sets a heart's rate in beats per minute. Cardiac output scales with it - CardiacOutputFactor is BPM/60 - so 0 stops delivery entirely and the body starves on its reserves, while values below 15 starve it gradually. Together these made three previously unobservable behaviours testable: that a stopped heart suffocates the body, that oxygen starvation causes bleeding, and that supply tracks heart rate. Both are administrator-level and server-only.
develop had every CirculatoryLayer register with a global consumer list from its constructor, so anything not fed by a beating heart drained its reserve and destroyed itself. The rework replaced that list with a per-entity CirculatoryController, and anything outside a living body then belonged to no controller at all - a severed limb lay on the floor untouched forever. BodyPart now implements IMetabolicController and registers with the scheduler while it is loose. A part with no blood supply is simply supply = 0 into the same CirculatoryLayer.MetabolicStep a perfused part uses, so there is no second decay model to keep in step with the living one. A part is loose when it is neither inside another body part nor under a HealthController. Both are required: each rules out one of the two things that could already be driving it - a body's circulatory controller, or a containing part. Ruling out only one would double-drive, and with an or every limb in a healthy body would starve itself while being perfused. Deliberately not keyed on _isDetached. Every body part is also a spawnable item, so one can enter the world already loose through an admin spawn, map placement or loot without passing through SpawnDetachedBodyPart, and develop decayed those identically because registration was blind to provenance. _isDetached is also set only after the copy is spawned, so a copy's own OnStartServer sees it false. Organs never register on their own account - the part containing them drives them through MetabolicTick - which is how a brain rots inside a severed head. Child parts are not walked: severing produces an independent copy per part with no parent link between the copies, so each already registers itself. Registration is refreshed at every transition that can change the answer: OnStartServer, SetParentBodyPart, AddInternalBodyPart, RemoveInternalBodyPart, Dispose and OnStopServer. TryGet rather than Get throughout, because on scene teardown the scheduler is destroyed before the parts registered with it. Bleed() stays unreachable from this path - it dereferences BodyPart.HealthController.Circulatory.Container, which is null on a loose part. develop had the same property, since Heart.Bleed only ran over _connectedToHeart and a floor copy was never in it. Bleeding for attached parts is unchanged. Verified by severing a head. Only the floor copies take damage; the living body is untouched. The brain decays first over 13 ticks, "brain dies" fires and kills the player, then the head follows over 7. Damage is graded and dt-scaled - the first tick shows an unmet fraction of 0.115 for 0.510 damage as the reserve runs out, then saturates at 4.4, which is 25 x 0.176s. The head needing only 7 ticks against a MaxDamage of 200 confirms it arrived carrying ~100 from the bone break. The brain dying is NEW behaviour, not restored parity. On develop its oxygen demand was zero - see the earlier ComputeOxygenNeeded commit - so it never decayed there, and decapitation was not fatal on this timer.
MetabolicSubSystem.Update iterates every registered controller, and CirculatoryController.ApportionOxygen iterates every perfused part. Neither contained a throw, so a single failure aborted the whole loop: parts after the failing one went untended, and the exception escaped into Update and silently stopped the metabolic tick for every other entity on the server until it stopped throwing. Both loops now catch per iteration and log with the exception attached. Nothing is swallowed - a contained error is still a loud error, just not a fatal one. This is not hypothetical. Destroying a heart left the destroyed part in the perfused set; MetabolicStep threw MissingReferenceException on it every tick, and because it sat at index 1 of 14, no part after it was ever ticked. The body looked like it died correctly, but only the torso had actually been simulated. The scheduler guard catches a second case the inner one cannot. Starvation damage destroys a part, destruction purges its containers, and AttachedContainer.Purge throws a NullReferenceException that predates this work. That unwinds through BodyPart.MetabolicTick, which loose decaying parts own directly rather than going through ApportionOxygen. Four floor copies dying in the same tick took four controllers down with them; without the scheduler guard they would have taken the still-living human's tick with them too. Catching Exception is deliberate. Continuing in a degraded state beats stopping the simulation for every body in the game.
RebuildPerfusedList rooted its walk at the heart, so destroying the heart left the set empty. That drove _sumNeed to 0, MetabolicTick returned early, and the body stopped being simulated part-way through dying - a corpse frozen rather than finished. develop suffocated it instead: consumption there ran off a global list that had nothing to do with the heart. The set conflated two questions. Which parts belong to this body is topology, and stays true whether or not anything is pumping. Which parts are receiving blood is flow. Rooting at the heart made the first depend on the second. Walk from the body's own root parts instead - no parent, and not an organ sitting inside something else. The heart's role stays entirely in cardiacFactor, which is 0 when it is missing or stopped, so every part is offered nothing and starves. Removing the heart without destroying it now behaves correctly too. The connectivity rule is unchanged: the recursion still refuses to cross a part with no circulatory layer, so a living foot on a wooden leg still dies. Only the starting point moved. Also stops using ?. and ?? on _heart. Those compile to a raw reference comparison and bypass Unity's overloaded lifetime check, so a destroyed heart would have kept reporting a live cardiac output. Verified across four runs. A sixteen minute baseline stays at count=14 with demand 0.164538 and no exceptions. Destroying the heart now starves every part in exact reserve order - limbs at 1f, organs at 3f, head at 5f, torso at 8f - where before only the torso was ever ticked. Stopping the heart at 0 BPM behaves the same, and severing a head still gives count=12 and demand 0.150411.
AttachedContainer.HandleStoredItemsChanged dereferenced oldItem.Item and newItem.Item without checking them. On a Clear there is nothing to dereference: SyncList.Clear calls AddOperation(SyncListOperation.Clear, -1, default, default), so both are null and the Remove branch threw a NullReferenceException. Purge makes this routine rather than exotic. It deletes every item and then clears the list, so each removal is announced after its item is already gone - and the items can be destroyed as well as null, which is why these test truthiness rather than comparing against null. handleItemAdded and handleItemRemoved already guard exactly this way, so the null payload was anticipated; these two dereferences twenty lines further down were simply missed. Pre-existing and unrelated to RE-SS3D#1362, but constantly reached by that work: destroying a body part purges its containers, so every death threw. The exception unwound through BodyPart.MetabolicTick into MetabolicSubSystem.Update, which iterates every registered controller - so one dying body could stop the metabolic tick for every other body on the server. Verified: destroying a heart now produces zero exceptions of any kind, where the same test previously produced between 15 and 154.
A destroyed part stayed in the perfused set until some unrelated removal happened to trigger the next re-walk. After the heart was destroyed that was 3.4 seconds and 18 caught exceptions, during which the rest of the body went unperfused. The walk reaches organs through InternalBodyParts, whose null check is Unity truthiness. Object.Destroy leaves the reference valid for the remainder of the frame it is called in, and the removal event, the re-derive and the destruction all land in that same frame, so truthiness is structurally one frame too late to be the guard here. Moving the deactivation earlier cannot close the gap. Reject on IsDestroyed instead. It is true the instant the damage lands, several calls before the event fires, since it is the condition InflictDamage tests before calling DestroyBodyPart at all. It also reads only managed state, so it is safe on a part Unity has already torn down. Verified with destroybodypart HumanHeart(Clone): the first re-derive now reads 14 -> 13, zero metabolic steps throw, and starvation runs in exact reserve order - limbs, then organs, then head, then torso. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rohitkulkarni97
temporarily deployed
to
unity_tests
August 2, 2026 04:31 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
rohitkulkarni97
changed the title
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the random bleeding on the human. Oxygen ran on three independent clocks — the lungs' breath timer, the heart's beat timer, and
OxygenConsumerSubSystem's fixed 1 s timer — which were never aligned. A healthy body could be told to consume oxygen before any had been delivered, producing a deficit with the heart still beating. That inflictedOxydamage, and sinceBleed()keys offRelativeDamage(which sums every damage type,Oxyincluded), the limb bled. Hence bleeding with no attacker and no wound.Replaced by one dt-scaled circulatory tick.
MetabolicSubSystemdrives every entity'sCirculatoryControllerat a shared ~10 Hz, and delivery and consumption happen in a single pass scaled by the samedt— so they cannot cross. Parts draw against an oxygen reserve and take graded damage proportional to unmet demand, replacing the old discrete "did I get my second's worth?" gate.Includes: the single-clock model; demand computed once a part's layers exist; a perfusion set derived from body topology and re-derived when it changes; decay of severed parts and the organs they carry; failure isolation so one broken body can't halt the simulation; two admin console commands for testing.
Does not include: changes to the bleeding trigger, blood regeneration, ischemia-tolerance retuning, or damage capacity — all pre-existing, listed under Changes.
PR checklist
Pictures/Videos
None — the fix is the absence of a symptom, which doesn't film. See the steps below.
Testing
Two admin console commands are added for this (F12, Administrator).
Heart.SetBeatFrequencypreviously had no callers at all, so none of this could be exercised in game.1. The bug is gone. Spawn and idle a minute or two. Nothing should take
Oxydamage or bleed.2. Suffocation still bleeds you.
setheartrate HumanHeart(Clone) 14— slow enough to starve, fast enough that the pulse still fires. Limbs takeOxydamage after ~15 s and start bleeding; organs, head, then torso follow. This isdevelopbehaviour and was never in scope to change; the fix is that it now needs a reason.3. The pulse triggers bleeding, not the starvation.
setheartrate HumanHeart(Clone) 0— starves in the same order, but nothing bleeds, becauseBleed()is never called.4. Losing the heart.
destroybodypart HumanHeart(Clone)— parts starve strictly by ischemia tolerance: limbs ~0.8 s, lungs and brain ~2.8 s, head ~4.8 s, torso ~7.8 s."brain dies"and the player is killed.5. Losing a limb or your head.
detachbodypart HumanArmLeft/HumanHead— the severed part drops and decays on its own, brain included.6. Multiplayer. Host plus two clients, repeat step 4 on one player. The other two must be untouched.
Logs:
Logs/LogHost.login the editor,Builds/Game/Logs/LogHost.login a build.Networking checklist
Changes
Scripts only — no asset, prefab or scene changes.
MetabolicSubSystem.csOxygenConsumerSubSystem.cs. One shared ~10 Hz server tick; each controller wrapped in try/catch so one failing body can't halt the loop.IMetabolicController.csIOxygenConsumer.cs. OneMetabolicTick(deltaTime)covering deliver-and-consume.CirculatoryController.csCirculatoryLayer.csMetabolicStep(supply, dt)— accept into reserve, consume, inflict graded damage on the shortfall.Bodypart.csIMetabolicController, so a loose part decays withsupply = 0. Demand computed afterAddInitialLayers().HealthController.csBodyPartsOnEntitymaintained in both directions — parts are removed, not only added.Heart.cs,Lungs.cs,HealthConstants.csCardiacOutputFactorscales delivery with BPM.AttachedContainer.csDetachBodyPartCommand.cs,SetHeartRateCommand.csCommits are atomic and in dependency order — reviewing them individually is much easier than the whole diff. The non-obvious design decisions are documented as XML comments at the lines they apply to, rather than repeated here.
The one behaviour change to agree to: decapitation is now fatal in ~5 s. On
developa severed head was survivable indefinitely, because the brain's oxygen demand was pinned at zero —CirculatoryLayercomputed demand in its own constructor, seeing only layers added ahead of it, andBrainis the only part that adds its circulatory layer first. So it averaged over an empty list. The brain couldn't suffocate, in a game where it holds the player's mind. Demand is now computed once all layers exist (whole-body0.160937→0.164538, exactly the brain's missing1300 mL × 2.77e-6).HeadBodyPartalready carried the comment "death is near though.." — a dying severed head was always the intent. Working as designed, but it changes how the game plays, so it shouldn't be discovered by surprise.Two smaller deviations from
develop: organs are now checked for a circulatory layer before perfusion (developdereferenced it unguarded — an NRE on any organ without one; latent, since nothing currently produces such an organ); and breathing thresholds shift ~5.3%, because demand now sums over all 14 body parts rather than 10, matchingBodyPartsVolume, which already sized blood over 14. Not observable today —Lungs.breathingis written and never read, ondeveloptoo.Nothing is left broken by this change. Verified in a standalone build with three simultaneous bodies, and on a local merge with #1494 — zero health-system exceptions in any run.
Separately, pre-existing health-system defects, none introduced or worsened here, listed so they aren't rediscovered: ischemia tolerance is backwards (limbs least tolerant at 1 s, torso most at 8 s);
Bleed()keys off totalRelativeDamageincludingOxy, so suffocation → bleeding → blood loss → worse suffocation; blood never regenerates (ProduceBlood()throws, zero callers); damage is never clamped (ClampDamagenever called); damage capacity ignores body-part volume.None. The pre-existing defects above are separate concerns and shouldn't gate this.
Related issues/PRs
Closes #1362
Related: #1494 (addressables / async loading). Merged onto it locally and tested, since #1362 lands first. It works, and the async path depends on machinery added here — with organs spawning asynchronously the controller builds its perfused set before any organ exists (10 parts, no heart) and grows to 14 as they attach. On
develop's synchronous spawn that never happens. One merge conflict, inBodyPart.OnStartServer.