sb-edit icon indicating copy to clipboard operation
sb-edit copied to clipboard

scratch-js: should letter/item-of always output with a "- 1"?

Open towerofnix opened this issue 5 years ago • 4 comments
trafficstars

Right now, sb-edit will always output a - 1 in scratch-js projects, because JavaScript uses zero-indexed strings and lists, while Scratch uses one-indexing. Example code (adapted from this project):

this.costume = input[this.stage.vars.A - 1];
foo = this.stage.vars.B[this.stage.vars.C + 1 - 1] * s;
baz = this.stage.vars.B[this.stage.vars.C + 2 - 1];

This code functions fine, but it's possible to make it more syntactically simple - i.e, by combining adjacent addition/subtraction operations.

The overall simplification and optimization of code is a very large project; here, I think it would be pretty straightforward to write just that one optimization to make the letter/item-of blocks output simpler. In pseudo-JS:

function getIndexInput(indexInput) {
  if (indexInput.type === "block") {
    const {opcode} = indexInput;
    if (opcode === OpCode.operator_add || opcode === OpCode.operator_subtract) {
      if (indexInput.inputs.NUM2.type === "number") {
        let {value} = newBlock.inputs.NUM2;
        value += (opcode === OpCode.operator_add) ? -1 : 1;
        if (value === 0) {
          return indexInput.inputs.NUM1;
        } else {
          const newBlock = cloneBlock(indexInput);
          newBlock.inputs.NUM2.value = value;
          return newBlock;
        }
      }
    }
  }
  // not an addition/subtraction block: return a new ((indexInput) - 1) block
  return new Block({
    opcode: OpCode.operator_subtract,
    inputs: {
      NUM1: cloneBlock(indexInput),
      NUM2: 1
    }
  });
}

// then later...
return `blah[${inputToJS(getIndexInput(this.inputs.INDEX))}]`;

This essentially simplifies:

  • ((index) + 1)(index)
  • ((index) + 2)(index + 1)
  • ((index) - 1)(index - 2)
  • (index)((index) - 1)

(That implementation assumes a cloneBlock function exists, and that creating a block works like that. Haven't tested, obviously; just posing it for discussion :P)

towerofnix avatar Feb 02 '20 16:02 towerofnix

This makes sense, and I definitely want to make these sorts of optimizations.

I'd prefer to use code written by someone else for this kind of thing. It seems like it should be possible to fix all ugly output (such as unnecessary arithmetic, unused code, extra parenthesis, etc...) in one go rather than writing lots of optimizations by hand.

My original hope was that Prettier, which sb-edit currently applies programmatically to all output, would handle all these issues. It does take care of formatting, but never changes the content of the code at all; it is mostly useful for whitespace and parenthesis.

Is there another tool available that is willing to modify code more aggressively? (Can eslint fix this stuff?)

PullJosh avatar Feb 03 '20 15:02 PullJosh

Two counterpoints against using an aggressive JS optimizer:

  1. While such optimizations could certainly simplify the logical weight of the code, that isn't the stated purpose of sb-edit. sb-edit is designed to act as a one-to-one converter between any one format of Scratch blocks and another. It is not a goal of sb-edit to reduce the logical weight of the code. In my opinion, that would be better as a separate project, using sb-edit as a core library to perform those optimizations.

  2. Aggressively optimizing JavaScript output means just that - it would operate only on JS. If we or other developers implement new output formats, such as Python/Pygame (which also use zero-indexing), these kinds of optimizations need to be implemented all over again - or result in an inferior/logically different output.

  3. Bonus point: It's not as fun. Optimizing Scratch code is an intriguing project, and just delegating to someone else's library would skip out on that. Same reason we've implemented audio and library engines from the ground up! Writing our own code also means implementing better APIs for interacting with sb-edit, which is a net benefit to everyone.

Technical nerdy ramble, if you like that kind of thing: (click to expand)

So, considering the first point (which I feel to be the most important), why did I suggest this? There's an important concept I want to highlight, which I hinted at in point two: We want to avoid logically different code. IMO, that means any code which reads differently from the original project. That includes optimizations and added complexity - i.e, decreasing or increasing the logical weight. Striking a balance means aiming to change the reading of the code as minimally as possible, which involves making decisions like the one this issue prompts—but those decisions, as we'll see, don't always have solutions that follow the guidelines to a dot.

When outputting to scratch-js, we currently increase the weight by adding the code - 1 to any index operation. Due to JS using zero-indexing versus Scratch's one-, there are some cases where this cannot be avoided, e.g. (item (foo) of list)list[foo - 1]. There are also cases where optimized code would imply removing a block altogether, e.g. (item (baz + 1) of list)list[foo + 1 - 1]. Inevitably, we are changing the code, and this means some change in logic. The question is this: how do we choose to make that change?

To decide, let's look at the purpose of this "balance of logical weight" idea in the first place, since on its own that sounds somewhat pulled out of thin air. The phrase is just a way of summarizing the core principle of sb-edit: we want any output code to be as humanly similar as possible to its input code. "Humanly similar" means code is easy to understand and edit, provided you have a good grasp of the original code. It should be obvious how any necessary changes were made, and, whenever possible, original logic shouldn't be erased or modified.

The current method is the perfect solution in one of those departments: simply adding a - 1 means it is as obvious as it gets what's going on here. JavaScript uses zero-indexing, and Scratch uses one-, so we always need to add an offset to account for the difference.

However, it's not easy for the final user to edit code like this. Suppose sb-edit outputs a newly redundant operation, like list[baz + 3 - 1]. We would be leaving a decision up to the user: do I remove the redundant code (simplifying it to list[baz + 2]), or do I leave it as is, because I'm more used to working with 1-indexes or because I want to retain the exact original logic?

This is a decision that must be made every time such code is edited, and for a human working on a project that does lots of indexing operations, that can get straining quite quickly. There is no fundamentally correct answer; some people will land on either side of the choice. This is why deciding whether or not to optimize that code is not an easy decision for us to make, either.

But if we don't make it, we're posing the user with that question every time they edit. I feel that this is something we should avoid. scratch-js is a library "designed to be easy-to-use for real human beings", and sb-edit is developed under the same spirit. Making the user ask questions like this every time they edit doesn't jibe with that.

For the sake of discussion, let's apply that thinking to the recommendation of performing this type of optimization in all areas of scratch-js output. It would result in simpler code, taking less time to literally read through. But it could mean getting rid of unoptimized code that was intentionally put there. There are reasons for such; for example, specifying a fraction like 413/612 to make the purpose of the value clearer than its decimal value (0.674836601...).

Here, simplifying the logic means outputting simpler, but unfamiliar code. I feel it's better to leave this decision to the user, because they are the one who made it in the first place, by choosing not to optimize their code. If they would like to change their mind later, that's up to them. If we choose to do so, that leaves them with mysterious values that may be confusing - and thus, mental strain, as they try to figure out where it came from and then must decide whether to bring it back to their original deoptimized code or to leave it as sb-edit changed it.

TL;DR: Aggressively optimizing all output code would leave the user with unfamiliar code to read and edit, usually without any worthwhile gain. Optimizing this case (-1 index offset) also means unfamiliar code, but reducing the redundancy is worth it, because otherwise the user has to choose whether or not to do so themselves every time they edit the code.

PS: If you don't mind, do you have any particular plans for reviewing some of the PRs stickin' around? I completely understand if it's just a matter of time not being readily available, but it'd be cool to see them reviewed and eventually merged when you have a chance :)

towerofnix avatar Feb 03 '20 19:02 towerofnix

All excellent points. I'm sold. 👍

PS: If you don't mind, do you have any particular plans for reviewing some of the PRs stickin' around? I completely understand if it's just a matter of time not being readily available, but it'd be cool to see them reviewed and eventually merged when you have a chance :)

Yeah, I'm definitely aware that I'm falling behind, and I apologize. It's been a busy time (this weekend I participated in a scholarship competition and then spent the next day feeling mildly ill), but that's mostly an excuse to justify my laziness. Thanks for the kick in the pants; I'll do my best to get those PRs taken care of. (If I don't, please continue to harass me.)

PullJosh avatar Feb 04 '20 04:02 PullJosh

Yeah, I'm definitely aware that I'm falling behind, and I apologize. It's been a busy time (this weekend I participated in a scholarship competition and then spent the next day feeling mildly ill), but that's mostly an excuse to justify my laziness. Thanks for the kick in the pants; I'll do my best to get those PRs taken care of. (If I don't, please continue to harass me.)

No worries! If you want then sure, but thanks for the attention and response just the same :)

(Also retroactive good luck with the competition—there are absolutely no worries for getting behind a bit! :))

towerofnix avatar Feb 04 '20 14:02 towerofnix

Yay! This is implemented now. My extended ramble above is still a pretty good read; it covers design principles I've been applying recently without even remembering I wrote about them three years ago.

towerofnix avatar Mar 10 '23 16:03 towerofnix