Handle MAKI functions that need input, implement popupmenu #854

jberg · 2019-08-15T23:20:47Z

No description provided.

captbaritone · 2019-08-16T06:13:34Z

modern/src/utils.js

@@ -1,5 +1,9 @@
 import { xml2js } from "xml-js";

+export function isPromise(obj) {
+  return "function" == typeof obj.then;


I’d swap the order of these and use ===.

https://en.m.wikipedia.org/wiki/Yoda_conditions

sounds good

captbaritone · 2019-08-16T06:14:20Z

modern/src/maki-interpreter/virtualMachine.js

  let val = gen.next();
  while (!val.done) {
    val = gen.next();
+    if (val.value && isPromise(val.value)) {


Should we just make isPromise support nullable values?

Yea, I was debating whether it should or not, guess it kinda looks like it should hah

captbaritone · 2019-08-16T06:15:03Z

modern/src/maki-interpreter/interpreter.js

-        stack.push(obj[methodName](...methodArgs));
+        const ret = obj[methodName](...methodArgs);
+        if (ret && isPromise(ret)) {
+          stack.push(yield ret);


Does this break the debugger? If so, we can fix in a subsequent PR.

Yea, I think it would break the debugger right now

jberg · 2019-08-16T08:14:55Z

modern/src/App.js

@@ -15,6 +15,8 @@ function useJsUpdates(node) {
  useEffect(() => node.js_listen("js_update", forceUpdate));
 }

+let mouseposition;


Not sure how we want to do this. We need to keep track of the mouse position for some things, but the mousedown events get spammed, and I dont think the changes affect rendering. How do we want to store these?

jberg · 2019-08-16T08:16:35Z

modern/src/maki-interpreter/interpreter.js

@@ -295,7 +312,8 @@ export function* interpret(start, program, stack = []) {
      case 96: {
        const classesOffset = command.arg;
        const Klass = classes[classesOffset];
-        const klassInst = new Klass();
+        const system = variables[0].getValue();
+        const klassInst = new Klass(null, system.scriptGroup);


We are kind of abusing the MAKI object notion of a parent to get a reference to the scriptGroup in Maki Objects incase they need to do something related to the MAKI tree they have a reference

Seems like a reasonable hack. I wonder if we can find an experiment that will tell us what the parent of a created object is.

One thing that's a bit odd here (and I guess is intentional) is that we end up with a node that has another node as its parent but that parent does not have the new node as a child.

All of this will get reconsidered when (if?) we switch to Redux, so I'm okay leaving it as is for now.

Seems like a reasonable hack. I wonder if we can find an experiment that will tell us what the parent of a created object is.

Yea that would be great. PopupMenu is really bizarre because it never actually gets added to any object in the MAKI script, it just gets created and then later popAtMouse runs, so it really doesn't seem to have any parent.

And yea, the tree structure gets kinda weird. hopefully we'll have better ideas of how to do it in the future :D

captbaritone · 2019-08-17T21:14:21Z

modern/src/App.js

+      </li>
+    );
+  });
+  const { x, y } = mouseposition;


I would do this with a useEffect that listens for the document mousemove event and then calls a local setState. If it happens too frequently, we can throttle it.

captbaritone · 2019-08-17T21:15:23Z

modern/src/App.js

+        top: Number(y),
+        left: Number(x),
+        backgroundColor: "#000000",
+        color: "#FFFFFF",


Maybe some // TODO comment here noting that we should come back to this.

captbaritone · 2019-08-17T21:28:00Z

modern/src/maki-interpreter/interpreter.js

@@ -295,7 +312,8 @@ export function* interpret(start, program, stack = []) {
      case 96: {
        const classesOffset = command.arg;
        const Klass = classes[classesOffset];
-        const klassInst = new Klass();
+        const system = variables[0].getValue();
+        const klassInst = new Klass(null, system.scriptGroup);


Seems like a reasonable hack. I wonder if we can find an experiment that will tell us what the parent of a created object is.

captbaritone · 2019-08-17T21:31:50Z

modern/src/maki-interpreter/virtualMachine.js

      // Interpret is a generator that yields before each command is exectued.
      // `handler` is reponsible for `.next()`ing until the program execution is
      // complete (the generator is "done"). In production this is done
      // synchronously. In the debugger, if execution is paused, it's done
      // async.
-      debugHandler(interpret(commandOffset, program, args.reverse()));
+      await debugHandler(interpret(commandOffset, program, args.reverse()));


Do we actually need to await here? I think it would be fine to return form this function without waiting for the promise to resolve.

whoops. tested it and we do not need it, thanks!

captbaritone · 2019-08-17T21:32:02Z

modern/src/runtime/GuiObject.js

  }

  hide() {
    this.visible = false;
+    this.parent.js_trigger("js_update");


captbaritone · 2019-08-17T21:32:15Z

modern/src/runtime/MakiObject.js

@@ -10,7 +10,7 @@ class MakiObject {
    } else {
      // When dynamically creating an object with `new` we have no underlying node
      this.attributes = {};
-      this.name = this.constructor.name.toLowerCase();
+      this.name = this.getclassname().toLowerCase();


captbaritone · 2019-08-17T21:32:37Z

modern/src/runtime/MakiObject.js

@@ -67,7 +71,7 @@ class MakiObject {
   * @ret The class name.
   */
  getclassname() {
-    return "Object";
+    return "MakiObject";


Derp. Thanks!

captbaritone · 2019-08-17T21:33:33Z

modern/src/runtime/PopupMenu.js

+    super(node, parent);
+
+    this.commands = [];
+    this.resolveCmdSelection = null;


There are private properties right? Can they start with _?

yup, thanks!

oh whoops, they are actually referenced in the rendering. I will rename the resolve function with a js_ prefix though

captbaritone · 2019-08-17T21:35:43Z

modern/src/maki-interpreter/interpreter.js

@@ -295,7 +312,8 @@ export function* interpret(start, program, stack = []) {
      case 96: {
        const classesOffset = command.arg;
        const Klass = classes[classesOffset];
-        const klassInst = new Klass();
+        const system = variables[0].getValue();
+        const klassInst = new Klass(null, system.scriptGroup);


One thing that's a bit odd here (and I guess is intentional) is that we end up with a node that has another node as its parent but that parent does not have the new node as a child.

All of this will get reconsidered when (if?) we switch to Redux, so I'm okay leaving it as is for now.

* initial test for generator class functions * return promise from functions that need async * move null check into isPromise and fix condition * implementing popmenu * fix dynamic node creation in prod build * remove unnecessary async/await * rename function with js_ prefix * TODO to fix styles

captbaritone reviewed Aug 16, 2019

View reviewed changes

jberg commented Aug 16, 2019

View reviewed changes

jberg marked this pull request as ready for review August 16, 2019 08:26

jberg added 4 commits August 16, 2019 09:38

initial test for generator class functions

f1d484a

return promise from functions that need async

375cbcb

move null check into isPromise and fix condition

fd2a127

implementing popmenu

7220c88

jberg force-pushed the handle-maki-functions-needing-input branch from 80f3313 to 7220c88 Compare August 16, 2019 16:39

fix dynamic node creation in prod build

8e4de9b

jberg changed the title ~~Handle MAKI functions that need input~~ Handle MAKI functions that need input, implement popupmenu Aug 16, 2019

captbaritone approved these changes Aug 17, 2019

View reviewed changes

jberg and others added 4 commits August 17, 2019 16:21

remove unnecessary async/await

c94dbbb

rename function with js_ prefix

6e2c5e6

TODO to fix styles

a73fca8

Merge branch 'master' into handle-maki-functions-needing-input

09866ff

jberg merged commit 067d4c1 into captbaritone:master Aug 17, 2019

jberg mentioned this pull request Aug 17, 2019

[modern] implement mouse position as an effect #858

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle MAKI functions that need input, implement popupmenu #854

Handle MAKI functions that need input, implement popupmenu #854

jberg commented Aug 15, 2019

captbaritone Aug 16, 2019

jberg Aug 16, 2019

captbaritone Aug 16, 2019

jberg Aug 16, 2019

captbaritone Aug 16, 2019

jberg Aug 16, 2019

jberg Aug 16, 2019

jberg Aug 16, 2019

captbaritone Aug 17, 2019

captbaritone Aug 17, 2019

jberg Aug 17, 2019

captbaritone Aug 17, 2019

captbaritone Aug 17, 2019

jberg Aug 17, 2019

captbaritone Aug 17, 2019

captbaritone Aug 17, 2019

jberg Aug 17, 2019

captbaritone Aug 17, 2019

captbaritone Aug 17, 2019

captbaritone Aug 17, 2019

captbaritone Aug 17, 2019

jberg Aug 17, 2019

jberg Aug 17, 2019

captbaritone Aug 17, 2019

Handle MAKI functions that need input, implement popupmenu #854

Handle MAKI functions that need input, implement popupmenu #854

Conversation

jberg commented Aug 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment