-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Added a simple drawing demo (GD paint) #89
Conversation
…nctions in CanvasItem to make a simple MS paint like program.
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.
Assuming this demo is desired, it will need to be rebased and updated to Godot 3.1.
Also, I noticed that there is no outline or preview of what will be drawn. Would this be possible to do?
draw_elements_list.pop_back() | ||
|
||
# Now that we've undone a stoke, we cannot undo again until another stoke is added | ||
undo_element_list_num = UNDO_NONE |
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.
Would it be possible to allow many undos?
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.
It would be possible, as I mentioned in the initial post.
It has been awhile so I don't remember exactly how the demo works, but if I recall correctly the elements drawn are stored in a list. The current undo functionality works because it stores the index of the element before drawing. When the undo button is pressed, it just removes all of the elements going forward after the stored index.
While this works for a single undo, now I can think of better ways to go about doing something like this. If there is interest and the demo is still desired, then I could see about rewriting the code to allow for multiple undos.
2d/gd_paint/ToolsPanel.gd
Outdated
if button_name == "mode_pencil": | ||
paint_control.brush_mode = "pencil" | ||
elif button_name == "mode_eraser": | ||
paint_control.brush_mode = "eraser" |
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 not use enums here?
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.
If I remember correctly, enums were not available in GDScript at the time. Enums would be better now that it is supported.
No doubt most of the code could/should be rewritten to use more modern GDScript practices. It is just a matter of finding the time/energy to rewrite the code.
is_mouse_in_drawing_area = false | ||
if mouse_pos.x > TL_node.global_position.x: | ||
if mouse_pos.y > TL_node.global_position.y: | ||
is_mouse_in_drawing_area = true |
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.
var global_pos = TL_node.global_position
is_mouse_in_drawing_area = mouse_pos.x > global_pos.x && mouse_pos.y > global_pos.y
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 say this change is debatable.
First, I feel the name global_pos
isn't very descriptive of what the variable actually contains. The TL_Node is the top left corner of the drawing area. Using the name global_pos
to me sounds like it is the global position of the node that PaintControl.gd is attached to. I don't know if defining a variable makes sense in this case, as it is only used once, but regardless I think the variable should be named something different, like topleft_global_pos
or tl_global_pos
, as that better defines what the variable stores.
Also, I'm not the biggest fan of assigning variables directly through a conditional, as it makes the code harder to read and understand (in my opinion), which is kinda opposite of the goal of a demo.
I do think that merging both of the if statements together makes sense. So the code would be the following:
is_mouse_in_drawing_area = false
if mouse_pos.x > TL_node.global_position.x and mouse_pos.y > TL_node.global_position.y:
is_mouse_in_drawing_area = true
or if using a variable for the global position:
is_mouse_in_drawing_area = false
var TL_global_pos = TL_node.global_position
if mouse_pos.x > TL_global_pos.x and mouse_pos.y > TL_global_pos.y:
is_mouse_in_drawing_area = true
…e code so it is cleaner and easier to understand. Updated comments within code according to changes
Updated this pull request so that it works with Godot 3.1. I also made some changes to make the code clearer and updated the comments accordingly. |
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.
The warnings added in Godot 3.1 need to be dealt with, but otherwise it's looking really nice.
PaintTools.png: The circle tool's icon looks like an octagon. This should be fixed. Also, consider scaling up the image, I think it would be better for the icons to be pixelated rather than blurry.
2d/gd_paint/PaintControl.gd
Outdated
set_process(true) | ||
|
||
|
||
func _process(delta): |
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.
To suppress Godot 3.1's warnings, prefix unused arguments with _
. This also applies to the _physics_process
method.
# Figure out how many elements/brushes we've added in the last stroke | ||
var elements_to_remove = brush_data_list.size() - undo_element_list_num | ||
# Remove all of the elements we've added this in the last stroke | ||
for elment_num in range(0, elements_to_remove): |
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.
Above the line with the for
loop, put #warning-ignore:unused_variable
save_dialog = get_parent().get_node("SaveFileDialog") | ||
|
||
# Assign all of the needed signals for the oppersation buttons | ||
get_node("ButtonUndo").connect("pressed", self, "button_pressed", ["undo_stroke"]) |
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.
Add #warning-ignore-all:return_value_discarded
somewhere, probably above the # Assign
comment.
access = 2 | ||
filters = PoolStringArray( "*.png" ) | ||
current_dir = "/home/ubuntu_noah/Documents/New_2019_GitHub_Repositories/godot-demo-projects/2d/gd_paint" | ||
current_path = "/home/ubuntu_noah/Documents/New_2019_GitHub_Repositories/godot-demo-projects/2d/gd_paint/" |
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.
This isn't really a problem with this demo, as it's more of a Godot bug, but I'm making a quick note here. This information shouldn't be saved or committed. You can leave this as-is in the meantime.
Made changes based on feedback (thanks @aaronfranke!). The blurry appearance of the icons is due to the As for the circle looking like an octagon, it is the best I can do in a 16x16 grid. Feel free to make a PR changing the image if you want, but personally I think for the purpose of this demo, showing off how to use the |
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.
2d/gd_paint/PaintControl.gd
Outdated
# Save the image with the passed in path we got from the save dialog | ||
cropped_image.save_png(path) | ||
|
||
return |
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.
Pointless return at the end of a function.
Thanks for the new tool sheet @aaronfranke, it looks great! |
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'm reviewing this as Approve because it's already merge-worthy IMO, but there are some editor/display_folded
lines which could be cleaned up.
custom_styles/panelnc = SubResource( 1 ) | ||
|
||
[node name="PaintControl" type="Control" parent="."] | ||
editor/display_folded = true |
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.
Please remove all editor/display_folded
lines. Here's a bash alias that you can add to ~/.bash_aliases
:
alias tscnnoeditor="find . -name '*.tscn' -exec sed -i '/^editor\//d' {} \;" # Remove "editor/display_folded"
I made a MS paint like example to help show how to use the drawing functions in CanvasItem, capturing and cropping the Viewport to get the final image, and saving the image to a png (which I actually didn't know was possible until I made this).
It has a pencil, eraser, rectangle shape, and circle shape. The pencil and eraser can use either a square shape or a circle shape. All of the code is (hopefully) neatly commented and easy to understand.
In theory this could be extended to allow extra features, like layers or unlimited undos, but I thought I'd leave it where so it (hopefully) serves as a decent starting point for anyone wanting to make a drawing program in Godot.