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

Proposal: Block Collections #1360

Open
BeksOmega opened this issue Jul 9, 2019 · 4 comments
Open

Proposal: Block Collections #1360

BeksOmega opened this issue Jul 9, 2019 · 4 comments

Comments

@BeksOmega
Copy link
Contributor

Problem Statement

Often you want to access all the blocks with a specific "tag". For example: all procedure blocks, all call blocks for a specific procedure, all blocks of a particular type, all top blocks, etc.

Currently there are two ways of doing this:

  1. Look through every block on the workspace to see if it has the tag.
  2. Create an array on the workspace where you register these blocks.

Both of these approaches have problems. 1 is not very efficient, and 2 is not very extendable.

Proposal

Create functions on the workspace to support adding blocks to and accessing blocks from dynamic collections based on tags.

The structure for the collections object will be a dictionary of string keys that point to arrays of blocks. Each collection can have multiple blocks, and each block can be in multiple collections.

Add a function to the workspace named addBlockToCollection(block, collection).
This function will:

  • Add a property to the collections object if the given collections key does not exist.
  • Optional) Add the key to a keys array that can be used to check if a collection exists.
  • Add the block to the collection array if it has not already been added.
  • Pass back an object to / assign an object to the given block. This object will work similarly to event binding data, and can be used to easily remove the block from all collections when it is disposed.

Add a function to the workspace named removeBlockFromCollection(block, collection).
This function will (all of this is dependent on how addBlockToCollection works):

  • Remove the block from the given collection array (if the array exists and the block is included).
  • (Efficiency?) Remove the collection property from the collections object if the array is now empty. Also remove it from the tags array.
  • Remove the collection data from the block if it was assigned directly instead of passed.

Add a function to the workspace named removeBlockFromAllCollections(block/bindData).
This function will:

  • Use the bind data to remove the block from all collections, in a fasion similar to how events bindings work.

Add a function to the workspace named getBlocksInCollection(collection).
This function will:

  • Return the blocks assigned to the given collection if it exists. Otherwise return an empty array.

If this proposal is accepted I think it may also be useful to set up some naming conventions for the tags. Maybe something like general category:specific tag. E.g. position:top, position:orphan, type:controls_if, caller:do_something.

Use Cases

I think this would be a useful addition to the blockly core because it would make the procedure code more efficient, but I also think it would be useful to outside developers.

Here are some use cases:

  • Operations on custom procedure-like blocks (I had to create these for my application).
  • Operations on blocks containing variables of different types.
  • Accessing all blocks with a certain theme (this could also be used in the core).
  • Collecting all colours used on the workspace to add "recently used colours" to a colour field.
  • Any other time you see people looping through all of the blocks in a workspace.

Plus the things that are already provided i.e. top blocks, all blocks, and blocks by type, this just makes it more extendable and dynamic.

@seldomU
Copy link
Contributor

seldomU commented Jul 20, 2019

I would have use for such collections and see workspace analysis performance as the main benefit. But I'm not sure if it should be part of Blockly or be left to projects to add if needed. Updating collections could be handled outside Blockly by a workspace event listener.

@jollytoad
Copy link

So, wrt to solution 1 above, if you are just iterating through plain old JS objects (ie. not DOM) then it should be perfectly fine performance wise, even when the block count reaches 1k or more. Performance problems usually only surface when DOM (or other non-JS API) interactions are involved.

But, what you propose sounds like indexing of blocks by a tag, so why not just use an object of key -> array of blocks.

@BeksOmega
Copy link
Contributor Author

But, what you propose sounds like indexing of blocks by a tag, so why not just use an object of key -> array of blocks.

An object like {key, [blocks]} is indeed the basic idea. The proposal just adds some utility functions to make things easier.

Using utility functions allows us to save information about where the block is stored to the block. We need to know where the block is stored so that when we dispose of the block we can remove it from all collections. If it gets left in a collection we'll get detached nodes :/

So addBlockToCollection, removeBlockFromCollection, and removeBlockFromAllCollections are just to make it so you don't have to think as much when you dispose of your block.

@maribethb
Copy link
Contributor

@BeksOmega How do you feel about moving this to samples?

Rachel and I discussed and we don't think this belongs in core-

  1. It's doable as a plugin right now using events
  2. The main use case you laid out, procedure blocks, are going to have their own management system thanks to the shared procedures API / procedure maps

@BeksOmega BeksOmega transferred this issue from google/blockly Nov 2, 2022
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

No branches or pull requests

4 participants