Skip to content
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

Add export functionality for dictionary blocks #2595

Merged
merged 6 commits into from
Nov 25, 2020

Conversation

meganindya
Copy link
Member

@meganindya meganindya commented Nov 8, 2020

I would've wanted a object key addition/object key query syntax, however, at the moment we have to go with method calls. I'll implement that.

@walterbender
Copy link
Member

So far it looks good. What else are you planning? (As it is marked DRAFT.)

@meganindya
Copy link
Member Author

I just separated the actions code from DictBlocks.js to DictActions.js. Now I got to implement the export API ... would end up with three methods showDict, setValue, and getValue

@walterbender
Copy link
Member

I was going to add a pie menu for the dictionary keys with the getValue block in the dictionary, but I realized that there is no way to know what turtle is associated with the block when you click on it. So we cannot know how to populate the pie menu. Bummer.

@meganindya
Copy link
Member Author

I got stuck too for a similar reason. turtleDicts and turtleHeaps are implemented as dictionaries with Turtle indices in turtleList as keys. It conflicts with having the Turtle object. For the same reason, I moved all other block properties from logo to Singer/Painter.

But, turtleHeaps was more convoluted so that wasn't done. Now, it's the same with turtleDicts too. If you ask me, I think we could move these into individual Turtle objects, but that has one problem — we wouldn't be able to share dictionaries/heaps between different start blocks.

I think we need a different implementation for these entirely.

@walterbender
Copy link
Member

@meganindya Is this ready to test/review?

@meganindya
Copy link
Member Author

I thought this was done. Very sorry :p

@meganindya
Copy link
Member Author

Here's a working example:

e30dbf98-10ea-46eb-af9e-b875344ce626

@meganindya meganindya marked this pull request as ready for review November 25, 2020 10:17
@meganindya
Copy link
Member Author

methods added:

setValue(key, value)                       set value to unnamed dictionary
setValue(key, value, dictionary)  set value to named dictionary
getValue(key)                                  get value from unnamed dictionary
getValue(key, dictionary)              get value from named dictionary
getDict(dictionary)                        get a dictionary
showDict(dictionary)                      print a dictionary (on top)

@meganindya
Copy link
Member Author

meganindya commented Nov 25, 2020

BTW, we aren't being able to share dictionaries between turtles as of now. I think being able to do that would be really powerful e.g. being able to use actions as parameterized functions with the arguments being in some dictionary.

@meganindya
Copy link
Member Author

closes #2587.

@walterbender walterbender merged commit 94ee056 into sugarlabs:master Nov 25, 2020
@walterbender
Copy link
Member

Sharing between turtles was a big reason I refactored Christopher's original PR. I'd like to push the whole idea further at some point. Thx for the PR in any case.

@meganindya meganindya deleted the dict-js-export branch November 26, 2020 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants