Null reference on SharedSource/Items/Item.cs line 57

[jigardi]

Disclaimers

• [X] I have searched the issue tracker to check if the issue has already been reported.
• [X] My issue happened while using mods.

What happened?

First of all, thanks for making this beautiful game! : D

While working on my first real mod, I decided to make the simplest item I could imagine: a bouncy ball.

I disabled all other mods, using only my new WIP mod for the bouncy ball, and the autmatically generated one for the submarine I used to test it with.

While testing, I got the bouncy ball to implode and then delete itself by triggering code in a block, then figured the next and final step would be to make it conditionally triggered based on pressure - so I began to delve into the sourcecode for items.

private Hull currentHull; public Hull CurrentHull { get { return currentHull; } set { currentHull = value; } }

Almost right at the top, on lines 54-62, CurrentHull is defined and exposed, so without further ado I figured I'd use that as the conditional trigger.

This triggers precisely when intended, but perhaps not as intended, since it also instantly crashes the game, if the item is ever outside of a hull.

As far as I know, there is no way to fix or handle that from the modding/xlm side. (Unless there is a way to trigger on null?)

I really didn't want to give up on this though, so after some back and forth on the #baro-modding thread on Discord(kudos! @ "Bringbacktinkering", "Coda, Apostle of [0,255,169]" and "frog" ), I looked over the Items source code once more.

public float HullOxygenPercentage { get { return CurrentHull?.OxygenPercentage ?? 0.0f; } }

Just below the CurrentHull definition, on lines 64-67, HullOxygenPercentage is defined and exposed, and it is guaranteed to always return a value, even if CurrentHull itself is null.

At first I thought this could be a simple oneline fix, but there are several lines of code below the CurrentHull declaration, that rely on CurrentHull being null, when an item is outside of a hull.

In my specific usecase, reliance on HullOxygenPercentage is functionally equivalent to CurrentHull/pressure more than 99% of the time ingame; that is, if CurrentHull didn't crash the game and pressure was accessible.

As is, using HullOxygenPercentage remains technically wrong, and causes undesirable edge cases, such as the ball imploding in compartments where oxygen is depleted e.g. from a fire or characters breathing, and I suspect also in fully-flooded compartments regardless of the pressure therein(don't know if flooding displaces atmosphere).

As far as I can tell, there are three main ways to solve this, but I am not sure which is the most sensible:

1 ) Allow conditionals to check for null. 2 ) Expose pressure the same way as OxygenPercentage already is within item.cs 3 ) Rewrite CurrentHull to return something instead of null, and rewrite all remaining nullchecks in the file accordingly, so that other functionality is not broken by the fix.

Some of these solutions are technically also feature requests, except the features have already been programmed into the game - they are just not accessible. So I will also post in Barotrauma Discussions, and add links between them.

Reading the code made me wonder why only OxygenPercentage is exposed from Hull to the items? I'm guessing the variables have been variably exposed, based on arising need of items over the development of the game, and that no item hitherto needed to know if it was contained in a hull / under pressure?

This game and community has given me much joy, so these were my two cents back, hopefull they will be of some use. Thanks again for making this beautiful game! Looking forward to what the future holds!

Edit: Clarity. Some of the links got borked. Not sure how, think I spaced it too closely or interfered by ending the lines with a period. Hopefully fixed now.

Reproduction steps

1. Add the following xml to any item.

`

`

2. Throw that item out the airlock.

3. Observe how the game crashes.

NB: I'm fairly certain it crashes no matter what the value of the conditional string actually is, except if the string is actually empty, in which case it raises an error as it should. To my knowledge this can only be fixed from the xml side, if it is somehow possible to tell the conditional to expect a null - but as far as I know, that is not possible?

Bug prevalence

Happens every time I play

Version

0.19.14.0

-

No response

Which operating system did you encounter this bug on?

Windows

Relevant error messages and crash reports

Barotrauma Client crash report (generated on 11/15/2022 13:53:25)

Barotrauma seems to have crashed. Sorry for the inconvenience! 

0DC439F0B0CA5A5A6693500952309D61

Game version 0.19.14.0 (ReleaseWindows, branch release, revision 0daf5b87bd)
Graphics mode: 3840x2160 (BorderlessWindowed)
VSync ON
Language: English
Selected content packages: Vanilla, SRV Thalassa, IronbardsFirstMod
Level seed: no level loaded
Loaded submarine: SRV Thalassa (5ABC80C551CC63BB1880A67662640D29)
Selected screen: Barotrauma.GameScreen
SteamManager initialized

System info:
    Operating system: Microsoft Windows NT 10.0.19044.0 64 bit
    GPU name: NVIDIA GeForce GTX 1080
    Display mode: {Width:3840 Height:2160 Format:Color AspectRatio:1.7777778}
    GPU status: Normal

Exception: Object reference not set to an instance of an object. (System.NullReferenceException)
Target site: Boolean Matches(Barotrauma.ISerializableEntity, Barotrauma.SerializableProperty)
Stack trace: 
   at Barotrauma.PropertyConditional.Matches(ISerializableEntity target, SerializableProperty property) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\StatusEffects\PropertyConditional.cs:line 408
   at Barotrauma.PropertyConditional.Matches(ISerializableEntity target, Boolean checkContained) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\StatusEffects\PropertyConditional.cs:line 210
   at Barotrauma.StatusEffect.<HasRequiredConditions>g__AnyTargetMatches|104_1(IReadOnlyList`1 targets, String targetItemComponentName, PropertyConditional conditional) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\StatusEffects\StatusEffect.cs:line 1034
   at Barotrauma.StatusEffect.HasRequiredConditions(IReadOnlyList`1 targets, IReadOnlyList`1 conditionals, Boolean targetingContainer) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\StatusEffects\StatusEffect.cs:line 932
   at Barotrauma.StatusEffect.Apply(ActionType type, Single deltaTime, Entity entity, IReadOnlyList`1 targets, Nullable`1 worldPosition) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\StatusEffects\StatusEffect.cs:line 1158
   at Barotrauma.Item.ApplyStatusEffect(StatusEffect effect, ActionType type, Single deltaTime, Character character, Limb limb, Entity useTarget, Boolean isNetworkEvent, Boolean checkCondition, Nullable`1 worldPosition) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\Items\Item.cs:line 1667
   at Barotrauma.Item.ApplyStatusEffects(ActionType type, Single deltaTime, Character character, Limb limb, Entity useTarget, Boolean isNetworkEvent, Nullable`1 worldPosition) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\Items\Item.cs:line 1568
   at Barotrauma.Item.Update(Single deltaTime, Camera cam) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\Items\Item.cs:line 1938
   at Barotrauma.MapEntity.UpdateAll(Single deltaTime, Camera cam) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\Map\MapEntity.cs:line 600
   at Barotrauma.GameScreen.Update(Double deltaTime) in <DEV>\Barotrauma\BarotraumaShared\SharedSource\Screens\GameScreen.cs:line 254
   at Barotrauma.GameMain.Update(GameTime gameTime) in <DEV>\Barotrauma\BarotraumaClient\ClientSource\GameMain.cs:line 855
   at Microsoft.Xna.Framework.Game.DoUpdate(GameTime gameTime) in <DEV>\Libraries\MonoGame.Framework\Src\MonoGame.Framework\Game.cs:line 656
   at Microsoft.Xna.Framework.Game.Tick() in <DEV>\Libraries\MonoGame.Framework\Src\MonoGame.Framework\Game.cs:line 500
   at Microsoft.Xna.Framework.SdlGamePlatform.RunLoop() in <DEV>\Libraries\MonoGame.Framework\Src\MonoGame.Framework\SDL\SDLGamePlatform.cs:line 92
   at Microsoft.Xna.Framework.Game.Run(GameRunBehavior runBehavior) in <DEV>\Libraries\MonoGame.Framework\Src\MonoGame.Framework\Game.cs:line 397
   at Microsoft.Xna.Framework.Game.Run() in <DEV>\Libraries\MonoGame.Framework\Src\MonoGame.Framework\Game.cs:line 367
   at Barotrauma.Program.Main(String[] args) in <DEV>\Barotrauma\BarotraumaClient\ClientSource\Program.cs:line 58

Last debug messages:
[11/15/2022 13:53:05] WARNING: Could not compress a texture because the dimensions aren't a multiple of 4 (path: Content/Items/Legacy/legacyrailgunbarrel.png, size: 125x336)
[11/15/2022 13:52:57] WARNING: Could not compress a texture because the dimensions aren't a multiple of 4 (path: Content/Items/Legacy/legacyrailgunbarrel.png, size: 125x336)
[11/15/2022 13:52:07] Attempting to open ALC device "OpenAL Soft on Øresnegl på headset (6- CORSAIR HS70 Pro Wireless Gaming Headset)"
[11/15/2022 13:52:05] Logged in as Ironbard (SteamID STEAM_1:1:13121056)

[jigardi]

Addendum: Doh! I found an error while reviewing the corresponding feature request.

The way that pressure is assigned when outside a hull, should be the same as for HullOxygenPercentage - but the value should not be zero, as that would indicate absolute vacuum. Instead the pressure needs to be calculated. The following is the calculation from BarotraumaShared/SharedSource/Map/Hull.cs lines 610-615:

`surface = rect.Y - rect.Height + WaterVolume / rect.Width;

if CLIENT

        drawSurface = surface;

endif

        Pressure = surface;
    }

`

If an item can be considered as a pointmass, then everything after the minus should cancel out leaving the calculation as simply:

Pressure = rect.Y;

If the item cannot be considered a pointmass, then perhaps the following makes some sense:

Pressure = rect.Y - body.height + ( (body.height * body.width) * body.density ) / body.width;

Or if a rough approximation is sufficient:

Pressure = rect.Y + body.mass / body.density;

[Regalis11]

Thank you for the report! Fixed the crashing and implemented support for comparing to null in https://github.com/Regalis11/Barotrauma-development/commit/20577cfa45cdf3f7504c91c68d6b0906e6c4b739 (link to our private repo).

The way that pressure is assigned when outside a hull, should be the same as for HullOxygenPercentage - but the value should not be zero, as that would indicate absolute vacuum.

That's not actually the case. The oxygen percentage is not supposed to indicate the amount of air in the hull, but the amount of oxygen. Running out of oxygen in a poorly ventilated room doesn't mean the room has turned into a vacuum, just that there's not enough oxygen. The way I see it, 0 is the most sensible value to return when outside hulls: there is no oxygen around the item.

The suggested changes for calculating the pressure an item is under also wouldn't work, as it only considers the vertical position of the item, not the pressure inside the hull they're in (or the depth of the level when outside).

[jigardi]

Wohoo! I'm so glad to have been of help, that totally made my day : D

Ah, I see my mistake now. You perfectly explained my error, I somehow conflated HullOxygenPercentage with the remaining air in the hull. You are absolutely right, Pressure should definitely be 0 then.

Also, regarding the suggested new calculations above - I see now that I failed to explain my thinking properly - I must have forgotten to write out some thoughts between my erroneous vacuum logic and the suggested calculations.

I did not mean for the suggested calculations to replace the current calculations within BarotraumaShared/SharedSource/Map/Hull.cs, as those already work perfectly.

I'm not even sure that the suggested calculations should be in the Hull.cs file, since the suggested calculations only apply when the item is outside of a hull, and so cannot rely on a CurrentHull to calculate the pressure.

That is why I wrote out the approximations, where the "hull" dimensions are borrowed from the item.body dimensions, as those are the only comparable values items have access to, when they are outside of a hull. (Doh! I see now that I wrote simply, body.width - but it should have been item.body.width. Also I should have written that the suggestions were pseudocode, as the code would definitely need to change if it were no longer contained in Hull.cs)

My impression of the current calculations in Hull.cs, is that they approximate realworld calculations for pressure of a standing water column, but sort of boiled down to a single dimension.

A textbook realworld pressure equation:

P = ( ( p A h ) * g ) / A

Where P is the total pressure in pascals, p is the density of the fluid in kg/m^3, A is the cross-sectional area of the water column, h is the height of the water column in meters, and g is the local gravity in m/s^2.

This equation can be boiled down further as the two A's are canceled out, yielding:

P = h p g

The main takeaway of these equations, is that since p and g are constants(not technically true for p in saltwater, but close enough I suppose), pressure scales linearly with the height h of the water column; or in other words pressure scales linearly with depth below the surface.

For a game the precision of the liquid density and the gravitational constant are perhaps overkill, so they are simply not considered?

Is that not what happens in those calculations?

My understanding of it is as follows:

rect.Y represents the depth of the water column if it was measured from 0 at top and down to the bottom of the CurrentHull, rect.height is the height of the CurrentHull, which is subtracted from rect.Y since we are going to add back some height in the next step and don't want to overshoot, and finally the added height in the last step, is equal to the WaterVolume divided by the rect.width, or in other words the width of the CurrentHull.

The reason density p and local gravity g aren't present in the ingame equation, is because of this 1 dimensional approximation, where P i.e. total pressure is equated with height, implicitly yielding the ingame formula for pressure as:

P = rect.Y - CurrentHull.height + (CurrentHull.WaterVolume / CurrentHull.width)

which when fully flooded is simply equal to

P = rect.Y

Is this not how it works?

Edit: Clarity. Edit1: Left out a part of the final ingame equation. Edit:2 Doh! Also mangled the final equation by using wrong signs and names. Edit3: More clarity. Edit4: Added missing detail to main takeaway explanation. The ideal equation is very approximately true for pure water, but not saltwater, especially at the oceanic scale. Edit5: More clarity, damn it xD

[Regalis11]

I think you might be overestimating how "physically realistic" these ad-hoc formulas we use in the game were intended to be. :D

Basically, the "pressure" value doesn't represent any real unit, nor does it even make much sense if you look at the value itself (it can even be negative!). The value only works when comparing the pressure of the hull relative to another hull, and that's essentially the only thing it's used for - if the pressure in some hull is larger than in the one it's connected to, it pushes water into that hull.

It's also worth noting this value calculated based on the position of the surface of the water is only the initial value. Once the game starts running, that value soon becomes irrelevant: the pressure starts changing based on the amount of water moving from hull to another (and based on water entering the sub from outside). For instance, if you have two hulls on top of each other, the water flowing to the bottom hull starts increasing the pressure there. And if the bottom hull is connected to somewhere else, the pressure will start pushing water there.

This isn't exactly how water pressure actually works, and it doesn't take many real-life factors into account, but it's a simple approximation that provides results that are realistic-looking enough for the purposes of the game.

And yes, you are absolutely right, if the hull is fully flooded the initial pressure value will be P = rect.Y.

[jigardi]

Hello again! : D

I think you might be overestimating how "physically realistic" these ad-hoc formulas we use in the game were intended to be. :D

Aye, you're right again. I overestimated because I thought they were derived from the pressure equations. The fact that they are not makes it even more impressive; the effect is so convincing!

Basically, the "pressure" value doesn't represent any real unit,

Sweet! That's what I had deduced, thanks for confirming!

nor does it even make much sense if you look at the value itself (it can even be negative!).

Huh!? Does that occur when the sub is above 0 depth? I don't see how that would happen just by looking at it, but I'll definitely take your word for it xD This is so much fun though man, really makes me want to write my own watercode just to play around with and learn.

The value only works when comparing the pressure of the hull relative to another hull, and that's essentially the only thing it's used for - if the pressure in some hull is larger than in the one it's connected to, it pushes water into that hull.

Ah I see now. That instantly cleared out a misconception of mine. Without realizing, I applied my real life intuitions about water to the water in the game. I did not even consider that the different pressure systems might be decoupled, yet having them decoupled makes a lot of sense from a development standpoint.

It's also worth noting this value calculated based on the position of the surface of the water is only the initial value. Once the game starts running, that value soon becomes irrelevant: the pressure starts changing based on the amount of water moving from hull to another (and based on water entering the sub from outside). For instance, if you have two hulls on top of each other, the water flowing to the bottom hull starts increasing the pressure there. And if the bottom hull is connected to somewhere else, the pressure will start pushing water there.

Hmmm ok, I think it's all starting to make sense now. Does that mean the water rushes in at the same speed / with the same volume regardless of depth?

This isn't exactly how water pressure actually works, and it doesn't take many real-life factors into account, but it's a simple approximation that provides results that are realistic-looking enough for the purposes of the game.

I totally agree! It's made playing this game so much fun : D The water code causes all the right emotions at the right time! From worries about leaks and rising water levels, panic at approaching(or exceeding!) crush depth, plus a ton of fun moments near airlocks/doors/hatches and hull breaches.

And yes, you are absolutely right, if the hull is fully flooded the initial pressure value will be P = rect.Y.

Yay! I was right!

It's also worth noting this value calculated based on the position of the surface of the water is only the initial value.

Hmmm... but based on all of this, does it not make sense for P = rect.Y to be an approximation for pressure on items outside the hull? I still think it would be a worthwhile addition for modders at the very least.

The pressure data wouldn't even necessarily have to be tied to the object. It could simply be calculated on a per-request basis, this way it shouldn't break any existing items, and would have a minimal memory footprint where it is not used. It could perhaps even be a static funtion?

For regular people playing the game this has been a non-issue, however, as a modder it feels strange not having uniform access to pressure data, especially when the name of the game literally translates as "pressure wound". : P

Whichever direction you choose, thank you very much for your time, and the swift replies and fixes! PS: Great job on the upcoming update! It looks super exciting! Follwing the progress of this game has been and continues to be a source of joy and inspiration : D

[NilanthAnimosus]

Tested against release commit https://github.com/Regalis11/Barotrauma-development/commit/aa8388e4f9e8067110ef6827b3b1760f5a2f3f97, no issues found, closing.

Notes: currenthull="eq 0" should instead by checked by using currenthull="eq null" (and for checking a hull exists, not equals)

[jigardi]

Tested against release commit https://github.com/Regalis11/Barotrauma-development/commit/aa8388e4f9e8067110ef6827b3b1760f5a2f3f97, no issues found, closing.

Notes: currenthull="eq 0" should instead by checked by using currenthull="eq null" (and for checking a hull exists, not equals)

Thanks for the early Christmas present : D And Merry Christmas to the lot of you!