JirkaDellOro / FUDGE

Furtwangen University Didactic Game Editor
https://jirkadelloro.github.io/FUDGE
MIT License
33 stars 27 forks source link

Co-Dependant Scripts loaded in the wrong order cause the Graph in Internal.json to get corrupted #400

Open Plagiatus opened 1 year ago

Plagiatus commented 1 year ago

How to reproduce:

  1. create a new project, add Dependant.ts and Manager.ts (see below) to your scripts
  2. add a new graph, add two nodes to the graph: first a node called "manager", second a node called "dependant"
  3. attempt to add the DependantScript to the "dependant" node. You'll see an error in the console: grafik
  4. Add the ManagerScript to the manager node first, then add the DependantScript afterwards. No errors, the scripts load as intended.
  5. save (and reload) the project: everything still works fine
  6. now take the manager and make it a child of dependant, then a child of the top level graph again. this changes the order of the nodes, now the dependant is the first child of the graph, the manager is the second.
  7. save the project, and reload (or check the browser) - you'll see an error message like this: in the editor:
    grafik in the browser:
    grafik
  8. save and reload the project again without changes. You'll get this error message and the entire graph is gone. grafik

You can also start with the dependant being higher in the node tree from the start, it will save just fine if you add the manager script first even if it's lower in the list. Only when reloading it becomes a problem.


Related issue: when creating a different graph after step 4, before saving (and reloading), pulling that graph into the render, then pulling in the original one, the ManagerScript has vanished from the manager node, triggering the error of step 7 immediately as well as the subsequent corruption in step 8.


I'm not entirely sure how wide reaching this damage is yet, as sometimes it also deletes my other graphs from internal, sometimes it doesn't. A first test seems to reveal that any internal resource listed AFTER the affected graph in Internal is removed.


Scripts used:

Manager.ts

namespace Script {
    import ƒ = FudgeCore;
    ƒ.Project.registerScriptNamespace(Script);
    export class ManagerScript extends ƒ.ComponentScript {
        public static Instance: ManagerScript;
        data: string = "some data";

        constructor() {
            super();
            if(!ManagerScript.Instance) ManagerScript.Instance = this;
            return ManagerScript.Instance;
        }
    }
}

Dependant.ts

namespace Script {
    import ƒ = FudgeCore;
    ƒ.Project.registerScriptNamespace(Script);
    export class DependantScript extends ƒ.ComponentScript {
        private data: string;

        constructor() {
            super();
            this.data = ManagerScript.Instance.data;
        }
    }
}
Plagiatus commented 1 year ago

The manager script seems to be occasionally vanishing even from saved nodes. Once the project is in that state it seems it can't be saved and the offending graph and anything in the internal list newer than the graph will be removed.

It's not just from the first node in a graph, either, as I've deliberately put another node above the manager to prevent just that.

JirkaDellOro commented 1 year ago
  1. When loading fails, I'd expect it to stop. Saving the project then will only save resources that have been successfully loaded. That could explain why things appear to disappear. We could probably use exception handling to try to keep loading even after a partial fail.
  2. ComponentScripts are instantiated during deserialization and the order is determined by that. Dependencies in the constructor will thus remain a problem. I guess events should be used to achieve deferred dependencies.
  3. However, in that specific example trying to implement a singleton, public static Instance: ManagerScript = new ManagerScript(); should work, since it's called when the script loads.
Plagiatus commented 1 year ago

We could probably use exception handling to try to keep loading even after a partial fail.

I think that is advisable. Because as it stands it can destroy part of your work without being too obvious about it. Saving the project, even when it errors, causes an improper state to be saved, causing the project to not load correctly anymore. So maybe the issue should be one step earlier - not to save when the output is a broken savestate.

ComponentScripts are instantiated during deserialization and the order is determined by that. Dependencies in the constructor will thus remain a problem. I guess events should be used to achieve deferred dependencies.

I was afraid you'd say that. I dislike having to do it this way, but probably not much I can do. Maybe I can create a special type of static script that is always loaded before others are :eyes: /j I'll try 3 as a workaround.