Drivers UI: Added name validation/linting for Driver Variables
authorJoshua Leung <aligorith@gmail.com>
Thu, 24 Mar 2016 02:15:04 +0000 (15:15 +1300)
committerJoshua Leung <aligorith@gmail.com>
Thu, 24 Mar 2016 02:15:04 +0000 (15:15 +1300)
When attempting to change a driver variable name to an "invalid" name,
an indicator will now be shown beside the offending variable name.
Clicking on this icon will show a popup which provides more information
about why the variable name cannot be used.

Reasons that it knows about are:
1) Starts with number
2) Has a dot
3) Has a space
4) Starts with or contains a special character
5) Starts with an underscore (Python does allow this, but it's bad practice,
   and makes checking security of drivers harder)
6) Is a reserved Python keyword

source/blender/blenkernel/BKE_fcurve.h
source/blender/blenkernel/intern/fcurve.c
source/blender/editors/space_graph/graph_buttons.c
source/blender/makesdna/DNA_anim_types.h
source/blender/makesrna/intern/rna_fcurve.c

index 443a03a475ade5cd1d1153db3dffe55e01d0efb1..a1176b913129423b0fb9974882f09beca52ce0f5 100644 (file)
@@ -95,6 +95,7 @@ struct ChannelDriver *fcurve_copy_driver(struct ChannelDriver *driver);
 
 void driver_free_variable(struct ChannelDriver *driver, struct DriverVar *dvar);
 void driver_change_variable_type(struct DriverVar *dvar, int type);
+void driver_variable_name_validate(struct DriverVar *dvar);
 struct DriverVar *driver_add_new_variable(struct ChannelDriver *driver);
 
 float driver_get_variable_value(struct ChannelDriver *driver, struct DriverVar *dvar);
index abf847274c4adfe273bb97997992e72e6a71110a..e2b1acddc9c17221ceb652f8e7b390d68a14ec7d 100644 (file)
@@ -1593,6 +1593,69 @@ void driver_change_variable_type(DriverVar *dvar, int type)
        DRIVER_TARGETS_LOOPER_END
 }
 
+/* Validate driver name (after being renamed) */
+void driver_variable_name_validate(DriverVar *dvar)
+{
+       /* Special character blacklist */
+       const char special_char_blacklist[] = {
+               '~', '`', '!', '@', '#', '$', '%', '^', '&', '*', '+', '=', '-',
+           '/', '\\', '?', ':', ';',  '<', '>', '{', '}', '[', ']', '|',
+               ' ', '.', '\t', '\n', '\r'
+       };
+       
+       /* sanity checks */
+       if (dvar == NULL)
+               return;
+       
+       /* clear all invalid-name flags */
+       dvar->flag &= ~DVAR_ALL_INVALID_FLAGS;
+       
+       /* 1) Must start with a letter */
+       /* XXX: We assume that valid unicode letters in other languages are ok too, hence the blacklisting */
+       if (ELEM(dvar->name[0], '0', '1', '2', '3', '4', '5', '6', '7', '8', '9')) {
+               dvar->flag |= DVAR_FLAG_INVALID_START_NUM;
+       }
+       else if (dvar->name[0] == '_') {
+               /* NOTE: We don't allow names to start with underscores (i.e. it helps when ruling out security risks) */
+               dvar->flag |= DVAR_FLAG_INVALID_START_CHAR;
+       }
+       
+       /* 2) Must not contain invalid stuff in the middle of the string */
+       if (strchr(dvar->name, ' ')) {
+               dvar->flag |= DVAR_FLAG_INVALID_HAS_SPACE;
+       }
+       if (strchr(dvar->name, '.')) {
+               dvar->flag |= DVAR_FLAG_INVALID_HAS_DOT;
+       }
+       
+       /* 3) Check for special characters - Either at start, or in the middle */
+       for (int i = 0; i < sizeof(special_char_blacklist); i++) {
+               char *match = strchr(dvar->name, special_char_blacklist[i]);
+               
+               if (match == dvar->name)
+                       dvar->flag |= DVAR_FLAG_INVALID_START_CHAR;
+               else if (match != NULL)
+                       dvar->flag |= DVAR_FLAG_INVALID_HAS_SPECIAL;
+       }
+       
+       /* 4) Check if the name is a reserved keyword
+        * NOTE: These won't confuse Python, but it will be impossible to use the variable
+        *       in an expression without Python misinterpreting what these are for
+        */
+       if (STREQ(dvar->name, "if") || STREQ(dvar->name, "elif") || STREQ(dvar->name, "else") ||
+           STREQ(dvar->name, "for") || STREQ(dvar->name, "while") || STREQ(dvar->name, "def") ||
+           STREQ(dvar->name, "True") || STREQ(dvar->name, "False") || STREQ(dvar->name, "import") ||
+           STREQ(dvar->name, "pass")  || STREQ(dvar->name, "with"))
+       {
+               dvar->flag |= DVAR_FLAG_INVALID_PY_KEYWORD;
+       }
+       
+       
+       /* If any these conditions match, the name is invalid */
+       if (dvar->flag & DVAR_ALL_INVALID_FLAGS)
+               dvar->flag |= DVAR_FLAG_INVALID_NAME;
+}
+
 /* Add a new driver variable */
 DriverVar *driver_add_new_variable(ChannelDriver *driver)
 {
@@ -1619,7 +1682,7 @@ DriverVar *driver_add_new_variable(ChannelDriver *driver)
        if (driver->type == DRIVER_TYPE_PYTHON)
                driver->flag |= DRIVER_FLAG_RENAMEVAR;
 #endif
-
+       
        /* return the target */
        return dvar;
 }
index 1bab7bd349c93816ad63e6d10ec3bc3e87f38141..858baf61576e1b33142e93851e80745a722b19e4 100644 (file)
@@ -519,6 +519,39 @@ static void driver_delete_var_cb(bContext *UNUSED(C), void *driver_v, void *dvar
        driver_free_variable(driver, dvar);
 }
 
+/* callback to report why a driver variable is invalid */
+static void driver_dvar_invalid_name_query_cb(bContext *C, void *dvar_v, void *UNUSED(arg))
+{
+       uiPopupMenu *pup = UI_popup_menu_begin(C, CTX_IFACE_(BLT_I18NCONTEXT_OPERATOR_DEFAULT, "Invalid Variable Name"), ICON_NONE);
+       uiLayout *layout = UI_popup_menu_layout(pup);
+       
+       DriverVar *dvar = (DriverVar *)dvar_v;
+       
+       if (dvar->flag & DVAR_FLAG_INVALID_START_NUM) {
+               uiItemL(layout, "It cannot start with a number", ICON_ERROR);
+       }
+       if (dvar->flag & DVAR_FLAG_INVALID_START_CHAR) {
+               uiItemL(layout, 
+                       "It cannot start with a special character,"
+                       " including '$', '@', '!', '~', '+', '-', '_', '.', or ' '",
+                               ICON_NONE);
+       }
+       if (dvar->flag & DVAR_FLAG_INVALID_HAS_SPACE) {
+               uiItemL(layout, "It cannot contain spaces (e.g. 'a space')", ICON_ERROR);
+       }
+       if (dvar->flag & DVAR_FLAG_INVALID_HAS_DOT) {
+               uiItemL(layout, "It cannot contain dots (e.g. 'a.dot')", ICON_ERROR);
+       }
+       if (dvar->flag & DVAR_FLAG_INVALID_HAS_SPECIAL) {
+               uiItemL(layout, "It cannot contain special (non-alphabetical/numeric) characters", ICON_ERROR);
+       }
+       if (dvar->flag & DVAR_FLAG_INVALID_PY_KEYWORD) {
+               uiItemL(layout, "It cannot be a reserved keyword in Python", ICON_INFO);
+       }
+       
+       UI_popup_menu_end(C, pup);
+}
+
 /* callback to reset the driver's flags */
 static void driver_update_flags_cb(bContext *UNUSED(C), void *fcu_v, void *UNUSED(arg))
 {
@@ -816,8 +849,16 @@ static void graph_panel_drivers(const bContext *C, Panel *pa)
                /* variable name */
                uiItemR(row, &dvar_ptr, "name", 0, "", ICON_NONE);
                
-               /* remove button */
+               /* invalid name? */
                UI_block_emboss_set(block, UI_EMBOSS_NONE);
+               
+               if (dvar->flag & DVAR_FLAG_INVALID_NAME) {
+                       but = uiDefIconBut(block, UI_BTYPE_BUT, B_IPO_DEPCHANGE, ICON_ERROR, 290, 0, UI_UNIT_X, UI_UNIT_Y,
+                                          NULL, 0.0, 0.0, 0.0, 0.0, IFACE_("Invalid variable name. Click here for details"));
+                       UI_but_func_set(but, driver_dvar_invalid_name_query_cb, dvar, NULL); // XXX: reports?
+               }
+               
+               /* remove button */
                but = uiDefIconBut(block, UI_BTYPE_BUT, B_IPO_DEPCHANGE, ICON_X, 290, 0, UI_UNIT_X, UI_UNIT_Y,
                                   NULL, 0.0, 0.0, 0.0, 0.0, IFACE_("Delete target variable"));
                UI_but_func_set(but, driver_delete_var_cb, driver, dvar);
index d80b50945df07aadf06c59bed1cc052512d4cbf3..fc32bbd1e99e08543c95b6a0c94b867479ecb2c2 100644 (file)
@@ -327,13 +327,15 @@ typedef enum eDriverTarget_TransformChannels {
  */
 typedef struct DriverVar {
        struct DriverVar *next, *prev;
-
+       
        char name[64];              /* name of the variable to use in py-expression (must be valid python identifier) - MAX_ID_NAME-2 */
-
+       
        DriverTarget targets[8];    /* MAX_DRIVER_TARGETS, target slots */
-       short num_targets;          /* number of targets actually used by this variable */
-
-       short type;                 /* type of driver target (eDriverTarget_Types) */
+       
+       char num_targets;           /* number of targets actually used by this variable */
+       char type;                  /* type of driver variable (eDriverVar_Types) */
+       
+       short flag;                 /* validation tags, etc. (eDriverVar_Flags) */
        float curval;               /* result of previous evaluation */
 } DriverVar;
 
@@ -355,6 +357,38 @@ typedef enum eDriverVar_Types {
        MAX_DVAR_TYPES
 } eDriverVar_Types;
 
+/* Driver Variable Flags */
+typedef enum eDriverVar_Flags {
+       /* variable is not set up correctly */
+       DVAR_FLAG_ERROR               = (1 << 0),
+       
+       /* variable name doesn't pass the validation tests */
+       DVAR_FLAG_INVALID_NAME        = (1 << 1),
+       /* name starts with a number */
+       DVAR_FLAG_INVALID_START_NUM   = (1 << 2),
+       /* name starts with a special character (!, $, @, #, _, etc.) */
+       DVAR_FLAG_INVALID_START_CHAR  = (1 << 3),
+       /* name contains a space */
+       DVAR_FLAG_INVALID_HAS_SPACE   = (1 << 4),
+       /* name contains a dot */
+       DVAR_FLAG_INVALID_HAS_DOT     = (1 << 5),
+       /* name contains invalid chars */
+       DVAR_FLAG_INVALID_HAS_SPECIAL = (1 << 6),
+       /* name is a reserved keyword */
+       DVAR_FLAG_INVALID_PY_KEYWORD  = (1 << 7),
+} eDriverVar_Flags;
+
+/* All invalid dvar name flags */
+#define DVAR_ALL_INVALID_FLAGS (   \
+       DVAR_FLAG_INVALID_NAME |       \
+       DVAR_FLAG_INVALID_START_NUM | \
+       DVAR_FLAG_INVALID_START_CHAR | \
+       DVAR_FLAG_INVALID_HAS_SPACE |  \
+       DVAR_FLAG_INVALID_HAS_DOT |    \
+       DVAR_FLAG_INVALID_HAS_SPECIAL |  \
+       DVAR_FLAG_INVALID_PY_KEYWORD  \
+)
+
 /* --- */
 
 /* Channel Driver (i.e. Drivers / Expressions) (driver)
index fbc332f56582d8125715812909eecfe3d4d0e5fd..c34d8125367ac965f87b71606c3c9e94d5904017 100644 (file)
@@ -275,6 +275,15 @@ static void rna_DriverVariable_type_set(PointerRNA *ptr, int value)
        driver_change_variable_type(dvar, value);
 }
 
+void rna_DriverVariable_name_set(PointerRNA *ptr, const char *value)
+{
+       DriverVar *data = (DriverVar *)(ptr->data);
+       
+       BLI_strncpy_utf8(data->name, value, 64);
+       driver_variable_name_validate(data);
+}
+
+
 /* ----------- */
 
 static DriverVar *rna_Driver_new_variable(ChannelDriver *driver)
@@ -1474,6 +1483,7 @@ static void rna_def_drivervar(BlenderRNA *brna)
        /* Variable Name */
        prop = RNA_def_property(srna, "name", PROP_STRING, PROP_NONE);
        RNA_def_struct_name_property(srna, prop);
+       RNA_def_property_string_funcs(prop, NULL, NULL, "rna_DriverVariable_name_set");
        RNA_def_property_ui_text(prop, "Name",
                                 "Name to use in scripted expressions/functions (no spaces or dots are allowed, "
                                 "and must start with a letter)");
@@ -1493,6 +1503,12 @@ static void rna_def_drivervar(BlenderRNA *brna)
        RNA_def_property_collection_sdna(prop, NULL, "targets", "num_targets");
        RNA_def_property_struct_type(prop, "DriverTarget");
        RNA_def_property_ui_text(prop, "Targets", "Sources of input data for evaluating this variable");
+       
+       /* Name Validity Flags */
+       prop = RNA_def_property(srna, "is_name_valid", PROP_BOOLEAN, PROP_NONE);
+       RNA_def_property_boolean_negative_sdna(prop, NULL, "flag", DVAR_FLAG_INVALID_NAME);
+       RNA_def_property_clear_flag(prop, PROP_EDITABLE);
+       RNA_def_property_ui_text(prop, "Is Name Valid", "Is this a valid name for a driver variable");  
 }