-
Notifications
You must be signed in to change notification settings - Fork 85
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
Avoid null pointer exception in the UIScaler #839
Conversation
TLM/TLM/U/UIScaler.cs
Outdated
get { | ||
UIView uiView = Singleton<UIView>.instance; | ||
if (uiView != null) { | ||
Camera uiCamera = uiView.uiCamera; |
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.
just a minor note. it would be more compact to write like this:
UIView uiView = Singleton<UIView>.instance;
Camera uiCamera = uiView?.uiCamera;
if(uiCamera !=null)
...
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.
But this does not eliminate the null check, in your code null check will happen twice.
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.
yeah its the same code just more compact.
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.
Code compactness is nice, but not at the expense of obscuring clarity of intent of the code.
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 didn't manage to reproduce the problem using TMPE labs. So I cannot test if it is fixed now. the code looks good.
Pushed the suggested change. |
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'll see if this fixes any of the issues I'd been seeing in game. |
Fixes #838
Bug
UIScaler
was usinguiCamera
's pixel size, while eitherUIView.instance
orUIView.instance.uiCamera
was null. This is now cached and double checked.Potentially if UI never changes from 1920x1080, we can replace that entire code with a pair of
const
Solution
A null check was added for both
UIView.instance
andUIView.instance.uiCamera
, cached width and height are stored in case those are ever null.Also: Change to
WorldToScreenPoint
has been reverted as it always been usingScreen.height
not GUI height.