-
Notifications
You must be signed in to change notification settings - Fork 241
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
Adjust Hook worldobject to catch null crashes #3686
Conversation
@@ -27,7 +27,7 @@ public class Hook : Container | |||
|
|||
public bool HasItem => Inventory != null && Inventory.Count > 0; | |||
|
|||
public WorldObject Item => Inventory != null ? Inventory.Values.FirstOrDefault() : null; | |||
public WorldObject Item => Inventory?.Values.FirstOrDefault(); |
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.
Should not have anything related to null around it.
It is impossible for Inventory to be null
@@ -62,7 +62,7 @@ public override ActivationResult CheckUseRequirements(WorldObject activator) | |||
var rootHouse = House.RootHouse; | |||
var houseOwner = rootHouse.HouseOwner; | |||
|
|||
var houseHooksVisible = rootHouse.HouseHooksVisible ?? true; | |||
var houseHooksVisible = rootHouse?.HouseHooksVisible ?? true; |
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 check is pointless. The line immediately above where it says rootHouse.HouseOwner will have already crashed.
@@ -236,7 +236,7 @@ public void UpdateHookVisibility() | |||
{ | |||
if (!HasItem) | |||
{ | |||
if (!(House.HouseHooksVisible ?? false)) | |||
if (!(House?.RootHouse?.HouseHooksVisible ?? false)) |
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.
I was okay with putting this around 1 place where the actual crash occurred, and maybe some logging to find out why that happened, and which specific house was null, however i'm not as okay with sprinkling this logic around randomly throughout the program. House and RootHouse shouldn't be null. All of this code implies that they can be null, and no explanation as to why.
No description provided.