-
Notifications
You must be signed in to change notification settings - Fork 127
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 terrain_edited signal to Terrain3DEditor #274
Conversation
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.
Why does this class need to be a singleton in order to fire a signal? Can't normal classes do that?
So that tool scripts and other addon scripts besides the Terrain3D addon can access it. Currently Terrain3DEditor is created and the reference held by the EditorPlugin, but other scripts can't access this. |
I've updated my branch to address your other comments. |
Thanks! |
@@ -123,6 +153,10 @@ void Terrain3DEditor::_operate_map(Vector3 p_global_position, real_t p_camera_di | |||
} | |||
Object::cast_to<Node>(_terrain->get_plugin()->get("ui"))->call("set_decal_rotation", rot); | |||
|
|||
AABB edited_area; | |||
edited_area.position = p_global_position - Vector3(brush_size, 0.0, brush_size) / 2; |
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.
Shouldn't 2 be float just to avoid unnecessary conversions? I'm changing this in the new PR
As discussed in #270.
Terrain3DEditor becomes a singleton, and gains a new
terrain_edited(edited_area: AABB)
signal. The use case is integration with other level design tools and scripts - see the linked issue.The signal is emitted whenever:
In the end I decided not to include the map_type parameter in the signal, as it seems like this would not be relevant to non-brush-related tools (region add/remove, etc.).