-
Notifications
You must be signed in to change notification settings - Fork 27
Add 4 random number generation blocks #194
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, thanks @jfcharron ! Can you give me permission to edit your PR? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork#enabling-repository-maintainer-permissions-on-existing-pull-requests
So I can squash the commits and merge.
b.statement = "(randi_range({from}, {to}))" | ||
b.defaults = {"from": -1, "to": 1} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the 2 "random integer" blocks can be one, if the defaults are from: 0, to: 2^32-1
. Then the code (statement) can be:
randi() if {from} == 0 and {to} == 2^32-1 else randi_range({from}, {to})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will merge the two blocks, because they are pretty similar. Also I will do the same for randi.
I feel the if statement is unnecessary because after some thought, I believe that the randi method is not useful in our case. I don't see a case where you would want such a high number of random values. So I will just the let the users enter the range they desire.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will merge the two blocks, because they are pretty similar. Also I will do the same for randi.
I feel the if statement is unnecessary because after some thought, I believe that the randi method is not useful in our case. I don't see a case where you would want such a high number of random values. So I will just the let the users enter the range they desire.
Yes, very well thought @jfcharron. Programming with blocks should be easy and providing multiple blocks that do the same can be confusing to the end users.
b.statement = "(randf_range({from}, {to}))" | ||
b.defaults = {"from": -1.0, "to": 1.0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the 2 "random floating point" blocks can be one, if the defaults are from: 0.0, to: 1.0
. Then the code (statement) can be:
randf() if {from} == 0.0 and {to} == 1.1 else randf_range({from}, {to})
The |
@jfcharron merged! I tested placing a sprite in a random vertical position and it worked like a charm: I have converted the blocks to the new definition format, which uses resources. In case you want to contribute new blocks, this is now the way to do it. It is nicer because the definitions can be edited in the editor directly: Thanks! |
Add 4 random number generation blocks.
One for each of these godot's functions:
randi
randi_range(from,to)
randf
randf_range(from,to)