[Suggestion] Enforced arguments

[theoriginalbit]

Some functions within Lua/ComputerCraft require variables in order to work (as I'm sure you know) and error when they are not provided... As such I think when the Lua button is pressed to generate code, any functions that specify they require arguments should light up red (?) and specify that they require these arguments before allowing the person to see generated code.

I feel this is a good direction to go in as it will eliminate those who may think that blockly-lua does this check and immediately release their program without testing (believe me, those n00bs do exist!)

A quick list of some turtle functions that need arguments and error without them

• turtle.select
• turtle.getItemCount
• turtle.getItemSpace
• turtle.compareTo
• turtle.transferTo

[espertus]

BTW, our generated code does have values for the arguments. Defaults are specified in the code generators.

What would you think if I required all arguments to be explicitly specified. Do you see a downside?

[theoriginalbit]

Well the main reason that I think this should be, yes I know defaults can be specified in the code generators, but lets look at the code generator for turtle.transferTo for example......

Blockly.Lua['turtle_transfer_to'] = function(block) {
  // Generate Lua for comparing items in the current slot and the supplied one
  var argument0 = Blockly.Lua.valueToCode(block, 'SLOT', Blockly.Lua.ORDER_NONE) || '';
  var argument1 = Blockly.Lua.valueToCode(block, 'QUANTITY', Blockly.Lua.ORDER_NONE) || '';
  var code = 'turtle.transferTo(' + argument0 + (argument1 != '' ? ', ' + argument1 : '') + ')';
  return BlocklyLua.HELPER_FUNCTIONS.generatedCode(block, code);
}

if they didn't provide the variables it would end up as turtle.transferTo() which is wrong, it would error since there is no target slot provided

now if we were to add defaults to that

Blockly.Lua['turtle_transfer_to'] = function(block) {
  // Generate Lua for comparing items in the current slot and the supplied one
  var argument0 = Blockly.Lua.valueToCode(block, 'SLOT', Blockly.Lua.ORDER_NONE) || '1';
  var argument1 = Blockly.Lua.valueToCode(block, 'QUANTITY', Blockly.Lua.ORDER_NONE) || '';
  var code = 'turtle.transferTo(' + argument0 + (argument1 != '' ? ', ' + argument1 : '') + ')';
  return BlocklyLua.HELPER_FUNCTIONS.generatedCode(block, code);
}

the output would now be turtle.transferTo(1) meaning that it could cause unexpected results as they don't necessarily know that there's something wrong with excluding the slot variable.

This is the main reason for me making this suggestion to enforce arguments specified by the code block.

[espertus]

That's a good idea. I'll see what's necessary to implement it (unless you'd like to).

I may be able to reuse some code from " https://blockly-demo.appspot.com/static/apps/puzzle/index.html". (Click "Check Answers".)

On Mon, Nov 4, 2013 at 7:14 PM, Josh Asbury [email protected]:

Some functions within Lua/ComputerCraft require variables in order to work (as I'm sure you know) and error when they are not provided... As such I think when the Lua button is pressed to generate code, any functions that specify they require arguments should light up red (?) and specify that they require these arguments before allowing the person to see generated code.

I feel this is a good direction to go in as it will eliminate those who may think that blockly-lua does this check and immediately release their program without testing (believe me, those n00bs do exist!)

A quick list of some turtle functions that need arguments and error without them

• turtle.select http://computercraft.info/wiki/Turtle.select
• turtle.getItemCounthttp://computercraft.info/wiki/Turtle.getItemCount
• turtle.getItemSpacehttp://computercraft.info/wiki/Turtle.getItemSpace
• turtle.compareTo http://computercraft.info/wiki/Turtle.compareTo
• turtle.transferTo http://computercraft.info/wiki/Turtle.transferTo

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4 .

[theoriginalbit]

Yeh I quite like the idea of the popup saying that a code block is wrong and then having it flash.

[espertus]

Unless you have an idea to do otherwise, I think I will require all sockets for ComputerCraft functions to be filled. I'll provide a different way of specifying "all" than having a blank argument. (I could even have an "all" block that only can go in certain functions, just like "break" can only go into a loop.)

As always, your thoughts are welcome.

On Tue, Nov 5, 2013 at 7:57 PM, Josh Asbury [email protected]:

Yeh I quite like the idea of the popup saying that a code block is wrong and then having it flash.

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-27841225 .

[espertus]

Can you update this page [http://computercraft.info/wiki/Turtle.refuel] to indicate that the parameter is optional? It's protected, so I can't. (I don't know your status on the wiki.)

Thanks.

On Wed, Nov 6, 2013 at 12:19 PM, Ellen Spertus [email protected] wrote:

Unless you have an idea to do otherwise, I think I will require all sockets for ComputerCraft functions to be filled. I'll provide a different way of specifying "all" than having a blank argument. (I could even have an "all" block that only can go in certain functions, just like "break" can only go into a loop.)

As always, your thoughts are welcome.

On Tue, Nov 5, 2013 at 7:57 PM, Josh Asbury [email protected]:

Yeh I quite like the idea of the popup saying that a code block is wrong and then having it flash.

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-27841225 .

[theoriginalbit]

Well the design choice is definitely up to you as to what you want to do, whether its make all required and the block can specify if its optional, or whether make it all optional and the block specifies what is required it does not matter. I just feel that adding this feature would definitely help people learn :)

Updated wiki page with better description and better code examples.

[espertus]

I was thinking of making all arguments to turtle functions required, even if it is the new "all" block that doesn't generate any code. (I just don't like empty sockets, and they bug users too.) Do you see any problems with that approach?

I'd be grateful if you looked over my commits.

I don't like the lack of granularity github provides for contributors. Could I add you to the project but still be the one to make the commits? Since github doesn't enforce that, it would just be our agreement.

On Wed, Nov 6, 2013 at 4:14 PM, Joshua Asbury [email protected]:

Well the design choice is definitely up to you as to what you want to do, whether its make all required and the block can specify if its optional, or whether make it all optional and the block specifies what is required it does not matter. I just feel that adding this feature would definitely help people learn :)

Updated wiki page with better description and better code examples.

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-27927285 .

[theoriginalbit]

Yeh empty sockets are annoying, seems like you're missing something, maybe have a null block, which generates nil?

[espertus]

On Wed, Nov 6, 2013 at 4:24 PM, Joshua Asbury [email protected]:

Yeh empty sockets are annoying, seems like you're missing something, maybe have a null block, which generates nil?

I don't think we need it. I think we can place the "all" block, which generates the empty string (and an anti-comma if used in the second argument position).

Is there any other situation where an input would be left empty?

Ellen

[theoriginalbit]

Indeed... first that comes to mind is the read function.

function stub function read( _sReplaceChar, _tHistory )

so if I wanted to make a password read you'd do either of the following 2

local input = read("*")

local input = read("*", nil)

since having , nil is the same as omitting it

however if I wanted an input that had an input history but not mask the input you have to define like so

local history = {}
while true do
  local input = read(nil, history)
  table.insert(history, input)
end

[espertus]

Thank you. I would never have known about this.

I think I'll still go with an "all" block for the Turtle API.

On Wed, Nov 6, 2013 at 4:33 PM, Joshua Asbury [email protected]:

Indeed... the read function.

function stub function read( _sReplaceChar, _tHistory )

so if I wanted to make a password read you'd do either of the following 2

local input = read("*")

local input = read("*", nil)

since having , nil is the same as omitting it

however if I wanted an input that had an input history but not mask the input you have to define like so

local history = {}while true do local input = read(nil, history) table.insert(history, input)end

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-27928300 .

[espertus]

FYI, I had to change my "all" checking when I realized that it's on a per-input basis, not a per-block basis. IOW, turtle_transfer_to, has two inputs, only one of which permits "all".

On Wed, Nov 6, 2013 at 4:42 PM, Ellen Spertus [email protected] wrote:

Thank you. I would never have known about this.

I think I'll still go with an "all" block for the Turtle API.

On Wed, Nov 6, 2013 at 4:33 PM, Joshua Asbury [email protected]:

Indeed... the read function.

function stub function read( _sReplaceChar, _tHistory )

so if I wanted to make a password read you'd do either of the following 2

local input = read("*")

local input = read("*", nil)

since having , nil is the same as omitting it

however if I wanted an input that had an input history but not mask the input you have to define like so

local history = {}while true do local input = read(nil, history) table.insert(history, input)end

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-27928300 .

[espertus]

I implemented enforced arguments. Take a look at the test server ( http://test.blockly-lua.appspot.com/static/apps/code/index.html) and let me know what you think, including offering an override. I know the dialog that pops up is positioned poorly. I'll see if I can get placement help from my colleague Mr. Blockly.

On Thu, Nov 7, 2013 at 12:35 PM, Ellen Spertus [email protected] wrote:

FYI, I had to change my "all" checking when I realized that it's on a per-input basis, not a per-block basis. IOW, turtle_transfer_to, has two inputs, only one of which permits "all".

On Wed, Nov 6, 2013 at 4:42 PM, Ellen Spertus [email protected] wrote:

Thank you. I would never have known about this.

I think I'll still go with an "all" block for the Turtle API.

On Wed, Nov 6, 2013 at 4:33 PM, Joshua Asbury [email protected]:

Indeed... the read function.

function stub function read( _sReplaceChar, _tHistory )

so if I wanted to make a password read you'd do either of the following 2

local input = read("*")

local input = read("*", nil)

since having , nil is the same as omitting it

however if I wanted an input that had an input history but not mask the input you have to define like so

local history = {}while true do local input = read(nil, history) table.insert(history, input)end

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-27928300 .

[theoriginalbit]

yep looks good, esp once you get that placement bug fixed.

The only thing I could pick on is that if I make a program that is solely turtle.transferTo(1,2) it will bring up the popup 'cause that block has a return value and Blockly seems to want it stored somewhere, even though you don't have to get the return values from any function if you don't want to.

[espertus]

Sounds like I should make turtle.transferTo switchable from expression/statement.

On Thu, Nov 14, 2013 at 12:13 AM, Joshua Asbury [email protected]:

yep looks good, esp once you get that placement bug fixed.

The only thing I could pick on is that if I make a program that is solely turtle.transferTo(1,2) it will bring up the popup 'cause that block has a return value and Blockly seems to want it stored somewhere, even though you don't have to get the return values from any function if you don't want to.

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-28466229 .

[espertus]

On Thu, Nov 14, 2013 at 12:13 AM, Joshua Asbury [email protected]:

The only thing I could pick on is that if I make a program that is solely turtle.transferTo(1,2) it will bring up the popup 'cause that block has a return value and Blockly seems to want it stored somewhere, even though you don't have to get the return values from any function if you don't want to.

That block is switchable between expression and statement, so the user could remove the left plug. Options for me are:

1. Not giving an error if the block is switchable and its output unused.
2. Switching the expression to a statement automatically.
3. Displaying a message showing how they can switch the block from an expression to a statement.
4. Leaving as is and giving a warning.

What do you think would be best?

Ellen

[theoriginalbit]

The "ease of use for newbies" programmer inside me says #2. The "usability nut" inside me says #3.

[espertus]

I actually thought that #3 wasn't a very usable solution, since the user would likely feel: Why not just do it for me?

The best option might be to do both 2 and 3 (telling them how to fix it in the future and fixing it automatically) if it's not too hard to implement. What do you think?

Ellen

On Thu, Nov 14, 2013 at 2:31 PM, Joshua Asbury [email protected]:

The "ease of use for newbies" programmer inside me says #2. The "usability nut" inside me says #3.

— Reply to this email directly or view it on GitHubhttps://github.com/espertus/blockly-lua/issues/4#issuecomment-28529600 .

[theoriginalbit]

I assumed that #3 was just like info saying "did you know? You can switch them........" Which is better usability than assuming what they want and making the change, for example they may think it's connected, auto changing it may not alert them to this fact.

I think a system where you state something like "Did you know? You can switch them........." Is probably the better way. However if you combined 2 and 3 to be "did you know? You can switch them.... I've switched this one for you!" And have it flashing, would be ok too, maybe better.

Also, since I didn't actually test this aspect, do all of the problematic ones, or ones in focus, flash? Or is it just the one? If it's the one would it be difficult to have multiple? Since you may make changes to multiple during validation it may be a good idea to have them all flash.

[espertus]

On Thu, Nov 14, 2013 at 2:44 PM, Joshua Asbury [email protected]:

I assumed that #3 was just like info saying "did you know? You can switch them........" Which is better usability than assuming what they want and making the change, for example they may think it's connected, auto changing it may not alert them to this fact.

Ah, I see what you mean. If I really wanted to spoil them, I'd give them a dialog offering to change if for them, but I'm not going to go that far.

I think a system where you state something like "Did you know? You can switch them........." Is probably the better way.

You've convinced me.

Also, since I didn't actually test this aspect, do all of the problematic ones, or ones in focus, flash? Or is it just the one?

Just the first one flashes. I think it was done that way in the Puzzle app so as not to give away the answer (only give them a hint), but it would be more appropriate for me to make all such blocks flash. I'll add it to my list.

Thanks again for all of your help.

Ellen

[theoriginalbit]

Ah, I see what you mean. If I really wanted to spoil them, I'd give them a dialog offering to change if for them, but I'm not going to go that far.

Yeh fair enough, that would be better usability though ;)

You've convinced me.

xD

Just the first one flashes. I think it was done that way in the Puzzle app so as not to give away the answer (only give them a hint), but it would be more appropriate for me to make all such blocks flash. I'll add it to my list.

Yeh fair enough too. Definitely more appropriate in this use case to flash all of them though :)

Thanks again for all of your help. No problems :)

[espertus]

On Thu, Nov 14, 2013 at 3:01 PM, Joshua Asbury [email protected]:

Just the first one flashes. I think it was done that way in the Puzzle app so as not to give away the answer (only give them a hint), but it would be more appropriate for me to make all such blocks flash. I'll add it to my list.

Yeh fair enough too. Definitely more appropriate in this use case to flash all of them though :)

Alas! I spoke with Neil, and only one block can be selected at a time, so I can't easily flash all of the blocks.

Ellen

[theoriginalbit]

:( damn.