JakAttack909 / blockly

Automatically exported from code.google.com/p/blockly
0 stars 0 forks source link

Graph demo -- "random integer from A to B" doesn't work #170

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?

http://blockly-demo.appspot.com/static/apps/graph/index.html#b8cdz7

What is the expected output? What do you see instead?

Expected to get graph similar to 
http://blockly-demo.appspot.com/static/apps/graph/index.html#vcg2m5
But, nothing is displayed

What browser are you using?

Chrome ver 30.0.1599.69 for Mac

Original issue reported on code.google.com by bup...@google.com on 15 Oct 2013 at 12:37

GoogleCodeExporter commented 8 years ago
i think i nad similar problems with that block.

I changed the generated js code to this and it's worked for me:
Blockly.JavaScript['math_random_int'] = function(block) {
  // Random integer between [X] and [Y].
  var argument0 = Blockly.JavaScript.valueToCode(block, 'FROM',
      Blockly.JavaScript.ORDER_COMMA) || '0';
  var argument1 = Blockly.JavaScript.valueToCode(block, 'TO',
      Blockly.JavaScript.ORDER_COMMA) || '0';

  var code = 'Math.round(' + argument0 + ' + Math.random() * (' + argument1 + ' - ' + argument0 + '))';
  return [code, Blockly.JavaScript.ORDER_FUNCTION_CALL];
};

Original comment by tvikto...@gmail.com on 20 Oct 2013 at 6:18

GoogleCodeExporter commented 8 years ago
Interesting.  Thanks for the clear bug report.  I have verified.

I suspect that the problem has to do with including the function that Blockly 
uses for random_int.  Viktor's code eliminates the need to include a function 
but does not work if argument1 < argument0.

I've never touched the graph app so am going to pass this to Neil.

Original comment by sper...@google.com on 21 Oct 2013 at 7:26

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Sorry, but i must say, it is working both arg1>arg0 and arg0>arg1.

Example arg0=30 arg1=100 -> 30+random()*70 the result will be between 30 and 100

arg0=100 arg1=30 -> 100 + random()*(-70) -> 100 - random()*70 the result will 
be between 100 and 30

Both are give the same range of rsult; as expected, the first case of the 
result in ascendant row, the second in the descendant row.

Original comment by tvikto...@gmail.com on 22 Oct 2013 at 2:26

GoogleCodeExporter commented 8 years ago
http://blockly-demo.appspot.com/static/apps/graph/index.html#zhp4oe

I think code generation line should be slight changed:

var code = 'Math.round((' + argument0 + ') + Math.random() * ((' + argument1 + 
') - (' + argument0 + ')))';

Original comment by bup...@google.com on 22 Oct 2013 at 5:05

GoogleCodeExporter commented 8 years ago
tviktor, I apologize.  Your code does not depend on arg0 being less than arg1.  
There is another issue, however.  Using round() biases the function against a 
result of arg0 or arg1, favoring values in between them.  I believe the correct 
function is:

Math.floor(math.min(argument0, argument1) + Math.random() * (Math.abs(argument1 
- argument0) + 1))

bupjae, I do not believe the parentheses are necessary.  The following line 
will generate them if needed:

  var argument0 = Blockly.JavaScript.valueToCode(block, 'FROM',
      Blockly.JavaScript.ORDER_COMMA) || '0';

ORDER_COMMA means to parenthesize the code (if necessary) so it can't be broken 
apart by a comma.

Original comment by sper...@google.com on 22 Oct 2013 at 7:05

GoogleCodeExporter commented 8 years ago
Correcting capitalization, that should be:

Math.floor(Math.min(argument0, argument1) + Math.random() * (Math.abs(argument1 
- argument0) + 1))

Original comment by sper...@google.com on 22 Oct 2013 at 7:11

GoogleCodeExporter commented 8 years ago
tviktor, in the future, please share fixes with us so we can review and 
incorporate them.

Original comment by sper...@google.com on 22 Oct 2013 at 7:19

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r1458.

Original comment by neil.fra...@gmail.com on 29 Oct 2013 at 9:20