Rename basic_block to entry_block #15
Conversation
Also, all statement blocks should have the top knob.
There was a problem hiding this comment.
I totally agree we need the outline, the same color blocks just meld together haha. Maybe the outline could be cleaned up a little? Like it shouldn't extend past the blocks bounding box, rather it should just be an inside outline. And we need some way to add the outline to the middle section of control blocks as well.
There is also this issue: with the control block outline:
I think you just need to add a + shift_bottom somewhere :)
| @@ -3,7 +3,7 @@ extends Node | |||
|
|
|||
| enum BlockType { | |||
| NONE, | |||
| EXECUTE, | |||
There was a problem hiding this comment.
I feel like these renames diverge from the way I intended the blocks to be used. I think the switch from EXECUTE to STATEMENT is okay, I just used the execute terminology from how nodes in Unreal work (flow of execution), but I don't think StatementBlock blocks are the only blocks that can have this STATEMENT type. The StatementBlock and BasicBlock are both just templates that you can differentiate into new blocks. So while BasicBlock is only used to make entry blocks at the moment, it could be used for other blocks as well. You could even use a Statement block as an entry block with parameters, they are just a base to be extended into different blocks in the factory. So IMO the names should be kept as EXECUTE and BasicBlock, but interesting to hear your thoughts.
There was a problem hiding this comment.
Ah I don't know Unreal "execute". Can you explain what does the execute type mean? There is nothing in GDScript that maps to it. I was even considering renaming "entry" to "(virtual) method".
The difference between block types and block classes is a bit confusing to me. But maybe you can provide examples of:
- a potential basic block that's not an entry block
- a potential statement block that's not a statement block
I would like to hear input from @dylanmccall as well.
There was a problem hiding this comment.
The only thing that block_type does is dictate which blocks can be snapped to a certain SnapPoint. The SnapPoint also has a block_type, and only blocks with matching block_type can be snapped. So, an EXECUTE block will only snap to the inside of control blocks, or the end of other statement blocks. A STRING block will only snap to parameter inputs of STRING type.
The block class is associated with the scene (see get_scene_path(), and is a template for building whatever block you may want. Say you need a block that has a specific name and label, but doesn't generate code, then you would instantiate and customize a BasicBlock. But if you needed a block that generates code from parameters then you could use a StatementBlock. What if you wanted to have an entry point with parameters (like the on signal block). Then you could instantiate the StatementBlock and give it the ENTRY type so it will not snap to EXECUTE snap points.
Execution is just a way of describing the flow of instructions in the program. The program starts executing at the entry block, then moves on to the next instruction/statement (block) it needs to execute. That's why I named these blocks that are supposed to be executed in this order EXECUTE. Snapping a block to the bottom of another is essentially saying to execute it next.
The reason to not rename BasicBlock is because there is no reason to specialize it as an entry block, when it could have (albeit very limited) other purposes. You can specialize it in CategoryFactory.
There was a problem hiding this comment.
@wnbaum thanks for the explanation. For now I will split the UI stuff into its own PR.
I see now how the "On signal {signal: STRING}" block is a statement_block class of ENTRY type that doesn't relate to anything in GDScript. I guess this is the only exception we have now. I find it a bit out of scope as it's not related to anything GDScript (or any Script) can do. But let's leave the discussion for later.
There was a problem hiding this comment.
There is another exception! For the body entered node (or even the process nodes if we want to include delta), we want to have a parameter block that is copyable in the scope of the following script. The code for the body entered node looks like this:
b = BLOCKS["statement_block"].instantiate()
b.block_type = Types.BlockType.ENTRY
b.block_format = "On body [body: NODE] entered"
test_list.append(b)
With the same logic that creates parameter inputs, you can create these copyable function parameters, where you use square brackets instead of curlies. There are definitely other ways of splitting this functionality up away from StatementBlock, but for the MVP I think this is acceptable.
| @@ -14,9 +14,6 @@ var param_input_strings: Dictionary # Only loaded from serialized | |||
|
|
|||
| func _ready(): | |||
| super() | |||
|
|
|||
| if block_type != Types.BlockType.EXECUTE: | |||
| _background.show_top = false | |||
| _background.color = color | |||
There was a problem hiding this comment.
This goes back to the thing about statement blocks able to be used for multiple purposes. Maybe the real rename we should do is changing StatementBlock to BlockWithParametersThatCanFormatStringToCode or something lol. I think we should keep these lines in because they allow a statement block to not just be an execute block, meaning they have no snapping notch at the top.
There was a problem hiding this comment.
In my view "StatementBlock" means presicely that, a block that translate to a line of GDScript, that is, a statement.
There was a problem hiding this comment.
Yes, I agree, the general purpose of StatementBlock is to generate a line of GDScript, but I had recently used it to implement these entry blocks with parameters. Seemed silly to make an entire new block class to do something I already could do with the StatementBlock, that's why I am thinking it could be renamed to something like ParameterInputsBlock or ParameterFormatBlock to cover it's general functionality.
There was a problem hiding this comment.
Well, "On process" will need to expose the delta parameter at some point too. In my mind, it should be an Entry block, not a StatementBlock/ParameterInputsBlock considering Entry blocks as (from my spec):
- Entry block:
- Translates to a Godot method, which may be virtual a method like
_ready().- Thus, may have parameters, each one with its own Variant type.
- Can't be placed inside other blocks slots (it's root)
- Has one slot.
- If nothing is inside, the method body translates to
pass.
- If nothing is inside, the method body translates to
- Only one possible. Ie. if there are 2 "On process" blocks in the canvas, one is greyed out.
- Block parameters can be used only in their body blocks:
- Can drag "body" from "On body [body] entered" into an "if node [] is in group ["foo"]"
- Can't drag "body" outside this set of blocks.
- Translates to a Godot method, which may be virtual a method like
There was a problem hiding this comment.
That makes sense. You can do all of the functionality you describe in the spec with a StatementBlock but if you want to create another EntryBlock class that shares much of its functionality with StatementBlock you could (look at ParameterBlock for example).
However, it does seem like the easiest option is to just rename StatementBlock to ParameterInputsBlock and then when actually constructing the entry blocks, set their block_type to ENTRY. This is what is currently happening.
Good catch! For the outline I have a better idea: pass -1 as width so it's independent of the scale (for when we add zoom):
|
|
@wnbaum I just fixed the outline shape and width. |
Great! Happy to take a look when it's ready :) And yes the -1 width thing sounds right. |
|
Oh, I just had to pull the force push. Looks really good! |
|
@wnbaum actually I'm not convinced by the -1 outline, it doesn't look as good as MS Arcade blocks. I will set it back as it was before. Also I tried making it an inline, but couldn't (two nested IFs looked very bold) I think we can iterate on this later. |
|
Ok hehe. Yea the outline is gonna be a little hard to do. Maybe there's some way to do a drop shadow/inner shadow type thing... possibly with a shader that reads the alpha mask and expands it inward. Not too sure. |
|
Closing because there is now an entry block type, and the rest is in #43 |
No description provided.