Skip to content
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

address BoundingBoxRigHandle nullreferenceexception #2086

Closed
wants to merge 9 commits into from
Closed

address BoundingBoxRigHandle nullreferenceexception #2086

wants to merge 9 commits into from

Conversation

david-c-kline
Copy link

BoundingBoxRigHandle has a potential to encounter a NullReferenceException in OnSourceLost. This change avoids the exception as well as moving the call to eventData.Use inside the if block.

This MAY be related to what was reported in #2009. I was unable to reproduce the exact issue when I encountered this one.

[may18] Removing unused Animation script & animation in HolographicButton prefab
Copy link
Contributor

@StephenHodgson StephenHodgson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to use the inputDownEventData?

@david-c-kline
Copy link
Author

@cre8ivepark is likely the best to answer the question regarding inputDownEventData.

@david-c-kline
Copy link
Author

FYI: OnInputUp is setting inputDownEventData to null, so this exception will happen if a source is lost if not in a down state.

Copy link
Contributor

@StephenHodgson StephenHodgson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment

public void OnSourceLost(SourceStateEventData eventData)
{
if (eventData.SourceId == inputDownEventData.SourceId)
if ((inputDownEventData != null) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

event data should never be null. If it is then, something is being used improperly in another location.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this class is setting inputDownEventData during an up event. is this the wrong behavior? it has been this way since the start, I believe

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it is wrong, but we can address a fix in vNEXT

Copy link
Contributor

@StephenHodgson StephenHodgson May 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be related to those null references in the InputManger you were getting.

event datums should never be null

@david-c-kline
Copy link
Author

To reiterate, the change in this PR is to match the Update method and check to see if the cached inputDownEventData is null before attempting to dereference.

@david-c-kline
Copy link
Author

this PR got messed up due to me trying to keep it in sync with may18_dev.... going to re-branch and re-submit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants