-
-
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 @tool_button
annotation for easily creating inspector buttons.
#78355
Conversation
fc69175
to
7ecec2a
Compare
To change the order of a tool button in the inspector, are you forced to move the function up? This does not follow the GDScript style guide, which recommends that variables be placed above functions. |
The last time we discussed the syntax it was suggested that you can add the annotation wherever you want if you pass a callable "reference" to it, see #59289 (comment). |
Can you explain how this affects this pr? Like how to change? |
7ecec2a
to
514e37e
Compare
514e37e
to
8f0330a
Compare
ToolButtonEditorPlugin(); | ||
}; | ||
|
||
#endif // TOOL_BUTTON_EDITOR_PLUGIN_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.
Missing line termination
Can someone help me test this? Also, does anyone know how to call this feature from c++? |
@@ -4174,6 +4178,44 @@ bool GDScriptParser::warning_annotations(const AnnotationNode *p_annotation, Nod | |||
#endif // DEBUG_ENABLED | |||
} | |||
|
|||
bool GDScriptParser::tool_button_annotation(const AnnotationNode *p_annotation, Node *p_node) { | |||
#ifndef TOOLS_ENABLED // !TOOLS_ENABLED |
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'd flip the condition here, with both cases having the negative condition first is confusing
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
func reset_color(old_color): | ||
modulate = old_color | ||
[/codeblock] | ||
[b]Note:[/b] Make sure to provide separate `do` and `undo` methods that perform and reverse the action respectively. |
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.
[b]Note:[/b] Make sure to provide separate `do` and `undo` methods that perform and reverse the action respectively. | |
[b]Note:[/b] Make sure to provide separate [code]do[/code] and [code]undo[/code] methods that perform and reverse the action respectively. |
Also, this code sample won't work as-is because set_color
and reset_color
should be called do_method_name
and undo_method_name
for this example respectively.
/**************************************************************************/ | ||
|
||
#include "tool_button_editor_plugin.h" | ||
#include "editor/editor_node.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.
#include "editor/editor_node.h" | |
#include "editor/editor_node.h" |
virtual bool can_handle(Object *p_object) override; | ||
virtual bool parse_property(Object *p_object, const Variant::Type p_type, const String &p_path, const PropertyHint p_hint, const String &p_hint_text, const BitField<PropertyUsageFlags> p_usage, const bool p_wide = false) override; | ||
void update_action_icon(Button *p_action_button); | ||
void call_action(Object *p_object, StringName p_do_method_name, StringName p_undo_method_name); |
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.
void call_action(Object *p_object, StringName p_do_method_name, StringName p_undo_method_name); | |
void call_action(Object *p_object, const StringName &p_do_method_name, const StringName &p_undo_method_name); |
#ifndef TOOLS_ENABLED // !TOOLS_ENABLED | ||
// Only available in editor. | ||
return true; | ||
#else // TOOLS_ENABLED | ||
AnnotationNode *annotation = const_cast<AnnotationNode *>(p_annotation); | ||
FunctionNode *func = static_cast<FunctionNode *>(p_node); | ||
if (annotation->resolved_arguments.size() < 1) { | ||
push_error("Tool buttons must specify a name.", p_annotation); | ||
return false; | ||
} | ||
|
||
if (!this->is_tool()) { | ||
push_error("Tool buttons can only be used in tool scripts.", p_annotation); | ||
return false; | ||
} | ||
|
||
annotation->export_info.name = annotation->resolved_arguments[0]; | ||
annotation->export_info.type = Variant::Type::CALLABLE; | ||
annotation->export_info.usage = PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_INTERNAL; | ||
if (func->parameters.size() != 1) { | ||
push_error("Tool button methods must have an UndoRedo as their only argument.", func); | ||
return false; | ||
} | ||
if (func->parameters[0]->datatype_specifier != nullptr && func->parameters[0]->datatype_specifier->type_chain[0]->name != "UndoRedo") { | ||
push_error("Tool button methods must have an UndoRedo as their only argument.", func); | ||
return false; | ||
} | ||
String hint_string = vformat("%s,%s", annotation->resolved_arguments[0], func->identifier->name); | ||
if (annotation->resolved_arguments.size() > 1) { | ||
// Icon. | ||
hint_string += "," + annotation->resolved_arguments[1].operator String(); | ||
} | ||
annotation->export_info.hint_string = hint_string; | ||
return true; | ||
#endif // TOOLS_ENABLED |
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.
#ifndef TOOLS_ENABLED // !TOOLS_ENABLED | |
// Only available in editor. | |
return true; | |
#else // TOOLS_ENABLED | |
AnnotationNode *annotation = const_cast<AnnotationNode *>(p_annotation); | |
FunctionNode *func = static_cast<FunctionNode *>(p_node); | |
if (annotation->resolved_arguments.size() < 1) { | |
push_error("Tool buttons must specify a name.", p_annotation); | |
return false; | |
} | |
if (!this->is_tool()) { | |
push_error("Tool buttons can only be used in tool scripts.", p_annotation); | |
return false; | |
} | |
annotation->export_info.name = annotation->resolved_arguments[0]; | |
annotation->export_info.type = Variant::Type::CALLABLE; | |
annotation->export_info.usage = PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_INTERNAL; | |
if (func->parameters.size() != 1) { | |
push_error("Tool button methods must have an UndoRedo as their only argument.", func); | |
return false; | |
} | |
if (func->parameters[0]->datatype_specifier != nullptr && func->parameters[0]->datatype_specifier->type_chain[0]->name != "UndoRedo") { | |
push_error("Tool button methods must have an UndoRedo as their only argument.", func); | |
return false; | |
} | |
String hint_string = vformat("%s,%s", annotation->resolved_arguments[0], func->identifier->name); | |
if (annotation->resolved_arguments.size() > 1) { | |
// Icon. | |
hint_string += "," + annotation->resolved_arguments[1].operator String(); | |
} | |
annotation->export_info.hint_string = hint_string; | |
return true; | |
#endif // TOOLS_ENABLED | |
#ifdef TOOLS_ENABLED | |
AnnotationNode *annotation = const_cast<AnnotationNode *>(p_annotation); | |
FunctionNode *func = static_cast<FunctionNode *>(p_node); | |
if (annotation->resolved_arguments.size() < 1) { | |
push_error("Tool buttons must specify a name.", p_annotation); | |
return false; | |
} | |
if (!this->is_tool()) { | |
push_error("Tool buttons can only be used in tool scripts.", p_annotation); | |
return false; | |
} | |
annotation->export_info.name = annotation->resolved_arguments[0]; | |
annotation->export_info.type = Variant::Type::CALLABLE; | |
annotation->export_info.usage = PROPERTY_USAGE_EDITOR | PROPERTY_USAGE_INTERNAL; | |
if (func->parameters.size() != 1) { | |
push_error("Tool button methods must have an UndoRedo as their only argument.", func); | |
return false; | |
} | |
if (func->parameters[0]->datatype_specifier != nullptr && func->parameters[0]->datatype_specifier->type_chain[0]->name != "UndoRedo") { | |
push_error("Tool button methods must have an UndoRedo as their only argument.", func); | |
return false; | |
} | |
String hint_string = vformat("%s,%s", annotation->resolved_arguments[0], func->identifier->name); | |
if (annotation->resolved_arguments.size() > 1) { | |
// Icon. | |
hint_string += "," + annotation->resolved_arguments[1].operator String(); | |
} | |
annotation->export_info.hint_string = hint_string; | |
return true; | |
#else // !TOOLS_ENABLED | |
// Only available in editor. | |
return true; | |
#endif // TOOLS_ENABLED |
As per the earlier suggestion
Is this still on track to make 4.3? |
It's been abandoned by the author and needs to be salvaged, so it's waiting for someone to pick it up and continue the work |
I might be insterested in picking it up. Can you please summarize for me why the (arguably, overcomplicated) design with do/undo was chosen? Wouldn't it be much simpler and more user-friendly to give up on undo? @export var mesh_parameter: int = 42
@tool_button("Build mesh.")
func build_mesh():
# Use mesh_parameter here, no function arguments allowed This is exactly what people use the "boolean hack" for these days, only nicer and without the ugly workaround code. |
Undo/redo functionality would be used by a plugin or tool developer. |
This feature could be added without undo/redo concerns by using a confirm dialogue. |
I'm not sure to understand why plugin or tool developper can't use the regular |
You can. You're going to provide do and undo functions either way. This uses undo/redo in the background, so it's just more concise, syntactic sugar. |
Then, IMHO it's not a good idea to make "regular" user to have to provide a null undo callback as they probably would be the main users of this feature... On my addons, I'm personally not really bothered to have to use a specific UndoRedo API to handle this pretty advanced feature (and for now, I mainly used an EditorPlugin, EditorNode3DPlugin and EditorNode3DGizmoPlugin to perform undoable actions). |
I think the solution for undoredo problems is just exposing EditorUndoRedoManager via EditorInterface (which is accessible as a singleton). This way the users would be able to optionally provide undo for their action, without making button callbacks more complex. I'm already exposing it in #90130. |
I salvaged the great work here in #96290 to fit my use-cases. If anyone would like to check it out and let me know if I've missed any obvious wins, that'd be swell. |
Supersedes: #59289
Closes: godotengine/godot-proposals#2149