Conversation
Eg. use "Remove" instead of "Queue Free". Also, remove the "On physics process" block. This is confusing since for the default project settings the _process runs at the same pace. https://phabricator.endlessm.com/T35563
The labels have changed "body" by "something" so the blocks holding the "body" variable have dissapeared.
97af8ba to
e383ce9
Compare
| variant_type = 0 | ||
| display_template = "On [body: OBJECT] entered" | ||
| code_template = "func _on_body_entered(body: Node2D): | ||
| display_template = "When entering collision with [something: OBJECT]" |
There was a problem hiding this comment.
I don't like the "entering collision with" / "leaving collision with" pair. I don't think collisions are something you leave. It's awkward to phrase this because we don't have a subject
Brainstorming:
- When colliding with [something] / When no longer colliding with [something]
- When this node collides with [something] / When this node stops colliding with [something]
- When collision with [something] starts / When collision with [something] ends
- When [something] collides with this node / When [something] stops colliding with this node
Not sure I like these much better…
There was a problem hiding this comment.
I like the second one of those; "When this node collides with / stops colliding with [something]". Alternatively, we could write these in the first person, like "When I collide with", which I think is the nicest to read here but stylistically it's tricky because we don't want to be writing everything in first person.
There was a problem hiding this comment.
This is a good point, should we switch everything to first person? Ie. "Remove me" instead of "Remove"? I'm open to do it.
| type = 1 | ||
| variant_type = 0 | ||
| display_template = "On Process" | ||
| display_template = "Every frame" |
There was a problem hiding this comment.
Are we going to consistently use Sentence case rather than Title case for blocks?
I checked what MakeCode Arcade does and actually it uses lower case!
There was a problem hiding this comment.
Yes, I proposed lower case once to the other Will. Today we have a mix of Title case for short labels ("Viewport Center") and Sentence case for long labels. I just checked and Scratch has all in lower case too. So unless anyone has a strong argument I'll switch all to lower case.
- The display template in resource files. - The display template in blocks defined dynamically for properties. - The display template in blocks for the simple nodes. - The true and false booleans. - The options in dropdowns. https://phabricator.endlessm.com/T35563
|
Ready for review again. I think that the switch to lower case makes the label much more readable inside blocks. |
wjt
left a comment
There was a problem hiding this comment.
I think this would be fine but now all the block names are in the diff it's easier to review the others :)
| type = 3 | ||
| variant_type = 3 | ||
| display_template = "Random floating point number between {from: FLOAT} and {to: FLOAT}" | ||
| display_template = "random floating point number between {from: FLOAT} and {to: FLOAT}" |
There was a problem hiding this comment.
A question for another day, but, do students know what "floating point number" means? Would "real number" be better understood?
There was a problem hiding this comment.
I think that we should remove one of these random number blocks and have a single "random number between X and Y". What I'm not sure about is which one should survive, the one for floats or the one for ints. Let's do that as follow-up.
If/when we're looking at English language optimization can we check the animation player blocks text please (I think it had something like "play animation ahead" which can maybe be improved). |
Yes, is "play animation forward" better? |
Co-authored-by: Will Thompson <wjt@endlessos.org>
|
@wjt @dylanmccall this is ready for review again. |
Eg. use "Remove" instead of "Queue Free".
Also, remove the "On physics process" block. This is confusing since for the default project settings the _process runs at the same pace.
https://phabricator.endlessm.com/T35563