[IDEA] Making patching done by extensions more manageable and less conflict prone

[Xeltalliv]

I know this is a weird idea and is unlikely to be implemented, but it is still something interesting to consider.

Problem: currently extensions that patch the same parts of code are likely to conflict and break in some way.

While trying to make Simple3D extension work with AR extension, I ran into an issue that they weren't compatible with one another by default, because Simple3D was wrapping renderer.draw method, while AR extension was overwriting renderer.draw when entering AR mode, an thus, the Simple3D's wrapped function wasn't getting called when in AR. I made a simple fix to make just those 2 extensions work together, but it wouldn't automatically work with any other extensions that also mess with renderer.draw.

Another issue I ran into is trying to add support for extension removing recently added to Snail IDE. I realized that with the current system, there is no way for the extension to undo it's wrapped functions, if they were also wrapped by some other extension afterwards.

I think it may be possible to solve some of those issues by providing some kind of centralized API for patching the code and porting all existing extensions to use it.

An example of what it may look like. It doesn't have to be like this, just an example:

class Patcher {
    ORDER_EARLY = -1
    ORDER_NORMAL = 0
    ORDER_LATE = 1

    constructor(extensionId) {} // Specifies the extension id to be able to output warnings

    wrap(object, propName, fn, fnSetter, order) {} // Adds a wrapper/hook to a function. Unlike the current approach, they are get sorted by the specified order and can be removed in arbitrary order.
    unwrap(object, propName, fn) {} // Removes wrapper

    intendReplace(object, propName) {}  // Needs to run on startup of the extension. If multiple extensions try to do the same and the editor is opened, show a warning with extensionIDs of both extensions stating that there may be some issues.
    unintendReplace(object, propName) {} // Needs to run when extension is removed.

    replace(object, propName, fn, shouldTriggerWrappers) {} // Swaps out existing function for the custom one (often copy-pasted from the source code, but with modifications). If multiple extensions try to replace the same one, the entire list is kept, but only the most recently replaced one is used.
    unreplace(object, propName, fn) {} // Undoes replacing, removed from the list. If there are still other functions in the list, switches to using the most recent replaced one from it.
    isReplaced(object, propName) {} // Checks if anything has already replaced the specified function.
}

// Simple3D
const patcher = new Scratch.Patcher("xeltallivSimple3D");
let drawOriginal = null;
function updateDrawOriginal(newFn) {
    drawOriginal = newFn;
}
function wrappedDraw() {
    if(this.dirty) redraw();
    drawOriginal.call(this);
}
patcher.wrap(Scratch.vm.renderer, "draw", wrappedDraw, updateDrawOriginal, patcher.ORDER_NORMAL);

// Augmented reality
const patcher = new Scratch.Patcher("AR");
function drawXR() {
    const bl = this.xr.renderState.baseLayer; // ADDED
    if (!bl) return; // Should fix very rare crash during exiting  // ADDED

    this._doExitDrawRegion();

    const gl = this._gl;

    gl.bindFramebuffer(gl.FRAMEBUFFER, bl.framebuffer); // CHANGED
    gl.viewport(0, 0, bl.framebufferWidth, bl.framebufferHeight); // CHANGED
    gl.clearColor(0, 0, 0, 0);
    gl.clear(gl.COLOR_BUFFER_BIT);

    this._drawThese(
      this._drawList,
      "default" /*ShaderManager.DRAW_MODE.default*/,
      this._projection,
      {
        framebufferWidth: bl.framebufferWidth, // CHANGED
        framebufferHeight: bl.framebufferHeight, // CHANGED
      }
    );
    if (this._snapshotCallbacks.length > 0) {
      const snapshot = gl.canvas.toDataURL();
      this._snapshotCallbacks.forEach((cb) => cb(snapshot));
      this._snapshotCallbacks = [];
    }
}
patcher.intendReplace(Scratch.vm.renderer, "draw");
patcher.replace(Scratch.vm.renderer, "draw", drawXR, true);

[GarboMuffin]

the Scratch Addons approach is to add if (disabled) return original(); everywhere