-
Notifications
You must be signed in to change notification settings - Fork 28
feat: Added Script Window #180
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
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.
I like this and had been thinking about doing the same thing for a while. I left a few cleanup comments, but overall this looks good to me. Thanks!
addons/block_code/plugin.cfg
Outdated
@@ -3,5 +3,5 @@ | |||
name="BlockCode" | |||
description="Create games using a high-level, block-based visual programming language." | |||
author="Endless" | |||
version="$Format:%(describe:tags)$" | |||
version="v0.6.1-27-g73861397" |
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.
Can you leave this out of the patch? It's there for when we make releases.
addons/block_code/ui/main_panel.gd
Outdated
|
||
#BlockCodePlugin.script_window.script_content = script | ||
add_script_window.emit(script) | ||
#_print_generated_script() |
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.
Can you remove the commented lines?
addons/block_code/ui/main_panel.gd
Outdated
@@ -1,6 +1,8 @@ | |||
@tool | |||
extends Control | |||
|
|||
signal add_script_window(script: String) |
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.
Can we call this signal script_window_requested
?
|
||
|
||
func _on_close_requested(): | ||
visible = false |
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 don't think this is ever connected, so I think it can be removed.
layout_mode = 2 | ||
size_flags_horizontal = 0 | ||
size_flags_vertical = 0 | ||
text = "Here is what your Block Code looks like in GDScript!" |
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 think this would be better as the window title instead of a label in the window. I think I'd probably make it something a little more generic like Block Code Generated GDScript
. But we can always fine tune it after merging.
Happy to see you liked the feature :). Sure thing! I will make the requested changes. |
If there is any other problem with the pull request, let me know ;-) |
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.
Thank you for the contribution @GustavoLR548 ! One more detail.
@@ -0,0 +1,14 @@ | |||
@tool | |||
extends Window | |||
class_name ScriptWindow |
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.
@GustavoLR548 please remove the class name. You are already preloading this script fine in addons/block_code/block_code_plugin.gd
using the resource path. The problem we found with class_name
was that they can collide with the user project. Suppose a user has a ScriptWindow
class in their project and then they add this plugin, the names collide.
class_name ScriptWindow |
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.
Oh, sorry about that!
Let me fix this.
Okay, I removed the class name. Additionally, I changed the name of the node and the function called by the signal responsible for showing the code to reflect the changes made. Let me know if you need any further adjustments. |
func script_window_requested(script: String): | ||
|
||
var script_window = ScriptWindow.instantiate() | ||
script_window.script_content = script |
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.
@GustavoLR548 I noticed in your screenshot that there is no indentation. I think you can get indentation by replacing the tabs with 4 spaces:
script_window.script_content = script | |
script_window.script_content = script.replace('\t', ' ') |
And then add a monospace font to the label:
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.
@GustavoLR548 with syntax highlight this is just perfect! Please squash your hotfixes into a single commit so we can merge.
Ok, I think I was able to correctly squash the commits into a single one. If I made any mistakes or if there is any other adjustments to be made, let me know |
Obrigado! This is now in a single commit. As a last request, can I ask you to change the commit description to describe the change? Something like the following would be enough:
|
De nada :) Sure! I will change the commit description. |
Show generated script in a window Replace printing the generated script to the output. Display it in a separate window instead.
@GustavoLR548 your contribution is now merged and will be in next release, thank you! We have some checks and they were failing so I decided to fix them myself in #183 . |
In the future it might be best to have the contributor allow edits by maintainers and update their branch directly. Then you won't need to open a separate PR on a separate branch. |
Oh I didn't know about that feature. Thanks @dbnicholson, I will do so next time. |
Script Window for generated GDScript code
Instead of just printing the results to the screen, I created a Window to popup with the generated script.
You can also copy the entire generated code with a press of a button
If there is any problem with the code or anything you would like to change, please let me know!
Edit: The "Copiar" is misleading, I inserted the text as "Copy", but when running the code, it translated to "Copiar" for some reason that I cannot explain.