feat: basic review of sa.editor (#507)

solves #410

Author: faretek1

scratchattach/editor/asset.py

@@ -11,15 +11,15 @@
 @dataclass(init=True, repr=True)
 class AssetFile:
     """
-    Represents the file information for an asset
+    Represents the file information for an asset (not the asset metdata)
     - stores the filename, data, and md5 hash
     """
     filename: str
     _data: Optional[bytes] = field(repr=False, default=None)
     _md5: str = field(repr=False, default_factory=str)
 
     @property
-    def data(self):
+    def data(self) -> bytes:
         """
         Return the contents of the asset file, as bytes
         """
@@ -35,9 +35,9 @@ def data(self):
         return self._data
 
     @property
-    def md5(self):
+    def md5(self) -> str:
         """
-        Compute/retrieve the md5 hash value of the asset file data
+        Compute/retrieve the md5 hex-digest of the asset file data
         """
         if self._md5 is None:
             self._md5 = md5(self.data).hexdigest()
@@ -51,7 +51,7 @@ def __init__(self,
                  file_name: str = "b7853f557e4426412e64bb3da6531a99.svg",
                  _sprite: commons.SpriteInput = build_defaulting.SPRITE_DEFAULT):
         """
-        Represents a generic asset. Can be a sound or an image.
+        Represents a generic asset, with metadata. Can be a sound or a costume.
         https://en.scratch-wiki.info/wiki/Scratch_File_Format#Assets
         """
         try:
@@ -72,14 +72,14 @@ def __repr__(self):
     @property
     def folder(self):
         """
-        Get the folder name of this asset, based on the asset name. Uses the turbowarp syntax
+        Get the folder name of this asset, based on the asset name. Uses the TurboWarp syntax
         """
         return commons.get_folder_name(self.name)
 
     @property
     def name_nfldr(self):
         """
-        Get the asset name after removing the folder name
+        Get the asset name after removing the folder name. Uses the TurboWarp syntax
         """
         return commons.get_name_nofldr(self.name)
 
@@ -96,14 +96,16 @@ def md5ext(self):
         """
         Get the exact file name, as it would be within an sb3 file
         equivalent to the md5ext value using in scratch project JSON
+
+        (alias for file_name)
         """
         return self.file_name
 
     @property
     def parent(self):
         """
-        Return the project that this asset is attached to. If there is no attached project,
-        try returning the attached sprite
+        Return the project (body) that this asset is attached to. If there is no attached project,
+        try returning the attached sprite instead.
         """
         if self.project is None:
             return self.sprite

scratchattach/editor/backpack_json.py

@@ -1,18 +1,16 @@
 """
-Module to deal with the backpack's weird JSON format, by overriding with new load methods
+Module to deal with the backpack's weird JSON format, by overriding editor classes with new load methods
 """
 from __future__ import annotations
 
 from . import block, prim, field, inputs, mutation, sprite
 
 
-def parse_prim_fields(_fields: dict[str]) -> tuple[str | None, str | None, str | None]:
+def parse_prim_fields(_fields: dict[str, dict[str, str]]) -> tuple[str | None, str | None, str | None]:
     """
     Function for reading the fields in a backpack **primitive**
     """
     for key, value in _fields.items():
-        key: str
-        value: dict[str, str]
         prim_value, prim_name, prim_id = (None,) * 3
         if key == "NUM":
             prim_value = value.get("value")
@@ -103,7 +101,7 @@ def load_script(_script_data: list[dict]) -> sprite.Sprite:
     """
     Loads a script into a sprite from the backpack JSON format
     :param _script_data: Backpack script JSON data
-    :return: a blockchain object containing the script
+    :return: a Sprite object containing the script
     """
     # Using a sprite since it simplifies things, e.g. local global loading
     _blockchain = sprite.Sprite()

scratchattach/editor/base.py

@@ -52,7 +52,7 @@ def to_json(self):
 
     def save_json(self, name: str = ''):
         """
-        Save a json file
+        Save json to a file. Adds '.json' for you.
         """
         data = self.to_json()
         with open(f"{self.__class__.__name__.lower()}{name}.json", "w") as f:

scratchattach/editor/block.py

@@ -68,8 +68,8 @@ def link_subcomponents(self):
 
     def add_input(self, name: str, _input: inputs.Input) -> Self:
         """
-        Add an input to the block. 
-        """ # not sure what else to say
+        Attach an input object to the block.
+        """
         self.inputs[name] = _input
         for val in (_input.value, _input.obscurer):
             if isinstance(val, Block):
@@ -78,15 +78,15 @@ def add_input(self, name: str, _input: inputs.Input) -> Self:
 
     def add_field(self, name: str, _field: field.Field) -> Self:
         """
-        Add a field to the block. 
-        """  # not sure what else to sa
+        Attach a field object to the block.
+        """
         self.fields[name] = _field
         return self
 
     def set_mutation(self, _mutation: mutation.Mutation) -> Self:
         """
-        Attach a mutation object and call mutation.link_arguments()
-        """ # this comment explains *what* this does, not *why*
+        Attach a mutation object and link it, also asking the mutation to link its own subcomponents.
+        """
         self.mutation = _mutation
         _mutation.block = self
         _mutation.link_arguments()
@@ -113,14 +113,15 @@ def check_toplevel(self):
     @property
     def target(self):
         """
-        Alias for sprite
-        """ # remove this?
+        Alias for .sprite
+        """
+        # should this be deprecated?
         return self.sprite
 
     @property
     def block_shape(self) -> blockshape.BlockShape:
         """
-        Search for the blockshape stored in blockshape.py
+        Search for & return the associated blockshape stored in blockshape.py
         :return: The block's block shape (by opcode)
         """
         _shape = blockshape.BlockShapes.find(self.opcode, "opcode")
@@ -133,15 +134,16 @@ def block_shape(self) -> blockshape.BlockShape:
     @property
     def can_next(self):
         """
-        :return: Whether the block *can* have a next block (basically checks if it's not a cap block, also considering the behaviour of control_stop)
+        :return: Whether the block *can* have a next block (checks if it's not a cap block, but also considering the special behaviour of control_stop)
         """
         _shape = self.block_shape
         if _shape.is_cap is not blockshape.MUTATION_DEPENDENT:
             return _shape.is_attachable
         else:
             if self.mutation is None:
                 # If there's no mutation, let's just assume yes
-                warnings.warn(f"{self} has no mutation! Assuming we can add block ;-;")
+                # add filterwarnings?
+                warnings.warn(f"{self} has no mutation! Assuming we can add blocks!")
                 return True
 
             return self.mutation.has_next
@@ -150,6 +152,7 @@ def can_next(self):
     def id(self) -> str:
         """
         Work out the id of this block by searching through the sprite dictionary
+        If one cannot be found, generate a new one and return that.
         """
         if self._id:
             return self._id
@@ -191,8 +194,9 @@ def next_id(self):
     @property
     def relatives(self) -> list[Block]:
         """
-        :return: A list of blocks which are related to this block (e.g. parent, next, inputs)
+        :return: A list of blocks which are related to this block (i.e. parent & next)
         """
+        # TODO: consider adding input blocks?
         _ret = []
 
         def yield_block(_block: Block | None):
@@ -209,6 +213,7 @@ def children(self) -> list[Block | prim.Prim]:
         """
         :return: A list of blocks that are inside of this block, **NOT INCLUDING THE ATTACHED BLOCK**
         """
+        # does this include procedure definitions' inner block?
         _children = []
         for _input in self.inputs.values():
             if isinstance(_input.value, Block) or isinstance(_input.value, prim.Prim):
@@ -223,7 +228,8 @@ def previous_chain(self):
         """
         Recursive getter method to get all previous blocks in the blockchain (until hitting a top-level block)
         """
-        if self.parent is None: # todo: use is_top_level?
+        # TODO: check if this hits the recursion limit
+        if self.parent is None: # TODO: use is_top_level?
             return [self]
 
         return [self] + self.parent.previous_chain
@@ -233,6 +239,7 @@ def attached_chain(self):
         """
         Recursive getter method to get all next blocks in the blockchain (until hitting a bottom-levell block)
         """
+        # TODO: check if this hits the recursion limit
         if self.next is None:
             return [self]
 
@@ -248,8 +255,7 @@ def complete_chain(self):
     @property
     def top_level_block(self):
         """
-        Get the first block in the block stack that this block is part of
-        same as the old stack_parent property from sbedtior v1
+        Get the first (top level) block in the block stack that this block is part of
         """
         return self.previous_chain[-1]
 
@@ -266,7 +272,7 @@ def stack_tree(self):
         Useful for showing a block stack in the console, using pprint
         :return: A tree-like nested list structure representing the stack of blocks, including inputs, starting at this block
         """
-        _tree = [self]
+        _tree: list[Block | prim.Prim | list[Block]] = [self]
         for child in self.children:
             if isinstance(child, prim.Prim):
                 _tree.append(child)
@@ -281,7 +287,7 @@ def stack_tree(self):
     @property
     def category(self):
         """
-        Works out what category of block this is using the opcode. Does not perform validation
+        Works out what category of block this is as a string, using the opcode. Does not perform validation
         """
         return self.opcode.split('_')[0]
 
@@ -526,7 +532,7 @@ def attach_block(self, new: Block) -> Block:
     def duplicate_single_block(self) -> Block:
         return self.attach_block(self.dcopy())
 
-    def attach_chain(self, *chain: Iterable[Block]) -> Block:
+    def attach_chain(self, *chain: Block) -> Block:
         attaching_block = self
         for _block in chain:
             attaching_block = attaching_block.attach_block(_block)
@@ -576,4 +582,3 @@ def delete_chain(self):
         """
         for _block in self.attached_chain:
             _block.delete_single_block()
-

scratchattach/editor/build_defaulting.py

@@ -9,7 +9,7 @@
     from . import sprite, block, prim, comment
 from . import commons
 
-
+# TODO: should deque be used here?
 class _SetSprite(commons.Singleton):
     INSTANCE = 0
     def __repr__(self):