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

Show correct icons in remote object inspector (EditorDebuggerRemoteObject) #98156

Merged

Conversation

jaydensipe
Copy link
Contributor

@jaydensipe jaydensipe commented Oct 13, 2024

Used to implement godotengine/godot-proposals#2084, used to fix #95209. <- Better implemented in #88427.

This commit allows the remote scene view to display custom icons from classes (annotated with @icon) and allows the remote object inspector to display the correct class icon for an Object. These help a lot with readability.

This commit allows the remote object inspector (EditorDebuggerRemoteObject) to display the correct class icon for an Object.


Before After
Screen.Recording.2024-10-13.185549.mp4
Screen.Recording.2024-10-13.185222.mp4

@jaydensipe jaydensipe requested a review from a team as a code owner October 13, 2024 22:50
@timothyqiu timothyqiu added this to the 4.4 milestone Oct 13, 2024
Copy link
Member

@timothyqiu timothyqiu left a comment

Choose a reason for hiding this comment

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

I think you can merge #98157 into this one. It is essentially the same as this one.

@jaydensipe jaydensipe force-pushed the show-custom-icons-in-remote-scene-view branch from 45dbad6 to 0739cf0 Compare October 14, 2024 00:00
@jaydensipe jaydensipe requested review from a team as code owners October 14, 2024 00:07
@jaydensipe jaydensipe force-pushed the show-custom-icons-in-remote-scene-view branch from d5c1db8 to 0c8efba Compare October 14, 2024 00:10
@jaydensipe jaydensipe changed the title Show custom icons in remote scene view Show custom icons in remote scene view and correct icons in remote object inspector (EditorDebuggerRemoteObject) Oct 14, 2024
@timothyqiu
Copy link
Member

I set the node's icon via @icon:

@icon("res://icon.svg")
extends Node2D
Before After Expected
Remote Scene Tree No Icon No Icon Icon
Remote Inspector Icon No Icon Icon

Project: test.zip

@jaydensipe
Copy link
Contributor Author

jaydensipe commented Oct 14, 2024

I set the node's icon via @icon:

@icon("res://icon.svg")
extends Node2D

Before After Expected
Remote Scene Tree No Icon No Icon Icon
Remote Inspector Icon No Icon Icon
Project: test.zip

So, I did a lot of looking into this; to fix your test you can declare a class_name like so, and it will work: (BUT...)

@icon("res://icon.svg")
extends Node2D
class_name Test2D

I just found out there was another PR (#88427) that implemented this... oops :P. Theirs works without declaring class_name so depending on if we want to merge that one or I can work on this one to implement that. Theirs also doesn't implement the EditorDebuggerRemoteObject fix, so I can wait until theirs gets merged to use some of their code to implement that better.

@CrayolaEater
Copy link
Contributor

CrayolaEater commented Oct 14, 2024

If implementation wise your fix is similar to mine then I don't think having 2 open PRs that solve the same problem is a good idea. Maybe create a PR only for the EditorDebuggerRemoteObject fix?

@jaydensipe jaydensipe force-pushed the show-custom-icons-in-remote-scene-view branch from 0c8efba to 252047d Compare October 14, 2024 09:07
@jaydensipe jaydensipe changed the title Show custom icons in remote scene view and correct icons in remote object inspector (EditorDebuggerRemoteObject) Show correct icons in remote object inspector (EditorDebuggerRemoteObject) Oct 14, 2024
@jaydensipe
Copy link
Contributor Author

If implementation wise your fix is similar to mine then I don't think having 2 open PRs that solve the same problem is a good idea. Maybe create a PR only for the EditorDebuggerRemoteObject fix?

Agreed, changed this PR into the EditorDebuggerRemoteObject fix, but if we need a new PR for some reason I can do that. I tested my changes with yours, and they worked good. BTW, your fix works for Autoload singleton scripts which is awesome and will help a ton with my projects.

@timothyqiu
Copy link
Member

Currently, the node's icon is completely missing in the remote object inspector.

Using the test project from #98156 (comment)

@jaydensipe jaydensipe force-pushed the show-custom-icons-in-remote-scene-view branch from 5e43959 to 1ff12d0 Compare October 15, 2024 08:23
@jaydensipe
Copy link
Contributor Author

Currently, the node's icon is completely missing in the remote object inspector.

Using the test project from #98156 (comment)

Hey, #88427 has code in it that would fix the icon missing when not defining a class_name, so would we wait until that is merged to merge this?

Copy link
Member

@timothyqiu timothyqiu left a comment

Choose a reason for hiding this comment

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

Works as expected with #88427 which should be merged first.

@jaydensipe jaydensipe force-pushed the show-custom-icons-in-remote-scene-view branch from 1ff12d0 to 7261321 Compare October 15, 2024 20:40
@Repiteo Repiteo merged commit c032acb into godotengine:master Oct 16, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 16, 2024

Thanks!

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

Successfully merging this pull request may close these issues.

Remote Scene doesn't use C# IconAttribute for node instanciated, but Inspector is able to use it?
5 participants