Skip to content

Conversation

@wjt
Copy link
Member

@wjt wjt commented Sep 14, 2025

It has often annoyed me that the "Refill" action cannot be undone with Ctrl-Z.

Integrate with EditorUndoRedoManager to make this possible.

In addition, change how the child nodes are instantiated so they are persisted to the scene in just the same way as if they were instantiated by hand, fixing an issue where loading and saving the scene later, without changes, would change the .tscn.

wjt added 4 commits September 14, 2025 11:45
AreaFiller has no function at game runtime. queue_free() it when it
becomes ready if not running in the editor.
Previously, it was not possible to undo the "Refill" action, other than
with `git restore` or similar.

Use EditorUndoRedoManager to add the refill action to the editor's
undo/redo stack. This is intended to be used from EditorPlugins, which
this script is not, but since the EditorUndoRedoManager is actually a
singleton we can create a temporary EditorPlugin instance, get the
EditorUndoRedoManager from it, and free it.

Because the EditorPlugin and EditorUndoRedoManager classes are only
available in the editor, we cannot use their static types, and construct
the EditorPlugin object by name.

Special care is taken to make sure that children are removed then added
in that order, both for undo and redo. This is to give the nodes
sequentially-numbered names. Strictly speaking we are relying on the
editor generating the same names when undoing as the nodes had before
they were removed by doing the action; but this is very likely to be the
case, so this is likely to be good enough.
PackedScene.instantiate() accepts one argument, from the GenEditState
enum. The default value is GEN_EDIT_STATE_DISABLED = 0:

> If passed to instantiate(), blocks edits to the scene state.

In this mode, when the parent scene is saved, the instanced node has a "type"
attribute and a "script" property:

    [node name="Flower" type="Node2D" parent="OnTheGround/Flowerbed" instance=ExtResource("10_cbbm0")]
    position = Vector2(816.928, 202.694)
    script = ExtResource("9_cbbm0")

If, in the editor, you later reopen the scene and save it without changes, these
properties disappear:

    [node name="Flower" parent="OnTheGround/Flowerbed" instance=ExtResource("10_cbbm0")]
    position = Vector2(816.928, 202.694)

The latter serialisation is the one that you get if you instantiate a scene
using the editor UI, rather than in this @tool script.

Another member of the GenEditState enum is GEN_EDIT_STATE_INSTANCE = 1:

> If passed to instantiate(), provides local scene resources to the local scene.

This isn't very well documented but it turns out this causes the scene instanced
in code to be persisted in the second way, as if it had been instanced from the
editor UI.

Pass this flag.
This is the result of opening the scene in the editor, and saving it
without changes.

The difference in serialisation is due to how AreaFiller was
instantiating the scenes it fills the area with. This will be fixed in a
future commit.
@wjt wjt requested a review from a team as a code owner September 14, 2025 10:55
@wjt wjt marked this pull request as draft September 14, 2025 10:55
@github-actions
Copy link

Play this branch at https://endlessm.github.io/threadbare/branches/endlessm/area-filler-undo.

(This launches the game from the start, not directly at the change(s) in this pull request.)

@wjt wjt marked this pull request as ready for review September 14, 2025 16:46
func _prepare_child(pos: Vector2) -> Node2D:
var child: Node2D = scenes.pick_random().instantiate()
var scene: PackedScene = scenes.pick_random()
var child: Node2D = scene.instantiate(PackedScene.GenEditState.GEN_EDIT_STATE_INSTANCE)
Copy link
Member Author

Choose a reason for hiding this comment

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

I wrote https://willthompson.co.uk/notes/instancing-scenes-in-tool-script/ so I can remember this next time I need it.

I learnt how to do this from godotengine/godot-docs#8801

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is news to me! Thanks for the blog post.

Copy link
Collaborator

@manuq manuq left a comment

Choose a reason for hiding this comment

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

Excellent!

@manuq manuq merged commit eceda30 into main Sep 15, 2025
8 checks passed
@manuq manuq deleted the area-filler-undo branch September 15, 2025 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants