usability issues with Variable Set

Is there an existing issue for this?

[X] I have searched the existing issues

Problem description

First, I believe I need to report this here, as I'm using ondsel as a distribution, and from the limited release notes I can find this feature has not upstreamed yet. Please help me understand how I could have easily figured this out otherwise if this is not true :)

Second - this feature is a MAJOR improvement from the spreadsheet way - really fantastic to see Ondsel helping push things in this direction (both because it's objectively easier - create a part-global variable as one progresses through the model, versus having to bounce back to a separate spreadsheet.

My issues:

once I create the variable, there is not any way to figure out what geometry / constraints reference it. IE if I accidentally define a concept while building more than one part/body that will end up in an assembly, I'm sort of "up a creek" to de-duplicate them, as I have to pull up everything in all possible sketches & features to find uses to figure out which one to keep.
when trying to delete a body, the program fails silently to delete it if it uses a variable. In my reproduction, I started down a body that included a sweep, defined one or more variables relevant for design intent of the [later] assembly as I went. But I changed my mind on how to properly make it partway through, so just started a new body. ** But when I went to delete the first attempt later, nothing happened. Painstakingly I went in and found what variables it referenced, and deleted them from the the Variable set's property view. Then deleted that old body again (which succeeded, body is gone)
- ** Note: Later I ran into item 1 above as I couldn't remember which body was using which variable as I accidentally created duplicates

Thank you.

Full version info

OS: Ubuntu 22.04.4 LTS (i3/i3)
Word size of Ondsel: 64-bit
Version: 2024.2.0.37191 (Git) AppImage
Build type: Release
Branch: (HEAD detached at 2024.2.0)
Hash: 2ad5fb327d068ae6280cf702d528b3a6e35770a7
Python 3.11.9, Qt 5.15.13, Coin 4.0.2, Vtk 9.2.6, OCC 7.7.2
Locale: English/United States (en_US)
Installed mods: 
  * Ondsel-Lens 2024.1.22.01

Subproject(s) affected?

Other (specify in description)

Anything else?

No response

Code of Conduct

[X] I agree to follow this project's Code of Conduct

I'm also having some different VarSet issues. Given the title I thought this could fall under here, and I think there's some potential relation.

I was trying to use them in reference to another and it kept giving the value of 0 and would show as blue in the window. It would first show the proper value, but then after clicking ok it changes to 0. Trying to edit again shows the correct referenced value but continues to display 0.

I also tried clicking another variable as reference and this didn't work and suddenly I lost my window and can't seem to be able to get it back. The worst part about this is that if I go try to manually edit a variable (e.g. clicking a dimension) I am not able to adjust this value and no typing works. If I click it can still give the appearance of being selected while it isn't (this is how I accidentally hit some hotkeys and caused the aforementioned problem). If I click the blue expression button Variable Set shows Unnamed while it was previously Base, Group is empty, as is New Property but the text of Result shows the proper value and that bottom unlabeled box where you place in dimensions has the correct VarSet name (e.g. VarSet.Height). This one is modifiable. But something seems way off here as this is the dimension where this value is actually defined.

It appears that there is no documentation. The search in the help menu yields no results for "var" ("va" only shows "validate sketch"). Some documentation about this feature would be really nice because as @m10d has said, this is a GREAT feature.

I'd also like to note a minor quality of life improvement that could be fixed quickly. If you do something like 180/2 + 1mm it'll give an error when dimensioning because of "Unit mismatch". The "obvious" solution of (180/2 + 1)mm fails to parse. The solution ends up being 180mm/2 + 1mm which is needlessly cumbersome. Parentheses should definitely be able to be parsed as, other basic math operations.

Thank you for reporting, this helps us a lot!

First, I believe I need to report this here, as I'm using ondsel as a distribution, and from the limited release notes I can find this feature has not upstreamed yet. Please help me understand how I could have easily figured this out otherwise if this is not true :)

Indeed, it has not been upstreamed yet, but it is pending: #13317 and #13626. So, I would agree this is currently the right place to report.

Second - this feature is a MAJOR improvement from the spreadsheet way - really fantastic to see Ondsel helping push things in this direction (both because it's objectively easier - create a part-global variable as one progresses through the model, versus having to bounce back to a separate spreadsheet.

Thanks, that very motivating to hear!

My issues:

once I create the variable, there is not any way to figure out what geometry / constraints reference it. IE if I accidentally define a concept while building more than one part/body that will end up in an assembly, I'm sort of "up a creek" to de-duplicate them, as I have to pull up everything in all possible sketches & features to find uses to figure out which one to keep.

That's a good point but this is a problem that would exist with spreadsheets as well. In programming you have the concept of def-use chains that is a bit similar, given a definition (the created variable), where are all the uses of that definition, which also takes transitivity into account, so for example:

VarSet001: Height = 10mm
VarSet002: Length = 2*Height
Pad: Length = VarSet002.Length

Now, if you want to know where VarSet001.Height is used, the use-def analysis would report: VarSet002.Length and Pad.Length. A good target for such analysis would be the Property Manager that I introduced in #13112 but is currently in draft because it is a big change in FreeCAD.

I think a next step would be to create an issue in the FreeCAD repository for this. The issue could reference this issue. I can create the issue but you can yourself too.

For more context and the reasons behind the Property Manager: if we do proper parameterized design, I foresee a need to be able to "refactor" the parameters as well. For example, you come to the conclusion that the name Length was useful initially, but on improving the design, you notice that Length should become LengthBox because you also have a cylinder with a length that you want to call LengthCylinder, for example. So, operations such as renaming, moving between VarSets, find the uses, I consider that all to be important for an improved workflow for parameterized design.

when trying to delete a body, the program fails silently to delete it if it uses a variable. In my reproduction, I started down a body that included a sweep, defined one or more variables relevant for design intent of the [later] assembly as I went. But I changed my mind on how to properly make it partway through, so just started a new body. ** But when I went to delete the first attempt later, nothing happened. Painstakingly I went in and found what variables it referenced, and deleted them from the the Variable set's property view. Then deleted that old body again (which succeeded, body is gone)

** Note: Later I ran into item 1 above as I couldn't remember which body was using which variable as I accidentally created duplicates

I doubt that this is caused by the VarSets because they are very simple and very similar to other document objects, so deleting if a property would be referenced would be a general problem. I can also not reproduce this. Could you perhaps prepare a file in which I can see this behavior?

I'm also having some different VarSet issues. Given the title I thought this could fall under here, and I think there's some potential relation.

Great, thanks for reporting!

I was trying to use them in reference to another and it kept giving the value of 0 and would show as blue in the window. It would first show the proper value, but then after clicking ok it changes to 0. Trying to edit again shows the correct referenced value but continues to display 0.

I have seen this behavior and I believe it to be caused by the FreeCAD logic not doing a refresh on the document object. I believe this is something that is a problem in general and not specific to VarSets. I will take a closer look at this problem and either create an issue for this or fix it for VarSets if it turns out that it is only in the scope of VarSets.

I also tried clicking another variable as reference and this didn't work and suddenly I lost my window and can't seem to be able to get it back. The worst part about this is that if I go try to manually edit a variable (e.g. clicking a dimension) I am not able to adjust this value and no typing works. If I click it can still give the appearance of being selected while it isn't (this is how I accidentally hit some hotkeys and caused the aforementioned problem). If I click the blue expression button Variable Set shows Unnamed while it was previously Base, Group is empty, as is New Property but the text of Result shows the proper value and that bottom unlabeled box where you place in dimensions has the correct VarSet name (e.g. VarSet.Height). This one is modifiable. But something seems way off here as this is the dimension where this value is actually defined.

Right, these issues are FreeCAD quirks. I don't notice anymore because I'm used to it, but I think I understand the problem. I'll take a look in how to improve this.

It appears that there is no documentation. The search in the help menu yields no results for "var" ("va" only shows "validate sketch"). Some documentation about this feature would be really nice because as @m10d has said, this is a GREAT feature.

Yes, that's right. There is no documentation yet. I will look into that as well. Great to hear that you like the feature!

I'd also like to note a minor quality of life improvement that could be fixed quickly. If you do something like 180/2 + 1mm it'll give an error when dimensioning because of "Unit mismatch". The "obvious" solution of (180/2 + 1)mm fails to parse. The solution ends up being 180mm/2 + 1mm which is needlessly cumbersome. Parentheses should definitely be able to be parsed as, other basic math operations.

Right, that is a more general issue that is not related to VarSets per se. You could make an issue on the FreeCAD repo if you want. I can do that as well if you want.

So there's a partial "fix" for the variables that turn to 0 that I've realized and I had forgotten to update this for others that are having this issue.

The problem appears to be in the variable creation. So if you right-click on VarSet in the tree, then Properties you will likely see your defined variable as 0 instead of number you've defined. If you then change the value here it seems to be persistent.

For people using VarSets I'm finding that a useful workflow ends up being by first starting with defining the VarSet when you start your project. Define all (or all expected) values (use Add another for ease). Then check the values using the above VarSet -> Properties, checking for zeros. Then start drawing and dimensioning. I find that there are fewer errors when defining variables out of dimension time. So if you need a new variable defined draw it but don't dimension, exit out of sketch (pad, etc), modify VarSet, come back and dimension.

@pieterhijma, after some more use, I think there could be some useful improvements that shouldn't be too hard to implement.

1) Make variables easier to select from list. The most useful case is probably when doing dimensioning as you might want to just select from the VarSet. In the ExpressionEditor you can already get a dropdown list of suggestions when you start typing (e.g. you start typing << and you see a list of defined values). This dropdown should appear when clicking on the empty value (I notice that the suggestion list disappears if you click away and then click back). Essentially a Select from VarSet method.

2) Clarify language in the Expression editor. It seems like there are two "views" of this (shown below). My best understanding is that when Show variable sets is enabled that what's actually intended is Add to VarSet

Enabled	Disabled

3) Hierarchical structure. I think this could be user preference so aliasing might be best for this but <<VarSet>>.Property could alias to <<VarSet>>.Group.Property (or <<VarSet>>.<<Group>>.Property?). I think what might be most useful here is if the non-aliasing method was used you could make very reasonable naming structures like <<VarSet>>.Subpart0.Width and <<VarSet>>.Subpart1.Width, where currently you wouldn't be able to do this because Width will be overloaded. I suspect many users will expect this to be the behavior since it is a logical step to think that a Property Group is not just an organizational display tool but an actual hierarchical organization. I suspect this would be a more common interpretation, but maybe I'm wrong here.

I don't know how others feel about this but I think these were things I felt were unnatural and are (minor) frustrations when designing.

So there's a partial "fix" for the variables that turn to 0 that I've realized and I had forgotten to update this for others that are having this issue.

The problem appears to be in the variable creation. So if you right-click on VarSet in the tree, then Properties you will likely see your defined variable as 0 instead of number you've defined. If you then change the value here it seems to be persistent.

@stevenwalton, I don't think I have this problem at all. After a name and type have been determined, the property is created with a default value, often zero. Then if you give it a value, the value should be set to that value. The GUI may not update immediately, but this is a general FreeCAD problem and after a refresh on the document, everything should be stable. Perhaps you can send me a brief screencast?

For people using VarSets I'm finding that a useful workflow ends up being by first starting with defining the VarSet when you start your project. Define all (or all expected) values (use Add another for ease). Then check the values using the above VarSet -> Properties, checking for zeros. Then start drawing and dimensioning. I find that there are fewer errors when defining variables out of dimension time. So if you need a new variable defined draw it but don't dimension, exit out of sketch (pad, etc), modify VarSet, come back and dimension.

@pieterhijma, after some more use, I think there could be some useful improvements that shouldn't be too hard to implement.

Make variables easier to select from list. The most useful case is probably when doing dimensioning as you might want to just select from the VarSet. In the ExpressionEditor you can already get a dropdown list of suggestions when you start typing (e.g. you start typing << and you see a list of defined values). This dropdown should appear when clicking on the empty value (I notice that the suggestion list disappears if you click away and then click back). Essentially a Select from VarSet method.

Right, indeed, this could be a useful feature. However, every object in FreeCAD has properties, so, in total, there are many properties to select from. You would want to give preference to the properties defined in VarSets then?

Clarify language in the Expression editor. It seems like there are two "views" of this (shown below). My best understanding is that when Show variable sets is enabled that what's actually intended is Add to VarSet

Yes, indeed, that is much better.

Enabled Disabled

Hierarchical structure. I think this could be user preference so aliasing might be best for this but <<VarSet>>.Property could alias to <<VarSet>>.Group.Property (or <<VarSet>>.<<Group>>.Property?). I think what might be most useful here is if the non-aliasing method was used you could make very reasonable naming structures like <<VarSet>>.Subpart0.Width and <<VarSet>>.Subpart1.Width, where currently you wouldn't be able to do this because Width will be overloaded. I suspect many users will expect this to be the behavior since it is a logical step to think that a Property Group is not just an organizational display tool but an actual hierarchical organization. I suspect this would be a more common interpretation, but maybe I'm wrong here.

Right, got it. I'm not too happy about the groups as well. There is also the prefix option but that doesn't create an actual hierarchy, it is simply prefixing. I think this would be a major change in FreeCAD because it is in the core and lots of logic in the rest of the program is adapted to this. Changing it would probably also break backward compatibility. I think it would be good to have a discussion about groups.

I don't know how others feel about this but I think these were things I felt were unnatural and are (minor) frustrations when designing.

Thank you for reporting! It is good to be aware of this and I'm sure we can improve. VarSets is a new feature, so I expect lots of frustrations and potential improvements.

Regarding the "Variables set to 0" thing, it would be nice if VarSet dialog set a property to "Read only" between the creation step and when the user clicks OK button. My brain is square and sometimes I forget to click OK in the VarSet dialog, then proceed to edit the variable in the Property view.

This happens a lot because I use a "focus follows mouse" policy that changes window focus a lot, and VarSet dialog will create variables when it loses focus. This can also trigger an annoying error dialog if I move my mouse off the VarSet dialog before entering a proper name, but I wouldn't call that a bug.

Overall I'm quite happy with VarSets so far and understand that FreeCAD has a lot of catch-up to do before we can stop relying on spreadsheets. Thanks for all you've accomplished so far!

There happens to be a bug on that one point, https://github.com/FreeCAD/FreeCAD/issues/14787

Right, thanks! Indeed, it looks similar and it also explains why I don't seem to be able to replicate it.

Okay, I think I found some serious bugs while trying to make this comment. So forgive me if it is a bit disorganized as I was constantly editing while updating and new crashes were happening. I'm pretty confident a lot of this is related and that there are some serious bugs in the VarSet Property box. I worry that this is going to be a rabbit hole.

I don't think I have this problem at all. After a name and type have been determined, the property is created with a default value, often zero. Then if you give it a value, the value should be set to that value. The GUI may not update immediately, but this is a general FreeCAD problem and after a refresh on the document, everything should be stable. Perhaps you can send me a brief screencast?

VarSet Variable Resetting When Editing

I have a hard time reproducing it and it seems random but generally when I am adding multiple variables before I do any sketches and it is usually the first one. So it could be a user error or it could be a weird combination. ~~Next time it happens I'll try to report right away and see if I can fully reproduce and capture a video.~~ While testing I found a potentially related one? I created an inverse length variable and the variable sets. But when I click the variable in the Property View it immediately changes to 0. In this gif you can also see that I click on Group: Base to reveal Value and then I click on Value's box but my cursor goes to the Type box instead[^1]. And after you see the magic 0, I try to press undo and you can see it wipes the variable instead of going back like what happens when you manually change variables.

[^1]: I think hiding the value field really just results in users having to do more clicks. Since this always results in needing to make an intermediate click. Probably better to simply make the box nonmodifiable for when/if there is a property that does not take a value. With the standard associated visual clue of graying out that input field.

invlen

Type Errors

I also noticed some odd things. I was recently trying out some of the other variable types. I believe App::PropertyPercent is in error. This seems equivalent to an Integer type. So if you were trying to do something like create an offset by <<VarSet>>.OffsetPercent * <<VarSet>>.MyLength you'll get 100x what you'd be expecting. I think users are likely to expect that the value here (which must be an integer) would be divided by 100.

I also think a nice quality of life change would be to remove the leading App::Property from the type list. It also seems that many of these are also aliases for others, and if so it could be (???) beneficial to purge those. Idk if it is better to add a subtype? Since so many properties are aliases it would really reduce the things that people have to look at to find what they're actually looking for. I assume App::Property* is the actual name of the variable in the code, but no reason this can't be parsed for printing. I just find it particularly difficult to find what I'm looking for since 90% of the word is identical.

Property Box Causing Crashing

I've also noticed that these Types simply do not work and generate errors when selected: App::Property, App::PropertyComplexGeoData, App::PropertyExpressionContainer[^2], App::PropertyGeometry[^2], I stopped here because too many crashes. There are also some crashes that only happen when I actually click value. So probably worth the time having someone go through the permutations in a debug build.

I also noticed that some values immediately populate into the Property View before clicking "OK" but others do not.[^5] [^5]: This turns out to stop happening and I don't know why.

[^2]: This caused the program to crash when I had the value box open. I was going down the list and so the value box was opened due to previous properties.

Okay, and VERY WEIRD. I was trying to show that when you have a variable selected and named, that if you click the name field again it resets the value field to 0. This is clearly a bug but then when I did it while screen recording I crashed the program! lol[^4] [^4]: Also take note here that the Property View is not auto populating like it did in the previous screen record. I do not see this happening again and there is nothing I specifically did other than the crashes I am herein reporting.

LengthCrash

I uploaded the crash log to 0x0.st but note the link will only be valid for 48hrs. Ask if you need another upload. These seem to be the relevant lines:

...
Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGSEGV)
Exception Codes:       KERN_INVALID_ADDRESS at 0x000000000000006d
Exception Codes:       0x0000000000000001, 0x000000000000006d

Termination Reason:    Namespace SIGNAL, Code 11 Segmentation fault: 11
Terminating Process:   exc handler [5187]

VM Region Info: 0x6d is not in any region.  Bytes before following region: 4330766227
      REGION TYPE                    START - END         [ VSIZE] PRT/MAX SHRMOD  REGION DETAIL
      UNUSED SPACE AT START
--->  
      __TEXT                      102224000-102244000    [  128K] r-x/r-x SM=COW  /Applications/Ondsel ES.app/Contents/Resources/bin/ondsel-es
...

This actually ends up being reproducible! Here's another record where you can see if I click the Group before I click Value then things are okay. But if I click Value, then it crashes. Note that the value doesn't get reset until another field is selected.[^3] This may be a cause for my phantom 0 creation? Or potentially it could be related as I do sometimes rename variables. [^3]: Again note that the input field is not respecting where I am actually clicking. I am clicking on the Value and Type is being selected

Crash

System Info

$ inxi -SC
System:
  Host: DefinitelyMyRealHostname Kernel: 23.5.0 arch: arm64 bits: 64
  Console: s006 OS: Darwin 23.5.0
CPU:
  Info: 8-core model: Apple M2 bits: 64 type: MCP
  Speed: N/A min/max: N/A cores: No OS support for core speeds.

@stevenwalton, thank you, that is very helpful. Again, I cannot reproduce the value to resetting other than the inverse ones. I created the above issues. I think this covers everything right?

Yeah I'll let you know if I find anything else.

I'd also suggest to remove App:Property from all names. So like App::PropertyLength -> Length

once I create the variable, there is not any way to figure out what geometry / constraints reference it. IE if I accidentally define a concept while building more than one part/body that will end up in an assembly, I'm sort of "up a creek" to de-duplicate them, as I have to pull up everything in all possible sketches & features to find uses to figure out which one to keep.

Have you tried to look at Tools -> Dependency Graph. For me it shows where VarSet is used:

I found another issue with VarName. Supposed we want to have variables NumFoo and NumFooPerX (motivation: such as we have n items and want to divide into x rows. Leave configurable for user)

If you first define NumFoo you will be prevented from defining NumFooPerX. name_conflict

We have no such issues if we define them in the inverse order. (Alternatively, you can paste in your desired string) name_conflict_reverse

So this is a clear UX problem and I believe with a straight forward solution:

best: continue checking as user types, but provide the commonly seen red text below the name that says "variable name is already in use" and do not let a user click "ok". Best because it prevents invalid inputs, maintains values to allow for editing, and communicates to user what the problem is.
okay solution: Don't allow "ok" button to be pressed until arguments are valid. Okay because it does the above, but doesn't communicate to the user the underlying issue.
meh solution: Validate after user clicks "ok" but also maintain attribute population (keep all values in dialogue box the same as before). Meh because it at least won't drive the user insane (or at least as quickly).

I also noticed that the Qt Dialogue Box is critical (here and here). It is probably better to issue a warning.

If I understand the code correctly (doubtful) then it looks like the issue is how this function returns? If my understanding is correct, it is the first file (DlgAddPropertyVarSet.cpp) and this has an associated comment (I think made by either pieterhijma or chennes (not tagging in case I'm wrong). Squashed commit?). My understanding of the logic suggests that this should not be an if statement which also calls removeEditor(); or clearEditors();, but rather that there should be a while loop here where we innitialize DlgAddPropertyVarSet::setOkEnabled to False and the loop asserts that the dialogue properties are valid. Once valid DlgAddPropertyVarSet::setOkEnabled can be enabled. But this is a brief look at the code and I certainly do not understand the structure, so this is a guess. Maybe I'm in the ballpark? Or hopefully I can at least provide a hint.

I do like the dynamic behavior of validating arguments, but it is in poor design to prematurely kill a task. I think this is also in part why it is inappropriate to use the critical message, as warnings suggest things that can be resolved. Certainly this can be! (otherwise we couldn't do reverse ordering or paste in our string).

One thing I've liked about Onsel is that they consider design (UI and UX) as an important factor. It's definitely a big reason a lot of OSS projects do not gain traction. So I hope this can be resolved quickly and adequately.

Edit:

Ondsel Config

Still testing on M2 Macbook Air ``` OS: macOS 14.5 Word size of Ondsel: 64-bit Version: 2024.2.2.37240 (Git) Build type: Release Branch: (HEAD detached at 2024.2.2) Hash: fbb794cd2e56fba5fd911c8832332b5e5e235319 Python 3.11.9, Qt 5.15.13, Coin 4.0.2, Vtk 9.2.6, OCC 7.7.2 Locale: C/Default (C) Installed mods: * Ondsel-Lens 2024.7.5.02 (Disabled) * freecad.gears 1.2.0 * fasteners 0.5.24 * lattice2 1.0.0 * 3D_Printing_Tools ```

Ondsel-Development / FreeCAD