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

Add class icons to script list #96865

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aXu-AP
Copy link
Contributor

@aXu-AP aXu-AP commented Sep 11, 2024

Implements godotengine/godot-proposals#3557

Draws icons representing the script's or doc page's class on the right side of the script editor. Supports custom icons and icons inherited from parent class.

kuva

@aXu-AP aXu-AP requested a review from a team as a code owner September 11, 2024 15:59
Draws icons representing the script's or doc page's class on the right side of the script editor. Supports custom icons and icons inherited from parent class.
@Mickeon
Copy link
Contributor

Mickeon commented Sep 11, 2024

I remember when this was being discussed as different icons other than the gear were added. Given what was argued last time, I suppose having the icon on the right side is a good call.

Part of me wishes it weren't the case. Doing it this way harshly reduces horizontal space, which the editor is already sorely missing, and potentially introduces clutter. A bit of a sensory overload, and whatnot.

I can see this being an Editor Setting, but keeping it off by default is a nasty choice that severely harms discoverability. I can also see a compromise, where only the documentation pages have an icon, at least by default.

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Sep 11, 2024

Yeah, when this idea was approved (comment), it was mentioned that the preferred side is on the right, so I made it so. There was also mention of it functioning as a button to open docs, but I'm not fancy of that idea seeing it as easily misclicked (should be added to context menu though, I always wonder how we are supposed to open custom class docs).

Editor setting could be good to have, as this does add some visual noise that not everyone will appreciate. I think that the benefit of finding right scripts at glance outweigh the cons for most people though. And who doesn't love to see their custom @icons all over the editor 😆

@Mickeon
Copy link
Contributor

Mickeon commented Sep 11, 2024

Thank you for linking to the original proposal. I commented a mockup for potentially further discussion.

I always wonder how we are supposed to open custom class docs

Unrelated to this PR, but yeah...

@Maran23
Copy link
Contributor

Maran23 commented Sep 12, 2024

I also like the original idea in the proposal more, or from @Mickeon. Two icons are much more distracting. And is very unusual for a list, not just in Godot but also in any other application.

@radiantgurl
Copy link
Contributor

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine - Radiant Fork v4.3.1.rc.radiant_fork.transpride<33 also murder drones >:33 (26e1289ec8621b6673b255e3f2cfe4060c200d6f)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /usr/lib/libc.so.6(+0x3d1d0) [0x71eeb4d4a1d0] (??:0)
[2] EditorData::get_script_icon(Ref<Script> const&) (/home/radiant/godot/./editor/editor_data.cpp:1169)
[3] ScriptEditor::_draw_script_list() (/home/radiant/godot/./editor/plugins/script_editor_plugin.cpp:3301)
[4] /home/radiant/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm(_Z29call_with_variant_args_helperI12ScriptEditorJETpTnmJEEvPT_MS1_FvDpT0_EPPK7VariantRN8Callable9CallErrorE13IndexSequenceIJXspT1_EEE+0x89) [0x584c271578e9] (/home/radiant/godot/./core/variant/binder_common.h:309)
[5] void call_with_variant_args<ScriptEditor>(ScriptEditor*, void (ScriptEditor::*)(), Variant const**, int, Callable::CallError&) (/home/radiant/godot/./core/variant/binder_common.h:419)
[6] CallableCustomMethodPointer<ScriptEditor>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/radiant/godot/./core/object/callable_method_pointer.h:104)
[7] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/home/radiant/godot/./core/variant/callable.cpp:58)
[8] Object::emit_signalp(StringName const&, Variant const**, int) (/home/radiant/godot/./core/object/object.cpp:1187)
[9] Node::emit_signalp(StringName const&, Variant const**, int) (/home/radiant/godot/./scene/main/node.cpp:3903)
[10] Error Object::emit_signal<>(StringName const&) (/home/radiant/godot/./core/object/object.h:938)
[11] CanvasItem::_redraw_callback() (/home/radiant/godot/./scene/main/canvas_item.cpp:144)
[12] /home/radiant/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm(_Z29call_with_variant_args_helperI10CanvasItemJETpTnmJEEvPT_MS1_FvDpT0_EPPK7VariantRN8Callable9CallErrorE13IndexSequenceIJXspT1_EEE+0x89) [0x584c264fe8c9] (/home/radiant/godot/./core/variant/binder_common.h:309)
[13] void call_with_variant_args<CanvasItem>(CanvasItem*, void (CanvasItem::*)(), Variant const**, int, Callable::CallError&) (/home/radiant/godot/./core/variant/binder_common.h:419)
[14] CallableCustomMethodPointer<CanvasItem>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/radiant/godot/./core/object/callable_method_pointer.h:104)
[15] Callable::callp(Variant const**, int, Variant&, Callable::CallError&) const (/home/radiant/godot/./core/variant/callable.cpp:58)
[16] CallQueue::_call_function(Callable const&, Variant const*, int, bool) (/home/radiant/godot/./core/object/message_queue.cpp:221)
[17] CallQueue::flush() (/home/radiant/godot/./core/object/message_queue.cpp:270)
[18] SceneTree::physics_process(double) (/home/radiant/godot/./scene/main/scene_tree.cpp:492)
[19] Main::iteration() (/home/radiant/godot/main/main.cpp:4070)
[20] OS_LinuxBSD::run() (/home/radiant/godot/platform/linuxbsd/os_linuxbsd.cpp:962)
[21] /home/radiant/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm(main+0x1bf) [0x584c24c339cf] (/home/radiant/godot/platform/linuxbsd/godot_linuxbsd.cpp:86)
[22] /usr/lib/libc.so.6(+0x25e08) [0x71eeb4d32e08] (??:0)
[23] /usr/lib/libc.so.6(__libc_start_main+0x8c) [0x71eeb4d32ecc] (??:0)
[24] /home/radiant/godot/bin/godot.linuxbsd.editor.dev.x86_64.llvm(_start+0x25) [0x584c24c33735] (??:?)
-- END OF BACKTRACE --
================================================================

@aXu-AP
Copy link
Contributor Author

aXu-AP commented Sep 24, 2024

handle_crash: Program crashed with signal 11

Interesting. I'll look into it if I'll continue work in this branch... Consensus seems to be that this approach ended up visually too noisy, but alternatives aren't yet agreed on either.

@aXu-AP aXu-AP marked this pull request as draft September 24, 2024 08:18
@radiantgurl
Copy link
Contributor

Here is the workflow that created the build.
https://github.com/RadiantUwU/godot/actions/runs/10887097884

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.

5 participants