Allow script variables to be provided in arbitrary order

SanderMertens · Dec 25, 2024 · abbf8e3 · abbf8e3
1 parent 6f76b3d
commit abbf8e3
Show file tree

Hide file tree

Showing 10 changed files with 540 additions and 81 deletions.
diff --git a/distr/flecs.c b/distr/flecs.c
@@ -5470,6 +5470,7 @@ typedef struct ecs_script_eval_visitor_t {
     ecs_entity_t with_relationship;
     int32_t with_relationship_sp;
     bool is_with_scope;
+    bool dynamic_variable_binding;
     ecs_script_vars_t *vars;
 } ecs_script_eval_visitor_t;
 
@@ -59796,6 +59797,11 @@ int flecs_script_eval_template(
     template->entity = template_entity;
     template->node = node;
 
+    /* Variables are always presented to a template in a well defined order, so
+     * we don't need dynamic variable binding. */
+    bool old_dynamic_variable_binding = v->dynamic_variable_binding;
+    v->dynamic_variable_binding = false;
+
     if (flecs_script_template_preprocess(v, template)) {
         goto error;
     }
@@ -59808,6 +59814,8 @@ int flecs_script_eval_template(
         goto error;
     }
 
+    v->dynamic_variable_binding = old_dynamic_variable_binding;
+
     /* If template has no props, give template dummy size so we can register
      * hooks for it. */
     if (!ecs_has(v->world, template_entity, EcsComponent)) {
@@ -61626,13 +61634,11 @@ ecs_script_var_t* flecs_script_find_var(
     const char *name,
     int32_t *sp)
 {
-    ecs_assert(sp != NULL, ECS_INTERNAL_ERROR, NULL);
-
-    if (sp[0] != -1) {
+    if (sp && sp[0] != -1) {
         return ecs_script_vars_from_sp(vars, sp[0]);
     } else {
         ecs_script_var_t *var = ecs_script_vars_lookup(vars, name);
-        if (var) {
+        if (var && sp) {
             sp[0] = var->sp;
         }
         return var;
@@ -61658,7 +61664,7 @@ int flecs_script_find_entity(
         }
 
         const ecs_script_var_t *var = flecs_script_find_var(
-            v->vars, &path[1], sp);
+            v->vars, &path[1], v->dynamic_variable_binding ? NULL : sp);
         if (!var) {
             goto error;
         }
@@ -61795,7 +61801,9 @@ int flecs_script_eval_id(
     if (v->template) {
         /* Can't resolve variables while preprocessing template scope */
         if (id->first[0] == '$') {
-            if (flecs_script_find_var(v->vars, &id->first[1], &id->first_sp)) {
+            if (flecs_script_find_var(v->vars, &id->first[1], 
+                v->dynamic_variable_binding ? NULL : &id->first_sp)) 
+            {
                 return 0;
             } else {
                 flecs_script_eval_error(v, node, 
@@ -61804,8 +61812,8 @@ int flecs_script_eval_id(
             }
         }
         if (id->second && id->second[0] == '$') {
-            if (flecs_script_find_var(
-                v->vars, &id->second[1], &id->second_sp))
+            if (flecs_script_find_var(v->vars, &id->second[1], 
+                v->dynamic_variable_binding ? NULL : &id->second_sp))
             {
                 return 0;
             } else {
@@ -61905,7 +61913,8 @@ int flecs_script_eval_expr(
         .lookup_ctx = v,
         .vars = v->vars,
         .type = value->type,
-        .runtime = v->r
+        .runtime = v->r,
+        .disable_dynamic_variable_binding = !v->dynamic_variable_binding
     };
 
     if (expr->type_info == NULL) {
@@ -62290,17 +62299,13 @@ int flecs_script_eval_var_component(
     ecs_script_eval_visitor_t *v,
     ecs_script_var_component_t *node)
 {
-    ecs_script_var_t *var;
-
-    if (node->sp != -1) {
-        var = ecs_script_vars_from_sp(v->vars, node->sp);
-    } else {
-        var = ecs_script_vars_lookup(v->vars, node->name);
-        if (!var) {
-            flecs_script_eval_error(v, node, 
-                "unresolved variable '%s'", node->name);
-            return -1;
-        }
+
+    ecs_script_var_t *var = flecs_script_find_var(
+        v->vars, node->name, v->dynamic_variable_binding ? NULL : &node->sp);
+    if (!var) {
+        flecs_script_eval_error(v, node, 
+            "unresolved variable '%s'", node->name);
+        return -1;
     }
 
     if (v->is_with_scope) {
@@ -62391,19 +62396,8 @@ int flecs_script_eval_with_var(
     ecs_script_eval_visitor_t *v,
     ecs_script_var_component_t *node)
 {
-    ecs_script_var_t *var;
-
-    if (node->sp != -1) {
-        var = ecs_script_vars_from_sp(v->vars, node->sp);
-    } else {
-        var = ecs_script_vars_lookup(v->vars, node->name);
-        if (!var) {
-            flecs_script_eval_error(v, node, 
-                "unresolved variable '%s'", node->name);
-            return -1;
-        }
-    }
-
+    ecs_script_var_t *var = flecs_script_find_var(
+        v->vars, node->name, v->dynamic_variable_binding ? NULL : &node->sp);
     if (!var) {
         flecs_script_eval_error(v, node, 
             "unresolved variable '%s'", node->name);
@@ -62920,6 +62914,11 @@ void flecs_script_eval_visit_init(
         v->vars = flecs_script_vars_push(v->vars, &v->r->stack, a);
         v->vars->parent = desc->vars;
         v->vars->sp = ecs_vec_count(&desc->vars->vars);
+
+        /* When variables are provided to script, don't use cached variable
+         * stack pointers, as the order in which the application provides 
+         * variables may not be the same across evaluations. */
+        v->dynamic_variable_binding = true;
     }
 
     /* Always include flecs.meta */
@@ -77563,7 +77562,8 @@ int flecs_expr_variable_visit_eval(
         "variables available at parse time are not provided");
 
     const ecs_script_var_t *var = flecs_script_find_var(
-        ctx->desc->vars, node->name, &node->sp);
+        ctx->desc->vars, node->name, 
+            ctx->desc->disable_dynamic_variable_binding ? &node->sp : NULL);
     if (!var) {
         flecs_expr_visit_error(ctx->script, node, "unresolved variable '%s'",
             node->name);

diff --git a/distr/flecs.h b/distr/flecs.h
@@ -14853,18 +14853,23 @@ void ecs_script_vars_from_iter(
 typedef struct ecs_expr_eval_desc_t {
     const char *name;                /**< Script name */
     const char *expr;                /**< Full expression string */
+    const ecs_script_vars_t *vars;   /**< Variables accessible in expression */
+    ecs_entity_t type;               /**< Type of parsed value (optional) */
     ecs_entity_t (*lookup_action)(   /**< Function for resolving entity identifiers */
         const ecs_world_t*,
         const char *value,
         void *ctx);
     void *lookup_ctx;                /**< Context passed to lookup function */
-    const ecs_script_vars_t *vars;   /**< Variables accessible in expression */
-    ecs_entity_t type;               /**< Type of parsed value (optional) */
-
-    /* Disable constant folding (slower evaluation, faster parsing) */
+
+    /** Disable constant folding (slower evaluation, faster parsing) */
     bool disable_folding;
-
-    /* Allow for unresolved identifiers when parsing. Useful when entities can
+
+    /** This option instructs the expression runtime to lookup variables by 
+     * stack pointer instead of by name, which improves performance. Only enable 
+     * when provided variables are always declared in the same order. */
+    bool disable_dynamic_variable_binding;
+
+    /** Allow for unresolved identifiers when parsing. Useful when entities can
      * be created in between parsing & evaluating. */
     bool allow_unresolved_identifiers;
 

diff --git a/include/flecs/addons/script.h b/include/flecs/addons/script.h
@@ -539,18 +539,23 @@ void ecs_script_vars_from_iter(
 typedef struct ecs_expr_eval_desc_t {
     const char *name;                /**< Script name */
     const char *expr;                /**< Full expression string */
+    const ecs_script_vars_t *vars;   /**< Variables accessible in expression */
+    ecs_entity_t type;               /**< Type of parsed value (optional) */
     ecs_entity_t (*lookup_action)(   /**< Function for resolving entity identifiers */
         const ecs_world_t*,
         const char *value,
         void *ctx);
     void *lookup_ctx;                /**< Context passed to lookup function */
-    const ecs_script_vars_t *vars;   /**< Variables accessible in expression */
-    ecs_entity_t type;               /**< Type of parsed value (optional) */
-
-    /* Disable constant folding (slower evaluation, faster parsing) */
+
+    /** Disable constant folding (slower evaluation, faster parsing) */
     bool disable_folding;
-
-    /* Allow for unresolved identifiers when parsing. Useful when entities can
+
+    /** This option instructs the expression runtime to lookup variables by 
+     * stack pointer instead of by name, which improves performance. Only enable 
+     * when provided variables are always declared in the same order. */
+    bool disable_dynamic_variable_binding;
+
+    /** Allow for unresolved identifiers when parsing. Useful when entities can
      * be created in between parsing & evaluating. */
     bool allow_unresolved_identifiers;
 

diff --git a/src/addons/script/expr/visit_eval.c b/src/addons/script/expr/visit_eval.c
@@ -325,7 +325,8 @@ int flecs_expr_variable_visit_eval(
         "variables available at parse time are not provided");
 
     const ecs_script_var_t *var = flecs_script_find_var(
-        ctx->desc->vars, node->name, &node->sp);
+        ctx->desc->vars, node->name, 
+            ctx->desc->disable_dynamic_variable_binding ? &node->sp : NULL);
     if (!var) {
         flecs_expr_visit_error(ctx->script, node, "unresolved variable '%s'",
             node->name);

diff --git a/src/addons/script/template.c b/src/addons/script/template.c
@@ -516,6 +516,11 @@ int flecs_script_eval_template(
     template->entity = template_entity;
     template->node = node;
 
+    /* Variables are always presented to a template in a well defined order, so
+     * we don't need dynamic variable binding. */
+    bool old_dynamic_variable_binding = v->dynamic_variable_binding;
+    v->dynamic_variable_binding = false;
+
     if (flecs_script_template_preprocess(v, template)) {
         goto error;
     }
@@ -528,6 +533,8 @@ int flecs_script_eval_template(
         goto error;
     }
 
+    v->dynamic_variable_binding = old_dynamic_variable_binding;
+
     /* If template has no props, give template dummy size so we can register
      * hooks for it. */
     if (!ecs_has(v->world, template_entity, EcsComponent)) {

diff --git a/src/addons/script/visit_eval.c b/src/addons/script/visit_eval.c
@@ -134,13 +134,11 @@ ecs_script_var_t* flecs_script_find_var(
     const char *name,
     int32_t *sp)
 {
-    ecs_assert(sp != NULL, ECS_INTERNAL_ERROR, NULL);
-
-    if (sp[0] != -1) {
+    if (sp && sp[0] != -1) {
         return ecs_script_vars_from_sp(vars, sp[0]);
     } else {
         ecs_script_var_t *var = ecs_script_vars_lookup(vars, name);
-        if (var) {
+        if (var && sp) {
             sp[0] = var->sp;
         }
         return var;
@@ -166,7 +164,7 @@ int flecs_script_find_entity(
         }
 
         const ecs_script_var_t *var = flecs_script_find_var(
-            v->vars, &path[1], sp);
+            v->vars, &path[1], v->dynamic_variable_binding ? NULL : sp);
         if (!var) {
             goto error;
         }
@@ -303,7 +301,9 @@ int flecs_script_eval_id(
     if (v->template) {
         /* Can't resolve variables while preprocessing template scope */
         if (id->first[0] == '$') {
-            if (flecs_script_find_var(v->vars, &id->first[1], &id->first_sp)) {
+            if (flecs_script_find_var(v->vars, &id->first[1], 
+                v->dynamic_variable_binding ? NULL : &id->first_sp)) 
+            {
                 return 0;
             } else {
                 flecs_script_eval_error(v, node, 
@@ -312,8 +312,8 @@ int flecs_script_eval_id(
             }
         }
         if (id->second && id->second[0] == '$') {
-            if (flecs_script_find_var(
-                v->vars, &id->second[1], &id->second_sp))
+            if (flecs_script_find_var(v->vars, &id->second[1], 
+                v->dynamic_variable_binding ? NULL : &id->second_sp))
             {
                 return 0;
             } else {
@@ -413,7 +413,8 @@ int flecs_script_eval_expr(
         .lookup_ctx = v,
         .vars = v->vars,
         .type = value->type,
-        .runtime = v->r
+        .runtime = v->r,
+        .disable_dynamic_variable_binding = !v->dynamic_variable_binding
     };
 
     if (expr->type_info == NULL) {
@@ -798,17 +799,13 @@ int flecs_script_eval_var_component(
     ecs_script_eval_visitor_t *v,
     ecs_script_var_component_t *node)
 {
-    ecs_script_var_t *var;
-
-    if (node->sp != -1) {
-        var = ecs_script_vars_from_sp(v->vars, node->sp);
-    } else {
-        var = ecs_script_vars_lookup(v->vars, node->name);
-        if (!var) {
-            flecs_script_eval_error(v, node, 
-                "unresolved variable '%s'", node->name);
-            return -1;
-        }
+
+    ecs_script_var_t *var = flecs_script_find_var(
+        v->vars, node->name, v->dynamic_variable_binding ? NULL : &node->sp);
+    if (!var) {
+        flecs_script_eval_error(v, node, 
+            "unresolved variable '%s'", node->name);
+        return -1;
     }
 
     if (v->is_with_scope) {
@@ -899,19 +896,8 @@ int flecs_script_eval_with_var(
     ecs_script_eval_visitor_t *v,
     ecs_script_var_component_t *node)
 {
-    ecs_script_var_t *var;
-
-    if (node->sp != -1) {
-        var = ecs_script_vars_from_sp(v->vars, node->sp);
-    } else {
-        var = ecs_script_vars_lookup(v->vars, node->name);
-        if (!var) {
-            flecs_script_eval_error(v, node, 
-                "unresolved variable '%s'", node->name);
-            return -1;
-        }
-    }
-
+    ecs_script_var_t *var = flecs_script_find_var(
+        v->vars, node->name, v->dynamic_variable_binding ? NULL : &node->sp);
     if (!var) {
         flecs_script_eval_error(v, node, 
             "unresolved variable '%s'", node->name);
@@ -1428,6 +1414,11 @@ void flecs_script_eval_visit_init(
         v->vars = flecs_script_vars_push(v->vars, &v->r->stack, a);
         v->vars->parent = desc->vars;
         v->vars->sp = ecs_vec_count(&desc->vars->vars);
+
+        /* When variables are provided to script, don't use cached variable
+         * stack pointers, as the order in which the application provides 
+         * variables may not be the same across evaluations. */
+        v->dynamic_variable_binding = true;
     }
 
     /* Always include flecs.meta */

diff --git a/src/addons/script/visit_eval.h b/src/addons/script/visit_eval.h
@@ -18,6 +18,7 @@ typedef struct ecs_script_eval_visitor_t {
     ecs_entity_t with_relationship;
     int32_t with_relationship_sp;
     bool is_with_scope;
+    bool dynamic_variable_binding;
     ecs_script_vars_t *vars;
 } ecs_script_eval_visitor_t;
 

diff --git a/test/script/project.json b/test/script/project.json
@@ -277,6 +277,12 @@
                 "assign_call_scoped_func_w_using",
                 "eval_w_vars",
                 "eval_w_runtime",
+                "eval_w_other_vars",
+                "eval_w_vars_different_order",
+                "eval_w_vars_different_order_var_component",
+                "eval_w_vars_different_order_with_var",
+                "eval_w_vars_different_order_pair_w_var",
+                "eval_w_vars_different_order_pair_scope_w_var",
                 "component_in_entity_in_with_scope",
                 "entity_w_string_name",
                 "entity_w_interpolated_name",