Skip to content

Expose ScenePaint2DEditor#119052

Open
DexterFstone wants to merge 1 commit intogodotengine:masterfrom
DexterFstone:Expose-ScenePaint2DEditor
Open

Expose ScenePaint2DEditor#119052
DexterFstone wants to merge 1 commit intogodotengine:masterfrom
DexterFstone:Expose-ScenePaint2DEditor

Conversation

@DexterFstone
Copy link
Copy Markdown
Contributor

@DexterFstone DexterFstone commented Apr 28, 2026

closes godotengine/godot-proposals#14730

2026-04-28.01-36-50.mp4

Methods:

EditorInterface.get_scene_paint_2d()

get_painted_scene()
set_painted_scene()
unregister_scene_provider()
register_scene_provider()

## Note:
The documentation is written by AI, my native language is not English, so I appreciate any improvements.

@DexterFstone DexterFstone requested review from a team as code owners April 28, 2026 17:47
@DexterFstone DexterFstone force-pushed the Expose-ScenePaint2DEditor branch from 26bca26 to 41ebfe1 Compare April 28, 2026 18:25
@AThousandShips
Copy link
Copy Markdown
Member

The documentation is written by AI, my native language is not English, so I appreciate any improvements

Please do not use AI to write documentation, use translation software instead

Copy link
Copy Markdown
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Left some comments on the API.

I think it's fine overall, but I don't get what's the paint mode exposed for.

Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
Comment thread doc/classes/ScenePaint2DEditor.xml
Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
@Jordyfel
Copy link
Copy Markdown
Contributor

The way it is exposed conflicts with #109746. Which way should be preferred?

@DexterFstone DexterFstone force-pushed the Expose-ScenePaint2DEditor branch from 4bc647d to fb431dd Compare April 29, 2026 16:16
Comment thread doc/classes/ScenePaint2DEditor.xml Outdated
@DexterFstone DexterFstone force-pushed the Expose-ScenePaint2DEditor branch from fb431dd to f23b063 Compare April 29, 2026 17:50
@DexterFstone DexterFstone requested a review from KoBeWi April 30, 2026 06:49
Class for interacting with the 2D editor's Scene Paint Mode.
</brief_description>
<description>
This class is available only in editor and cannot be instantiated. Access it using [method EditorInterface.get_scene_paint_2d].
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
This class is available only in editor and cannot be instantiated. Access it using [method EditorInterface.get_scene_paint_2d].
This class is available only in the editor and cannot be instantiated. Access it using [method EditorInterface.get_scene_paint_2d].

Copy link
Copy Markdown
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Some minor code problems left, but otherwise looks fine.

The way it is exposed conflicts with #109746. Which way should be preferred?

Well, there are 2 problems with this:

  • That PR is not merged yet
  • ScenePaint2DEditor is not a plugin (the exposed methods would need to be moved)

The reason I opened that PR was because someone exposed a plugin with no way to access it properly. Unless I missed it, no one even suggested using EditorInterface for that 😅

Then again, all classes accessible via EditorInterface are core editor components, not plugins (with the exception of ScriptEditor, which has a parent plugin). So we don't really have established way for exposing plugins. Using EditorInterface is better for discoverability at least.

<method name="get_painted_scene" qualifiers="const">
<return type="Node2D" />
<description>
Returns the painted scene, or [code]null[/code] if none is selected.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could also mention that it's possible to customize the scene's properties.

Comment on lines +512 to +513
if (!ResourceLoader::exists(p_path)) {
EditorNode::get_singleton()->show_accept(TTR("The selected scene could not be found. It may have been moved or deleted."), TTR("OK"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if (!ResourceLoader::exists(p_path)) {
EditorNode::get_singleton()->show_accept(TTR("The selected scene could not be found. It may have been moved or deleted."), TTR("OK"));
if (!ResourceLoader::exists(p_path, "PackedScene")) {
EditorNode::get_singleton()->show_warning(TTR("The selected scene could not be found. It may have been moved or deleted."));

Comment on lines +522 to +534
Ref<SceneState> scene_state = scene->get_state();
String type;
while (scene_state.is_valid() && type.is_empty()) {
ERR_FAIL_COND_V(scene_state->get_node_count() < 1, nullptr);
type = scene_state->get_node_type(0);
scene_state = scene_state->get_base_scene_state();
}
ERR_FAIL_COND_V_EDMSG(type.is_empty(), nullptr, "The selected scene is invalid.");
bool extends_current_class = ClassDB::is_parent_class(type, "Node2D");
if (scene.is_valid() && extends_current_class) {
return Object::cast_to<Node2D>(scene->instantiate());
}
return nullptr;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could be simplified to (not tested)

Suggested change
Ref<SceneState> scene_state = scene->get_state();
String type;
while (scene_state.is_valid() && type.is_empty()) {
ERR_FAIL_COND_V(scene_state->get_node_count() < 1, nullptr);
type = scene_state->get_node_type(0);
scene_state = scene_state->get_base_scene_state();
}
ERR_FAIL_COND_V_EDMSG(type.is_empty(), nullptr, "The selected scene is invalid.");
bool extends_current_class = ClassDB::is_parent_class(type, "Node2D");
if (scene.is_valid() && extends_current_class) {
return Object::cast_to<Node2D>(scene->instantiate());
}
return nullptr;
Node *node = scene->instantiate();
ERR_FAIL_NULL_V_EDMSG(node, nullptr, "The selected scene is invalid.");
Node2D *n2d = Object::cast_to<Node2D>(node);
if (!n2d) {
node->queue_free();
return null;
}
return n2d;

} break;
case PICK_CUSTOM_SOURCE: {
Callable callback = custom_sources[p_control];
Variant result = callback.call();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could use callp() for error handling. EditorInterface code has some examples.

void ScenePaint2DEditor::register_scene_provider(Control *p_control, const Callable &p_callback) {
ERR_FAIL_COND_MSG(custom_sources.has(p_control), "Provider already registered.");
custom_sources[p_control] = p_callback;
if (!p_control->is_connected(SceneStringName(gui_input), callable_mp(this, &ScenePaint2DEditor::_custom_source_input))) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pretty sure the condition is not needed. You already safeguard against repeated registering, so mutliple connections are not possible.

void ScenePaint2DEditor::unregister_scene_provider(Control *p_control) {
ERR_FAIL_COND_MSG(!custom_sources.has(p_control), "Provider not found.");
custom_sources.erase(p_control);
if (p_control->is_connected(SceneStringName(gui_input), callable_mp(this, &ScenePaint2DEditor::_custom_source_input))) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here. If the source was registered before, it's guaranteed to be connected. (user meddling with signal connections is irrelevant)

if (input_tool == INPUT_TOOL_PICK || input_tool == INPUT_TOOL_QUICK_PICK) {
if (mb->is_pressed() && mb->get_button_index() == MouseButton::LEFT) {
callable_mp(this, &ScenePaint2DEditor::_update_scene_picker).call_deferred(PICK_CANVAS_ITEM);
callable_mp(this, &ScenePaint2DEditor::_update_scene_picker).call_deferred(PICK_CANVAS_ITEM, (Control *)nullptr);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the cast needed?

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.

Allow to extend Scene Paint tool with custom scene providers

4 participants