-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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 PluginConfigDialog. #19837
Add PluginConfigDialog. #19837
Conversation
Nice work atleast you didn't give up I know I would have. |
As I mentioned before, most of those are solutions in search of problems, or things that need a lot more discussion. We can leave this open and eventually discuss this stuff when time comes |
@reduz, well, true, in their current state, the majority of these just give answers to a now nonexistent set of problems (most deal with problems I encountered while building my own changes). The ones that are more usability-focused that solve real issues are...
Everything else most certainly addresses no-longer-existing problems. |
cef8f96
to
4237cd3
Compare
If people like the PluginCreateDialog idea, I can move it into a different PR to be merged while the others are in limbo / discussed. Or if you just wanna wait on them all, that's fine too. I'll be around, so let me know. 👍 (I trimmed the PR so only the significant stuff remains for discussion) |
a54afc0
to
8a8fd94
Compare
I like the |
Should I just extract the |
editor/editor_node.cpp
Outdated
@@ -5032,6 +5039,7 @@ EditorNode::EditorNode() { | |||
p->add_child(tool_menu); | |||
p->add_submenu_item(TTR("Tools"), "Tools"); | |||
tool_menu->add_item(TTR("Orphan Resource Explorer"), TOOLS_ORPHAN_RESOURCES); | |||
tool_menu->add_item(TTR("Rebuild Documentation"), TOOLS_RELOAD_DOCS); |
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 don't see the point of this feature.
Rebuilding documentation is only relevant when you made code changes and compiled them, and since that usually happens on the command line, it's much easier to run --doctool
directly.
It's even more wrong if you're using the official Godot binary, as it's not in bin/
next to the doc/
directory as your feature hardcodes it, so it will just dump a full documentation tree in an unexpected location in ../doc
.
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.
Well, the idea (at the time) was to be able to have the doctool generate content for XML files in your project folder / plugin folders too. Combined with the ability to generate XML files from a Script, this would allow you to create in-engine documentation for the scripts in your project. And in a case like that, being able to easily rebuild stuff from the editor would be useful.
However, it's a fair point that all of that is predicated on features that aren't even implemented yet (script-based XML generation, generating docs from local files rather than just the godot repo).
I can go ahead and remove it from this PR.
editor/plugin_create_dialog.h
Outdated
#ifndef PLUGIN_CREATE_DIALOG_H | ||
#define PLUGIN_CREATE_DIALOG_H | ||
|
||
#include "../scene/gui/check_box.h" |
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.
Please don't use relative paths, the root folder is already in the include path so you can do #include "scene/gui/check_box.h"
.
modules/gdscript/gdscript.cpp
Outdated
if (E) { | ||
|
||
if (!E->get()->is_static()) { | ||
WARN_PRINT(String("Can't call non-static function: 'can_instance' in script.").utf8().get_data()); |
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 FYI, you could use WARN_PRINTS("Can't call non-static function: 'can_instance' in script.");
directly.
05664a7
to
52e6c83
Compare
52e6c83
to
ff60441
Compare
Since I never mentioned it, this PR has been redesigned so that it's just the PluginConfigDialog changes. Should be good to go. |
Thanks! |
Here are a host of small changes that were all part of #19778. Someone please let me know which parts of this people wanna discard, and of those remaining, which ones I should merge into single commits (or if I should just roll all of these commits up into one, etc.).