Fix sporadic crash on home screen #1645
Conversation
| backdropColor = "#00a4db" ' set default in case global var is invalid | ||
| localGlobal = m.global | ||
|
|
||
| if isValid(localGlobal) and isValid(localGlobal.constants) and isValid(localGlobal.constants.poster_bg_pallet) |
There was a problem hiding this comment.
Since we're using an index value to directly access the poster_bg_pallet array, we should use isValidAndNotEmpty() on it here to ensure it has values.
There was a problem hiding this comment.
I'll update this but I don't think it's possible for the array to be valid and empty
There was a problem hiding this comment.
I did some testing on this. An empty array will evaluate as valid.
test = []
print isValid(test)
Output will be true
There was a problem hiding this comment.
I agree with what you're saying in general but I don't think it's possible for this specific array to ever be valid and empty. It's assigned a value when it's created (the 3rd line of actual code in main) and we never modify these values because they are constants. So why would it be empty?
There was a problem hiding this comment.
So why would it be empty?
The same reason why the global variables that we know are set before this code hits are getting flagged as inValid - because Brightscript defies reason 😆
There was a problem hiding this comment.
I have trust issues with brightscript too but the more I think about this the more I think we should leave it as it is and only make your change as needed if it shows up in the crash log. This node is used for every media item on the home screen so there could be dozens or hundreds of these HomeItems on the screen and for each one we'd need to check if the global constant array is empty in addition to all the extra validation that was added in this PR.
There was a problem hiding this comment.
That's a good point. It would be a good refactoring to instead pass the background color to the homeitem on generation and do the valid check on the global values in the parent element. That way we reduce the number of checks we perform on global.
There was a problem hiding this comment.
Approved as-is, but make note a good follow up would be for us to test the m.global variable before the home items are generated and pass the chosen backdropcolor into it. That could potentially save us tons of condition checks.
However, it will require a refactoring to make it happen.
The crash in #1642 should never happen in theory because we are setting the global constants super early in the app. This PR should fix the crash by setting a default backdrop color value and overwriting it with a random value like before but only if the global var is valid.
Issues
Fixes #1642