Fix few value is null cases I found. - #724
pietru2004 wants to merge 2 commits into
Conversation
BastiaanOlij
left a comment
There was a problem hiding this comment.
Other than some nitpicks on the syntax, this looks good. I do question the ground physics change though, might need @Malcolmnixon to weigh in.
| # Select the ground physics | ||
| var physics_node := ground_node.get_node_or_null("GroundPhysics") as XRToolsGroundPhysics | ||
| ground_physics = XRToolsGroundPhysics.get_physics(physics_node, default_physics) | ||
| if ground_node!=null: |
There was a problem hiding this comment.
Same here, just change to if ground_node:
There was a problem hiding this comment.
(though if I am not mistaken, XRToolsGroundPhysics.get_physics already does a null check and returns default_physics if physics_node is not usable, so possibly this fix is incorrect.
There was a problem hiding this comment.
IDK about XRToolsGroundPhysics.get_physics, but when I was developing my project it was one of points where I got null exception so mayby it failed to do so ?
There was a problem hiding this comment.
Hmm, I'd like to get @Malcolmnixon reaction on this bit then. I think its supposed to work so there may be a bug that needs fixing. Else we may end up disabling the default physics logic.
|
removed those 3 |
| # Update ground velocity information | ||
| _previous_ground_node = ground_node | ||
| _previous_ground_global = ground_collision.get_position() | ||
| _previous_ground_local = ground_node.to_local(_previous_ground_global) |
There was a problem hiding this comment.
I think this needs an else so that if we don't have a ground node, we don't keep some old ground node in the previous values.
Other than that, this looks good now.
I was testing my game and got variable is null errors at
var obj = with.get_collider()ground_node = ground_collision.get_collider()var collision_node := collision.get_collider()Added
variable!=nullchecks before code parts that would use those...Hope those are good...