Skip to content

Conversation

DexterFstone
Copy link

@DexterFstone DexterFstone commented Aug 6, 2025

Closes godotengine/godot-proposals#12945

2025-09-15.02-51-09.mp4

Tasks

  • Add ScenePaint tool
  • Add ScenePaintEditor
  • Add tool switch logic
  • Add basic toolbar
  • Add grid
  • Add grid highlight
  • Implement Folder, Add Folder, Remove Folder
  • Implement Add Scene, Open Scene, Remove Scene
  • Implement FreePainting, GridPainting, Erasing
  • Add scene preview
  • Allow to change scene properties from the inspector before painting it
  • Implement Picker tool
  • Make grid step unique for each Folder and Scene
  • Implement New ScenePalette, Save ScenePalette, Load ScenePalette
  • Implement Undo/Redo

@AThousandShips AThousandShips added this to the 4.x milestone Aug 6, 2025
@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch 3 times, most recently from 53d7c0a to d5f2afc Compare August 9, 2025 16:22
@Lazy-Rabbit-2001
Copy link
Contributor

Lazy-Rabbit-2001 commented Aug 10, 2025

Will it be allowed to create folders in the Scene Painter so that we can manage the panel and keep it tidy and clean and logical?

@DexterFstone
Copy link
Author

Will it be allowed to create folders in the Scene Painter so that we can manage the panel and keep it tidy and clean and logical?

This wasn't in my plan, but I'll add it.

@DexterFstone
Copy link
Author

DexterFstone commented Aug 10, 2025

Should the grid step be included in the folders so that each folder can have its own grid step or a single grid step setup?

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from d5f2afc to f07efdd Compare August 10, 2025 12:50
@Lazy-Rabbit-2001
Copy link
Contributor

Lazy-Rabbit-2001 commented Aug 10, 2025

Should the grid step be included in the folders so that each folder can have its own grid step or a single grid step setup?

I think it's better to set it for each scene that are listed in the painter

@jcostello
Copy link
Contributor

What about 3D?

@DexterFstone
Copy link
Author

What about 3D?

It is currently being developed for 2D, it remains to be seen whether it has the desired result or not, later a new proposal for 3D should be presented, the way this is implemented is only for 2D and for 3D something new needs to be developed.

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from f07efdd to a656919 Compare August 15, 2025 07:29
@DexterFstone
Copy link
Author

Hi @groud, could you check it so far? I would appreciate your feedback.

@Lazy-Rabbit-2001
Copy link
Contributor

I think you can add a preview that follows the cursor so as to help users to accurately know how and where the scene will be painted.

@arkology
Copy link
Contributor

I'm not strongly against this feature (with all respect to DexterFstone work, really) but...
At the moment don't see any reason why it should exist in core and not as addon. Such fancy and nice to have stuff is 100 % addons territory.

@groud
Copy link
Member

groud commented Aug 15, 2025

Hi @groud, could you check it so far? I would appreciate your feedback.

Oh sorry, I didn't check that earlier. I didn't expect you would do it that fast!

I had a thought about the feature btw. I am thinking that, instead of using a dedicated panel at the bottom, why not reuse the filesystem dock ? One simple solution would be to check when the selection changes then, if a scene is selected, use it as the scene to paint. That would simplify things a lot IMO, as you would not have to manage yet another set of scenes in the bottom panel, save the list, etc... (we could still keep the controls in the bottom panel though, I think that's fine). Also, if go that route, maybe we can add a lock next to "selected scene" field notifying which scene is being painted. And if you toggle the lock on, selecting a scene won't change the painted scene anymore (in case you have something to do in the filesystem dock at the same time)

Aside from that it looks pretty nice from the videos already. Good job!

I'm not strongly against this feature (with all respect to DexterFstone work, really) but... At the moment don't see any reason why it should exist in core and not as addon. Such fancy and nice to have stuff is 100 % addons territory.

No, instantiating many scenes in a 2D world is a hassle. Like, if you want to paint an organic forest such a tool is a real blessing. So it being core makes quite a lot of sense to me, especially if we manage to add it without too much code.

But that being said, the primary reason I want this feature is that people keep asking about being able to modify the properties of scenes instantiated by a TileMapLayer, which is not possible without a significant amount of code in an area that's already bloated, and with a performance cost that could end up hidden to the users. Instead, this solution simply brings the quite nice usability of TileMap to scene instantiation. It's simpler, less code, and a lot easier to maintain, while leveraging all the capabilities of the inspector, 2D editor and so on.

@arkology
Copy link
Contributor

arkology commented Aug 15, 2025

No, instantiating many scenes in a 2D world is a hassle. Like, if you want to paint an organic forest such a tool is a real blessing. So it being core makes quite a lot of sense to me, especially if we manage to add it without too much code.

That's why plugins exist. https://github.com/dalexeev/godot-node-brush-plugin for example. Related proposal - godotengine/godot-proposals#5553.

But that being said, the primary reason I want this feature is that people keep asking about being able to modify the properties of scenes instantiated by a TileMapLayer, which is not possible without a significant amount of code in an area that's already bloated, and with a performance cost that could end up hidden to the users. Instead, this solution simply brings the quite nice usability of TileMap to scene instantiation. It's simpler, less code, and a lot easier to maintain, while leveraging all the capabilities of the inspector, 2D editor and so on.

Unfortunately it sounds like even more bloat - "we have problem X in entity Y, so let's add new entity Z with the same functionality but with some changes".

But again, I'm not insisting on anything and I respect the work DexterFstone has done (what he has implemented definitely looks great).
I just said what I thought, and I'm not going to insist on it any further.

@groud
Copy link
Member

groud commented Aug 15, 2025

Unfortunately it sounds like even more bloat - "we have problem X in entity Y, so let's add new entity Z with the same functionality but with some changes".

No, that's quite the opposite. If problem X is better and more simply solved by a new, different solution Z, that's perfectly fine. Trying to make a generic solution that fixes everything is usually bad, as it makes code more difficult to maintain. See https://docs.godotengine.org/en/stable/contributing/development/best_practices_for_engine_contributors.html#to-each-problem-its-own-solution

But to make it clear, the goal of the TileMap and scene instantiation are quite different. TileMap aims to quite performance-focused, scene instantiation is more about flexibility. It's a bit like the MultiMeshInstance node for example.

@DexterFstone
Copy link
Author

I had a thought about the feature btw. I am thinking that, instead of using a dedicated panel at the bottom, why not reuse the filesystem dock ? One simple solution would be to check when the selection changes then, if a scene is selected, use it as the scene to paint. That would simplify things a lot IMO, as you would not have to manage yet another set of scenes in the bottom panel, save the list, etc... (we could still keep the controls in the bottom panel though, I think that's fine). Also, if go that route, maybe we can add a lock next to "selected scene" field notifying which scene is being painted. And if you toggle the lock on, selecting a scene won't change the painted scene anymore (in case you have something to do in the filesystem dock at the same time)

One solution I can think of is the Picker tool, which allows you to pick a scene from FileSystem, SceneTree, or CanvasItem without the need to add the scene to the ScenePalette

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from a656919 to 4b6edcb Compare August 17, 2025 12:41
@DexterFstone
Copy link
Author

Scene paint preview and edit scene properties are added

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch 2 times, most recently from b879bd7 to 06d8546 Compare August 28, 2025 07:17
@DexterFstone DexterFstone marked this pull request as ready for review August 28, 2025 07:17
@DexterFstone DexterFstone requested a review from a team as a code owner August 28, 2025 07:17
@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from 06d8546 to 95fe8e3 Compare August 28, 2025 08:01
@KoBeWi
Copy link
Member

KoBeWi commented Aug 28, 2025

So I gave this a test. Aside from the missing undo/redo:

  • The palette needs an explicit default folder. Right now the default folder has no item in the list, so you need to deselect everything to see it. It's non-intuitive and won't work once you have many folders (deselecting will be difficult).
  • Grid snapping does not work.
  • Picking needs to be improved. To pick a node from viewport, you need to click it's position exactly. It should work like selecting instances. Also, maybe I misunderstand the purpose of this tool, but I wouldn't expect it to add the picked scene to the palette. At least not from the viewport/scene tree (why is picking from FileSystem even a thing?).
  • Too many Resources. Palette folder does not need to be a Resource, and SceneData also doesn't need to either. You can serialize them similar to TileData in TileSet.
  • All actions in ScenePaint panel are missing shortcuts. Especially pressing Delete will delete the currently selected node in scene.
  • Why is grid snapping stored per scene?
  • There is no way to open a scene palette other than clicking the Open button. If you create new palette by accident (which is surprisingly easy), you can't just open the palette file from FileSystem.
  • The ScenePaint editor should utilize save_external_data() to save the palette. Although adding undo/redo might make this unnecessary.
  • It could be useful to pin a parent node for painting. It's easy to select different node by accident and add nodes to wrong parent.
  • Picking a node from viewport/scene should probably automatically copy it's properties. Right now there is no easy way to create "presets" of scenes, e.g. spikes rotated to different angles. You have to set it every time you select the scene.
  • When scene from palette is edited in the inspector, it's treated like foreign node. If you e.g. add a shader, you can't edit it. Though that might be unrelated editor bug.

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from 8ad2d65 to 2920882 Compare September 19, 2025 08:39
@KoBeWi
Copy link
Member

KoBeWi commented Sep 19, 2025

Doesn't seem fixed:

gz6cNhi8LK.mp4

I can't see nor paint any scenes, though I can delete them.

@DexterFstone
Copy link
Author

Doesn't seem fixed:

gz6cNhi8LK.mp4
I can't see nor paint any scenes, though I can delete them.

What is the steps to reproduce? I'm asking because I have no issues

@KoBeWi
Copy link
Member

KoBeWi commented Sep 19, 2025

More like what are the steps to paint?
It's the project I attached above, I select Paint tool, select scene, select 2D node and try to paint anything. Do I need to do something specific?

EDIT:
Looks like I need to use the Pick button first.

@DexterFstone
Copy link
Author

More like what are the steps to paint? It's the project I attached above, I select Paint tool, select scene, select 2D node and try to paint anything. Do I need to do something specific?

EDIT: Looks like I need to use the Pick button first.

Yes, first press i to select picker tool, then select a scene, then paint

@KoBeWi

This comment was marked as resolved.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 19, 2025

Patch that adds undo/redo:
0001-Add-scene-painting-undo-redo.patch

@DexterFstone
Copy link
Author

  • Grid snap is wrongly enabled when you disable it, but show the grid.

Let me explain how the grid system works. First, if none of the options are enabled, the painting is free. When toggling the grid, it works like a tile map layer, and when snap to grid is enabled, it works like a canvas item snapping

@DexterFstone
Copy link
Author

I switched from Select to Paint tool while TileMapLayer was selected, and had no scene picked.

I tested it, but nothing happened

@KoBeWi
Copy link
Member

KoBeWi commented Sep 20, 2025

I tested it again and it seems to happen randomly.

@DexterFstone
Copy link
Author

I tested it again, and it seems to happen randomly.

The error occurs when selected_scene is not valid, but there is an if to check for it, so I don't know how the program bypasses it.

		if (selected_scene && !instance) {
			instance = Object::cast_to<Node2D>(selected_scene->duplicate());

@DexterFstone
Copy link
Author

  • The pin option is overkill. It forcefully selects the pinned node and you can't select anything else. I think it should just mark the node internally and use it as parent even if you selected something else. Ideally there should also be an indicator for pinned node, but that might be a bit involved to add, so doesn't need to be done now.

How should the behavior work when picking a scene from SceneTree? Currently, after the scene is picked, the selection changes to the previous node

@KoBeWi
Copy link
Member

KoBeWi commented Sep 21, 2025

How should the behavior work when picking a scene from SceneTree? Currently, after the scene is picked, the selection changes to the previous node

That behavior is fine.

Did you change your point_grab_radius in editor settings?

Ah, I forgot I was testing something with grab radius and had it changed to some big value. Although if grid snap is enabled, I'd expect the deletion to respect grid instead and ignore that radius.

@DexterFstone
Copy link
Author

Ah, I forgot I was testing something with grab radius and had it changed to some big value. Although if grid snap is enabled, I'd expect the deletion to respect the grid instead and ignore that radius.

I changed the functionality, now I use find_canvas_items_at_pos for free painting and find_canvas_items_in_rect when grid or snapping is enabled.

@DexterFstone
Copy link
Author

Patch that adds undo/redo: 0001-Add-scene-painting-undo-redo.patch

Is this the best way to implement, or should I develop my own?

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from 2920882 to 785f486 Compare September 21, 2025 11:26
@DexterFstone
Copy link
Author

#109360 (comment)

Done

@KoBeWi
Copy link
Member

KoBeWi commented Sep 21, 2025

Is this the best way to implement, or should I develop my own?

The hack should be done differently, but otherwise that's how it should be implemented.

@DexterFstone
Copy link
Author

Is this the best way to implement, or should I develop my own?

The hack should be done differently, but otherwise that's how it should be implemented.

I implemented something like this:
undo_redo.patch
Everything works fine until erasing happens, then undo, then painting, and an error appears.

>	godot.windows.editor.dev.x86_64.exe!SceneTreeEditor::_get_node_configuration_warnings(Node * p_node) Line 60	C++
 	godot.windows.editor.dev.x86_64.exe!SceneTreeEditor::_update_node(Node * p_node, TreeItem * p_item, bool p_part_of_subscene) Line 484	C++
 	godot.windows.editor.dev.x86_64.exe!SceneTreeEditor::_update_node_subtree(Node * p_node, TreeItem * p_parent, bool p_force) Line 344	C++
 	godot.windows.editor.dev.x86_64.exe!SceneTreeEditor::_update_tree(bool p_scroll_to_selected) Line 955	C++
 	godot.windows.editor.dev.x86_64.exe!call_with_variant_args_helper<SceneTreeEditor,bool,0>(SceneTreeEditor * p_instance, void(SceneTreeEditor::*)(bool) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0> __formal) Line 223	C++
 	godot.windows.editor.dev.x86_64.exe!call_with_variant_args<SceneTreeEditor,bool>(SceneTreeEditor * p_instance, void(SceneTreeEditor::*)(bool) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 337	C++
 	godot.windows.editor.dev.x86_64.exe!CallableCustomMethodPointer<SceneTreeEditor,void,bool>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 103	C++
 	godot.windows.editor.dev.x86_64.exe!Callable::callp(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 57	C++
 	godot.windows.editor.dev.x86_64.exe!CallQueue::_call_function(const Callable & p_callable, const Variant * p_args, int p_argcount, bool p_show_error) Line 220	C++
 	godot.windows.editor.dev.x86_64.exe!CallQueue::flush() Line 268	C++
 	godot.windows.editor.dev.x86_64.exe!SceneTree::physics_process(double p_time) Line 638	C++
 	godot.windows.editor.dev.x86_64.exe!Main::iteration() Line 4719	C++
 	godot.windows.editor.dev.x86_64.exe!OS_Windows::run() Line 2252	C++
 	godot.windows.editor.dev.x86_64.exe!widechar_main(int argc, wchar_t * * argv) Line 96	C++
 	godot.windows.editor.dev.x86_64.exe!_main() Line 122	C++
 	godot.windows.editor.dev.x86_64.exe!main(int argc, char * * argv) Line 136	C++
 	godot.windows.editor.dev.x86_64.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 150	C++
 	[Inline Frame] godot.windows.editor.dev.x86_64.exe!invoke_main() Line 102	C++
 	godot.windows.editor.dev.x86_64.exe!__scrt_common_main_seh() Line 288	C++
 	godot.windows.editor.dev.x86_64.exe!ShimMainCRTStartup(...) Line 48	C
 	kernel32.dll!00007ffa3cdb7344()	Unknown
 	ntdll.dll!00007ffa3e8e26b1()	Unknown

@KoBeWi
Copy link
Member

KoBeWi commented Sep 22, 2025

Use add_do/undo_reference() only for nodes that are created/deleted as part of the action.

@KoBeWi
Copy link
Member

KoBeWi commented Sep 22, 2025

Another crash xd

CrashHandlerException: Program crashed
Engine version: Godot Engine v4.6.dev.custom_build (da5883b7e2e67d616cac690a62bb00e239243675)
Dumping the backtrace. Please include this when reporting the bug on: https://github.com/godotengine/godot/issues
[0] Node::is_accessible_from_caller_thread (C:\godot_source\scene\main\node.h:651)
[1] Node2D::set_transform (C:\godot_source\scene\2d\node_2d.cpp:373)
[2] ScenePaintEditor::_update_preview (C:\godot_source\editor\scene\scene_paint_editor_plugin.cpp:183)
[3] ScenePaintEditor::_update_node (C:\godot_source\editor\scene\scene_paint_editor_plugin.cpp:141)
[4] ScenePaintEditor::_edit (C:\godot_source\editor\scene\scene_paint_editor_plugin.cpp:66)
[5] ScenePaintEditorPlugin::edit (C:\godot_source\editor\scene\scene_paint_editor_plugin.cpp:702)
[6] EditorNode::edit_item (C:\godot_source\editor\editor_node.cpp:2594)
[7] EditorNode::_edit_current (C:\godot_source\editor\editor_node.cpp:2947)
[8] EditorNode::push_item (C:\godot_source\editor\editor_node.cpp:2662)
[9] EditorNode::push_node_item (C:\godot_source\editor\editor_node.cpp:2648)
[10] SceneTreeDock::_push_item (C:\godot_source\editor\docks\scene_tree_dock.cpp:1858)
[11] SceneTreeDock::input (C:\godot_source\editor\docks\scene_tree_dock.cpp:153)
[12] Node::_call_input (C:\godot_source\scene\main\node.cpp:3551)
[13] SceneTree::_call_input_pause (C:\godot_source\scene\main\scene_tree.cpp:1449)
[14] Viewport::push_input (C:\godot_source\scene\main\viewport.cpp:3502)
[15] Window::_window_input (C:\godot_source\scene\main\window.cpp:1834)
[16] call_with_variant_args_helper<Window,Ref<InputEvent> const &,0> (C:\godot_source\core\variant\binder_common.h:223)
[17] call_with_variant_args<Window,Ref<InputEvent> const &> (C:\godot_source\core\variant\binder_common.h:337)
[18] CallableCustomMethodPointer<Window,void,Ref<InputEvent> const &>::call (C:\godot_source\core\object\callable_method_pointer.h:103)
[19] Callable::callp (C:\godot_source\core\variant\callable.cpp:57)
[20] Callable::call<Ref<InputEvent> > (C:\godot_source\core\variant\variant.h:938)
[21] DisplayServerWindows::_dispatch_input_event (C:\godot_source\platform\windows\display_server_windows.cpp:4459)
[22] DisplayServerWindows::_dispatch_input_events (C:\godot_source\platform\windows\display_server_windows.cpp:4429)
[23] Input::_parse_input_event_impl (C:\godot_source\core\input\input.cpp:979)
[24] Input::flush_buffered_events (C:\godot_source\core\input\input.cpp:1262)
[25] DisplayServerWindows::process_events (C:\godot_source\platform\windows\display_server_windows.cpp:3820)
[26] OS_Windows::run (C:\godot_source\platform\windows\os_windows.cpp:2331)
[27] widechar_main (C:\godot_source\platform\windows\godot_windows.cpp:96)
[28] _main (C:\godot_source\platform\windows\godot_windows.cpp:122)
[29] main (C:\godot_source\platform\windows\godot_windows.cpp:136)
[30] WinMain (C:\godot_source\platform\windows\godot_windows.cpp:150)
[31] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[32] ShimMainCRTStartup (C:\godot_source\platform\windows\cpu_feature_validation.c:74)
[33] <couldn't map PC to fn name>
-- END OF C++ BACKTRACE --

Happens when reloading the scene while painting. I didn't check, but it might involve a freed node.

Also something is wrong with the grid.

godot.windows.editor.dev.x86_64_OEWiiOBq6H.mp4

Show Grid still snaps, while Snap to Grid makes things snap at intersection of grid lines?? That's weird, but might be useful (depending on how you setup the scenes), but then the options need to be renamed. Although instead of this weird behavior, there could be grid offset. I think this can be left for later.

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from 785f486 to 9279605 Compare September 23, 2025 05:21
@DexterFstone
Copy link
Author

Show Grid still snaps, while Snap to Grid makes things snap at intersection of grid lines??

yes

@DexterFstone
Copy link
Author

DexterFstone commented Sep 23, 2025

Here, another bug :)
I was painting and switched the scene

2025-09-22.22-40-25.mp4

Edit: I tested again, but it didn't happen :)

@arkology
Copy link
Contributor

As I see it, there are separate grid settings for the painter tool, in addition to the existing grid settings. Why not use the existing grid settings? Or is it inconvenient?

@DexterFstone
Copy link
Author

but then the options need to be renamed.

What is your suggestion?

@DexterFstone DexterFstone force-pushed the add-a-scene-painter-tool branch from 9279605 to b135914 Compare September 23, 2025 06:08
@DexterFstone
Copy link
Author

Happens when reloading the scene while painting. I didn't check, but it might involve a freed node.

I couldn't reproduce that, but I added a call_deferred. I guess that the problem is that the function is called too fast

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.

Add a scene painter tool
7 participants