Skip to content

Decouple UI from Code Generation #147

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

Merged
merged 17 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions addons/block_code/block_code_plugin.gd
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ const DISABLED_CLASSES := [
"ParameterBlock",
"StatementBlock",
"SnapPoint",
"BlockSerialization",
"BlockSerializedProperties",
"BlockScriptSerialization",
"CategoryFactory",
]
Expand Down
3 changes: 2 additions & 1 deletion addons/block_code/blocks/communication/define_method.tres
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
[resource]
script = ExtResource("1_6e473")
name = &"define_method"
target_node_class = ""
description = "Define a method/function with following statements"
category = "Communication | Methods"
type = 1
variant_type = 0
display_template = "Define method {method_name: NIL}"
display_template = "Define method {method_name: STRING_NAME}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for clarifying what was the "__nil__" prefix about in the original PR and for providing a better workaround. Still seems like a workaround for verbatim input. And I can see how we can benefit from StringName in the short term. Like the work @dylanmccall is doing for having a drop-down of current animation names (which are StringNames) in AnimationPlayer. But we can use Strings in there for now at the expense of optimization, in order to have this big PR merged.

Copy link
Member

Choose a reason for hiding this comment

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

I also tried TYPE_CALLABLE since that's closer to what we're actually doing in this case. You can use a dummy callable like Callable.create(null, 'my_method') and later get the name out with callable.get_method(). Unfortunately, godot only supports serializing an empty Callable(), so the name would be lost in the serialization.

I tried hard to think of different alternatives, but I couldn't think of anything if we are completely in the world that inputs are encoded as Variant and only that is used to determine the type.

Copy link
Contributor

@dylanmccall dylanmccall Sep 6, 2024

Choose a reason for hiding this comment

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

I haven't read through the commits here so sorry if I'm missing details, but I wonder if the thing we need in the end here is really more in the code template end of things? We need a way to say "use this parameter as written, but don't quote it or coerce it into anything new". Could communicate that like {{method_name}} instead of {method_name} in the code template, with nothing special in the display template.

I didn't touch that, but I was poking around here in #206 by removing the pretend OPTION parameter type and (as a result) using NIL as a type wherever we had used OPTION.

Copy link
Member

Choose a reason for hiding this comment

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

The problem isn't so much in the template side. We can put whatever we want there and add something special. The problem comes in the generation side because everything is introspected based on the variant type. Even right now where this all happens in ParameterInput.get_string, everything happens based on variant_type except for the oddball case for OPTION. Actually, that part looks pretty much the same. The only reason define_method worked before this is that EntryBlock is treated specially, and get_entry_statement doesn't go through the normal substitution path.

I guess a better way I hadn't thought of before is to use a dedicated class and then special case it within TYPE_OBJECT like OptionData is. E.g., FunctionParameter or something like that.

code_template = "func {method_name}():"
defaults = {}
signal_name = ""
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[gd_resource type="Resource" load_steps=4 format=3 uid="uid://c5e1byehtxwc0"]

[ext_resource type="Script" path="res://addons/block_code/code_generation/block_definition.gd" id="1_emeuv"]
[ext_resource type="Script" path="res://addons/block_code/ui/block_canvas/option_data.gd" id="1_xu43h"]
[ext_resource type="Script" path="res://addons/block_code/code_generation/option_data.gd" id="1_xu43h"]

[sub_resource type="Resource" id="Resource_vnp2w"]
script = ExtResource("1_xu43h")
Expand Down
2 changes: 1 addition & 1 deletion addons/block_code/blocks/logic/compare.tres
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[gd_resource type="Resource" load_steps=4 format=3 uid="uid://pr5wnn3ltkbo"]

[ext_resource type="Script" path="res://addons/block_code/ui/block_canvas/option_data.gd" id="1_hcv2h"]
[ext_resource type="Script" path="res://addons/block_code/code_generation/option_data.gd" id="1_hcv2h"]
[ext_resource type="Script" path="res://addons/block_code/code_generation/block_definition.gd" id="1_wp40r"]

[sub_resource type="Resource" id="Resource_ie4sg"]
Expand Down
2 changes: 1 addition & 1 deletion addons/block_code/blocks/sounds/pause_continue_sound.tres
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[gd_resource type="Resource" load_steps=4 format=3 uid="uid://wpdspamg3f6g"]

[ext_resource type="Script" path="res://addons/block_code/ui/block_canvas/option_data.gd" id="1_ilhdq"]
[ext_resource type="Script" path="res://addons/block_code/code_generation/option_data.gd" id="1_ilhdq"]
[ext_resource type="Script" path="res://addons/block_code/code_generation/block_definition.gd" id="1_q04gm"]

[sub_resource type="Resource" id="Resource_lalgp"]
Expand Down
37 changes: 37 additions & 0 deletions addons/block_code/code_generation/ast_list.gd
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
extends RefCounted

const Types = preload("res://addons/block_code/types/types.gd")
const BlockAST = preload("res://addons/block_code/code_generation/block_ast.gd")

var array: Array[ASTPair]


class ASTPair:
var ast: BlockAST
var canvas_position: Vector2

func _init(p_ast: BlockAST, p_canvas_position: Vector2):
ast = p_ast
canvas_position = p_canvas_position


func _init():
array = []


func append(ast: BlockAST, canvas_position: Vector2):
array.append(ASTPair.new(ast, canvas_position))


func clear():
array.clear()


func get_top_level_nodes_of_type(block_type: Types.BlockType) -> Array[BlockAST]:
var asts: Array[BlockAST] = []

for ast_pair in array:
if ast_pair.ast.root.data.type == block_type:
asts.append(ast_pair.ast)

return asts
145 changes: 145 additions & 0 deletions addons/block_code/code_generation/block_ast.gd
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
extends RefCounted

const BlockAST = preload("res://addons/block_code/code_generation/block_ast.gd")
const OptionData = preload("res://addons/block_code/code_generation/option_data.gd")
const Types = preload("res://addons/block_code/types/types.gd")

var root: ASTNode


class IDHandler:
static var counts: Dictionary = {}

static func reset():
counts = {}

static func get_unique_id(str: String) -> int:
if not counts.has(str):
counts[str] = 0

counts[str] += 1

return counts[str]

static func make_unique(formatted_string: String) -> String:
var unique_string = formatted_string
var regex = RegEx.new()
regex.compile("\\b__[^\\s]+")
var ids: Dictionary = {}
for result in regex.search_all(formatted_string):
var result_string = result.get_string()
if not ids.has(result_string):
ids[result_string] = get_unique_id(result_string)
unique_string = unique_string.replace(result_string, result_string + "_%d" % ids[result_string])

return unique_string


class ASTNode:
var data #: BlockDefinition
var children: Array[ASTNode]
var arguments: Dictionary # String, ASTValueNode

func _init():
children = []
arguments = {}

func get_code_block() -> String:
var code_block: String = data.code_template # get multiline code_template from block definition

# insert args

# check if args match an overload in the resource

for arg_name in arguments:
# Use parentheses to be safe
var argument = arguments[arg_name]
var code_string: String
if argument is ASTValueNode:
code_string = argument.get_code()
else:
code_string = BlockAST.raw_input_to_code_string(argument)

code_block = code_block.replace("{%s}" % arg_name, code_string)

return IDHandler.make_unique(code_block)

func get_code(depth: int) -> String:
var code: String = ""

# append code block
var code_block := get_code_block()
code_block = code_block.indent("\t".repeat(depth))

code += code_block + "\n"

# fill empty entry and control blocks with pass
if children.is_empty() and (data.type == Types.BlockType.ENTRY || data.type == Types.BlockType.CONTROL):
code += "pass\n".indent("\t".repeat(depth + 1))

for child in children:
code += child.get_code(depth + 1)

return code


class ASTValueNode:
var data #: BlockDefinition
var arguments: Dictionary # String, ASTValueNode

func _init():
arguments = {}

func get_code() -> String:
var code: String = data.code_template # get code_template from block definition

# check if args match an overload in the resource

for arg_name in arguments:
# Use parentheses to be safe
var argument = arguments[arg_name]
var code_string: String
if argument is ASTValueNode:
code_string = argument.get_code()
else:
code_string = BlockAST.raw_input_to_code_string(argument)

code = code.replace("{%s}" % arg_name, code_string)

return IDHandler.make_unique("(%s)" % code)


func get_code() -> String:
IDHandler.reset()
return root.get_code(0)


func _to_string():
return to_string_recursive(root, 0)


func to_string_recursive(node: ASTNode, depth: int) -> String:
var string: String = "%s %s\n" % ["-".repeat(depth), node.data.display_template]

for c in node.children:
string += to_string_recursive(c, depth + 1)

return string


static func raw_input_to_code_string(input) -> String:
match typeof(input):
TYPE_STRING:
return "'%s'" % input.replace("\\", "\\\\").replace("'", "\\'")
TYPE_VECTOR2:
return "Vector2%s" % str(input)
TYPE_COLOR:
return "Color%s" % str(input)
TYPE_OBJECT:
if input is OptionData:
var option_data := input as OptionData
return option_data.items[option_data.selected]
_:
return "%s" % input

return ""
104 changes: 104 additions & 0 deletions addons/block_code/code_generation/blocks_catalog.gd
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
extends Object

const BlockDefinition = preload("res://addons/block_code/code_generation/block_definition.gd")
const OptionData = preload("res://addons/block_code/code_generation/option_data.gd")
const Types = preload("res://addons/block_code/types/types.gd")
const Util = preload("res://addons/block_code/code_generation/util.gd")
const VariableDefinition = preload("res://addons/block_code/code_generation/variable_definition.gd")

const _BLOCKS_PATH = "res://addons/block_code/blocks/"

Expand Down Expand Up @@ -94,6 +96,40 @@ static func _setup_definitions_from_files():
_by_class_name[target][block_definition.name] = block_definition


static func _add_output_definitions(definitions: Array[BlockDefinition]):
Copy link
Contributor

Choose a reason for hiding this comment

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

So if I understand correctly this is adding a block definition for each argument in the display_template of each entry block. Eg. in &"define_method" block it has display_template = "method_name: STRING_NAME}" so it will add a value block to the catalog with name &"define_method_method_name" and variant type StringName and scope "func {method_name}():".

If so, can we call this _add_argument_definitions() instead of "output"?

Copy link
Member

Choose a reason for hiding this comment

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

This is actually for the Area2D and RigidBody2D on_{entered,exited} blocks, too. I called them output definitions because they're associated to what the code currently calls ParameterOutput. Those are encoded in display_template as [param: TYPE]. On the other hand, ParameterInput is used when the template has {param: TYPE}.

Technically, this is the difference between a parameter and an argument. What we're trying to handle here are parameters - the things in the function declaration. What we normally use are arguments, the things used in a function call.

So, in the short term it would probably be better to call this _add_parameter_definitions and ignore the confusion between ParameterInput and ParameterOutput. In the longer term it would be better to change ParameterInput to Argument and ParameterOutput to Parameter. But I suspect there's a lot of loose "parameter" usage within the code.

# Capture things of format [test]
var _output_regex := RegEx.create_from_string("\\[([^\\]]+)\\]")

for definition in definitions:
if definition.type != Types.BlockType.ENTRY:
continue

for reg_match in _output_regex.search_all(definition.display_template):
var parts := reg_match.get_string(1).split(": ")
var param_name := parts[0]
var param_type: Variant.Type = Types.STRING_TO_VARIANT_TYPE[parts[1]]

var output_def := BlockDefinition.new()
output_def.name = &"%s_%s" % [definition.name, param_name]
output_def.target_node_class = definition.target_node_class
output_def.category = definition.category
output_def.type = Types.BlockType.VALUE
output_def.variant_type = param_type
output_def.display_template = param_name
output_def.code_template = param_name
output_def.scope = definition.code_template

# Note that these are not added to the _by_class_name dict
# because they only make sense within the entry block scope.
_catalog[output_def.name] = output_def


static func _setup_output_definitions():
var definitions: Array[BlockDefinition]
definitions.assign(_catalog.values())
_add_output_definitions(definitions)


static func _add_property_definitions(_class_name: String, property_list: Array[Dictionary], property_settings: Dictionary):
for property in property_list:
if not property.name in property_settings:
Expand Down Expand Up @@ -218,6 +254,7 @@ static func setup():

_catalog = {}
_setup_definitions_from_files()
_setup_output_definitions()
_setup_properties_for_class()
_setup_input_block()

Expand All @@ -237,6 +274,45 @@ static func get_blocks_by_class(_class_name: String):
return block_definitions.values()


static func _get_builtin_parents(_class_name: String) -> Array[String]:
var parents: Array[String] = []
var current = _class_name

while current != "":
parents.append(current)
current = ClassDB.get_parent_class(current)

return parents


static func _get_custom_parent_class_name(_custom_class_name: String) -> String:
for class_dict in ProjectSettings.get_global_class_list():
if class_dict.class != _custom_class_name:
continue
var script = load(class_dict.path)
var builtin_class = script.get_instance_base_type()
return builtin_class
return "Node"


static func _get_parents(_class_name: String) -> Array[String]:
if ClassDB.class_exists(_class_name):
return _get_builtin_parents(_class_name)
var parents: Array[String] = [_class_name]
var _parent_class_name = _get_custom_parent_class_name(_class_name)
parents.append_array(_get_builtin_parents(_parent_class_name))
return parents


static func get_inherited_blocks(_class_name: String) -> Array[BlockDefinition]:
setup()

var definitions: Array[BlockDefinition] = []
for _parent_class_name in _get_parents(_class_name):
definitions.append_array(get_blocks_by_class(_parent_class_name))
return definitions


static func add_custom_blocks(
_class_name,
block_definitions: Array[BlockDefinition] = [],
Expand All @@ -252,4 +328,32 @@ static func add_custom_blocks(
_catalog[block_definition.name] = block_definition
_by_class_name[_class_name][block_definition.name] = block_definition

_add_output_definitions(block_definitions)
_add_property_definitions(_class_name, property_list, property_settings)


static func get_variable_block_definitions(variables: Array[VariableDefinition]) -> Array[BlockDefinition]:
var block_definitions: Array[BlockDefinition] = []
for variable: VariableDefinition in variables:
var type_string: String = Types.VARIANT_TYPE_TO_STRING[variable.var_type]

# Getter
var block_def = BlockDefinition.new()
block_def.name = "get_var_%s" % variable.var_name
block_def.category = "Variables"
block_def.type = Types.BlockType.VALUE
block_def.variant_type = variable.var_type
block_def.display_template = variable.var_name
block_def.code_template = variable.var_name
block_definitions.append(block_def)

# Setter
block_def = BlockDefinition.new()
block_def.name = "set_var_%s" % variable.var_name
block_def.category = "Variables"
block_def.type = Types.BlockType.STATEMENT
block_def.display_template = "Set %s to {value: %s}" % [variable.var_name, type_string]
block_def.code_template = "%s = {value}" % [variable.var_name]
block_definitions.append(block_def)

return block_definitions
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
class_name OptionData
extends Resource

@export var selected: int
Expand Down
Loading