- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 23.5k
 
Fix crash for classes without an icon #112339
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
Conversation
| 
           Ran into this just now. This does fix the issue for me. Happend when Ctrl + clicking on   | 
    
        
          
                editor/doc/editor_help.cpp
              
                Outdated
          
        
      | void EditorHelp::_add_type_icon(const String &p_type, int p_size, const String &p_fallback) { | ||
| Ref<Texture2D> icon = EditorNode::get_singleton()->get_class_icon(p_type, p_fallback); | ||
| if (icon.is_null()) { | ||
| return; | 
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.
| return; | |
| icon = EditorNode::get_singleton()->get_class_icon("Object"); | |
| ERR_FAIL_NULL(icon); | 
To match behavior to be the as before the change that introduced the crash (fallback to default Object "cube" icon).
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.
It should be ERR_FAIL_COND(icon.is_null()); in this case, you can't use ERR_FAIL_NULL here
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.
Not having an icon will leave an empty space (two spaces around the missing icon), so having it will make help page look more consistent.
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.
The crash is caused by @GDScript and @GlobalScope lacking an icon. Therefore, if we want to handle this error at this stage, we have two options: either implement special handling for these internal names, or assign them a fallback icon.
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 agree that we should fallback to Object again, like before. I'll amend this so we can merge for 4.6-dev3.
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.
Since there's no consensus about the icon, it's fine to keep it as is, since it is fixing the crash. We can decide which icon to use/not use later.
64b0f61    to
    8a6d044      
    Compare
  
    | 
           Thanks!  | 
    
Fixes #112304
The editor would crash due to a null pointer dereference in EditorHelp::_add_type_icon when trying to display the help page for a class that does not have a registered icon (e.g., internal types like
@GlobalScope).This PR adds null check to
EditorHelp::_add_type_icon