I haven't been following the named parameters discussions too closely, so forgive my ignorance, but I'm not sure what the scoping rules are, so a clarification would be very much appreciated. The comments in the code of this PR imply that the scope is based on a group's handle value, but that's not how entity scope is determined today. For regeneration, the scope is based on group order, which is mostly determined by group dependency (through opA). Ideally we should ditch the linear representation of groups in the text window and just draw a tree view of groups to make it more obvious to the user, but in any case, that's how the solver currently solves groups, and basing the scope on a group's handle value will conflict with the way the solver defines known parameters that are considered constants that shouldn't be solved.

Additionally, the code in this PR doesn't deal with named parameter dependencies, so when a named parameter is deleted, any requests and constraints that depend on it are left dangling. I'm not sure what the best way to deal with it is, but we should at least trigger a regeneration and prune such requests or constraints (honouring the scoping rules), similar to how entity deletion is handled today. A more difficult case is parameter rename, which doesn't seem to check for name conflicts, and can also leave dependants dangling.

Lastly, how should we deal with parameters that redefine constants (such as pi)? The expression parsing code seems to just use the constants, but the UI doesn't try to warn the user when a parameter name is set to the name of a known constant.

I only skimmed the code in this PR, so I'm sure I'll have more questions later when I go through the code more thoroughly, but these are the ones that came up during my first reading.

@iscgar the current implementation does not follow sketch generation scope rules. The plan is to create a valid set of names for the current group when generating a sketch. That would start with a copy of the parent group and adding any names from the current group.

It will also crash if you delete or rename a parameter that is used in a constraint. This will be "solved" when we add the ability to dynamically create a parameter in the parser. To do that it will need the handle for the constraint. Parameters can belong to a request, a group, or a constraint. The text window ones belong to the request. Dynamicly created one will belong to the constraint they are first encountered in. Thats also how it will cope with deletions or renaming - constraints will just create a new parameter with the old name. If someone wants to write a renaming algorithm to update all occurrences when a name is changed thats fine but not strictly necessary.

On the fly parameter creation is important for the relation constraints and the old demo. You can also define parameters on the sketch with "width = 12" or similar. That stuff also turns Solvespace into a small symbolic algebra tool.

BTW I tagged you specifically because you know the solver and regeneration process.

@iscgar the current implementation does not follow sketch generation scope rules. The plan is to create a valid set of names for the current group when generating a sketch. That would start with a copy of the parent group and adding any names from the current group.

Am I correct in my understading that that plan is to effectively follow the sketch regeneration scoping rules, and that the reason the current code doesn't do so is merely because it's a WIP draft for getting the ball rolling?

It will also crash if you delete or rename a parameter that is used in a constraint. This will be "solved" when we add the ability to dynamically create a parameter in the parser. To do that it will need the handle for the constraint. Parameters can belong to a request, a group, or a constraint. The text window ones belong to the request. Dynamicly created one will belong to the constraint they are first encountered in.

Since named parameters can also contain expressions, dynamically generated parameters will also need to be generated by requests, not just constraints.

One related concern that I forgot to mention: the bit space in constraint handles doesn't allow creating more than a single parameter (see my comment #1574 (comment) and #1574 (comment)), and the space in request handles is also very limited, although it should probably suffice (the approach in this PR, which reuses the 6th bit, allows creating up to 32 parameters).

What scope should the dynamically generated parameters belong to? Only the request or constraint that uses them? That seems like a recipe for surprises, since the solver will be free to solve parameters with the same name differently. Scoping them to the group that the request or constraint belong to seems more appropriate to me, although I'm likely missing some other corner cases that would be surprising with that behaviour.

This also brings up the issue of defining the same named parameter multiple times in the same group. The current code doesn't seem to handle that case, and from a cursory look at this PR it seems that the last parameter "wins" (because named parameters are iterated in order of request handle values).

Thats also how it will cope with deletions or renaming - constraints will just create a new parameter with the old name. If someone wants to write a renaming algorithm to update all occurrences when a name is changed thats fine but not strictly necessary.

Instead of keeping the expression in the request or constraint as a string, we could store the parsed expression, which would refer to the handle values of the parameters (although note that it would require decoupling the expression system from the sketch's temporary heap, as those expressions are long lived, unlike the current use of expressions only during regeneration, which allows them to be discarded as soon as a group is solved). That way, renaming a parameter doesn't require walking the tree, and deleting a paramater can be dealt with by either pruning the request/constraint during regeneration, as we do for dependant requests and constraints today, or we could just resolve them with dynamic parameter generation (although see my comment in the next paragraph about the scoping of dynamically generated parameters).

On the fly parameter creation is important for the relation constraints and the old demo. You can also define parameters on the sketch with "width = 12" or similar. That stuff also turns Solvespace into a small symbolic algebra tool.

The old demo is presumably this video by Evil-Spirit? Looking at it seems to imply that dynamically generated parameters don't belong to the constraint that mentions them, since all a and angle values are resolved to the same value. It rather seems like the Evil-Spirit implementation simply defined them globally as soon as they were first encountered (where "first encountered" seems to be the order of constraints based on their handle values). I'm also not sure how the solver is expected to handle an expression such as sqrt(a * a + b * b) with dynamically generated parameters, as any random value satisifes the constraint, as long as it's well defined (i.e. the expression under the sqrt doesn't become negative, infinity, or NaN).

As for sketch parameters, I'm assuming defining parameters on the sketch is the same as defining them in the references group, since you can't sketch anything in that group anyway, and it's the root of the group tree.

On the fly parameter creation is important for the relation constraints and the old demo. You can also define parameters on the sketch with "width = 12" or similar. That stuff also turns Solvespace into a small symbolic algebra tool.

The old demo is presumably this video by Evil-Spirit?

@iscgar yes. See also the https://github.com/solvespace/solvespace/commits/parameters/ which is the "modern", unfinished implementation. And (in case you are not aware of it) the O.G. issue and discussion #77

And this #1482 where @dgramop has a WIP of using parameters for the number of repeats in "Step trans/rotating" groups.

@iscgar yes. See also the https://github.com/solvespace/solvespace/commits/parameters/ which is the "modern", unfinished implementation. And (in case you are not aware of it) the O.G. issue and discussion #77

And this #1482 where @dgramop has a WIP of using parameters for the number of repeats in "Step trans/rotating" groups.

Thanks for the links! I did read both of those discussions, but it's always good to referesh my memory. After reading the discussions I'm still not sure how scoping for dynamically generated paremeters should be handled, especially for partial regeneration. This is because we'd need to collect named parameters from parent groups even if they are not being solved, just so we don't end up introducing free parameters where the user expects to reuse the solved ones from the parent groups. If the dynamically generated parameters are scoped to their group rather than to their request/constraint, that would be trvivial to implement.

Additionally, what should be the interactions with linked assemblies? Right now their solved form is being used and they never get regenerated during a parent regeneration, so we don't really need to do anything regarding their named parameters. However, if we ever get hierarchical sketches, I suppose we'd like them to be dynamic and be affected by parameters defined in the parent sketch or vice versa? Supposedly child sketches will be handled differently than linked assemblies anyway (i.e. they won't be groups of static entities), so we can just defer the decision to when we get hierarchical sketches.

I would love to implmenet the regeneration part, but coming back to my points above, it would be great to properly specify the behaviour of dynamically generated parameters and the scoping rules in general. If we really need constraints to dynamically generate parameters, we might want to hold off the implementation of named parameters until 64-bit handles land, since with the current encoding of contraint handles it's impossible to encode more than a single parameter as I mentioned above. Alternatively, we can let the constraint's group be the owner of these parameters, and in that case there shouldn't be any issue with their encoding. However, we still need to decide what to do in some of the weird cases that I mantioned above, such as multiple parameters with the same name defined ina single group, or worse -- a named parameter getting renamed to the name of an existing named parameter in the same group. If allowed, that last case will be sure to cause confusion since the parameter handles are not exposed in the UI, and it'll not be obvious which parameter shadows which.

The behaviour for the rest of the cases I presented (such as shadowing a known constant, which we might want to disallow, and also reserve a bunch of names to give us the option to use them as constants in the future) would also need to be defiend, but I feel that it shouldn't be a blocker for an initial implementation.

@iscgar each group should inherit (copy) the name/handle dictionary of its parent group. Then add any names from requests in the current group, overwriting the handle of any duplicates - most recent wins.

Then when generating equations for constraints we would ideally create new parameters for names that don't exist yet. These would also get copied to subsequent child groups. This would allow writing "width=5" on a sketch (relation constraint) and using it throughout, without needing to use a request.

Are parameters that belong to constraints recreated on regeneration? Are they saved to files? (I think they probably are?)

You noted in the old video a triangle gets a side "sqrt(aa+bb)" and the other sides are "a" and "b". We don't care which constraint those 2 parameters belong to, they could belong to the group. What matters is that all instances of "a" in that group refer to the same parameter. But notice the expression was used instead of "c". If it were called c then we'd have aa + bb = c*c - think pythagorous. After that expression was added it became a right triangle. A and B could be independently changed, but the angle stayed 90 degrees!

As for linked sketches. I wrote a plan for those recently ( issue #1436 ). If we have file A and in group 5 we link file B, we want to show parameters defined in B references group in File A group 5. Once we can load and solve linked sketches, those parameters in File A group 5 will be copies of those if File B and we will allow them to be edited. Their values will be copied to that instance of file B prior to solving. In this way you'll be able to create say an extrusion of 8020 with a length defined in references. Then you can use multiple instances of that part in another sketch by linking multiple times, and each one could be a different length. Reusable paramePythagoras.

We can do all of this without dynamic creation of new parameters in the parser, using text window definitions. But 1) it can be nice to just write them on a sketch. And 2) we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.

we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.

It just occurred to me that if a stored constraint expression becomes invalid because a variable got deleted or renamed, we could simply not generate an equation for that constraint. The sketch would gain a degree of freedom, but nothing should be moved by the solver. We might have a way to inform the user that the constraint is broken and maybe indicate "err" for the group too. Would it be possible to do this with a generic equation like 1=0 ?

Are parameters that belong to constraints recreated on regeneration? Are they saved to files? (I think they probably are?)

Currently parameters belong to requests, and the only constraint that can generate a (single) parameter is the point on line constraint (see cc07058 for the reason).

But yes, parameters get saved into the file, although that's only for the benefit of assemblies. When loading a model into a sketch (as opposed to linking), everything gets regenerated from groups, requests, and constraints.

You noted in the old video a triangle gets a side "sqrt(a*a+b*b)" and the other sides are "a" and "b". We don't care which constraint those 2 parameters belong to, they could belong to the group. What matters is that all instances of "a" in that group refer to the same parameter. But notice the expression was used instead of "c". If it were called c then we'd have a*a + b*b = c*c - think pythagorous. After that expression was added it became a right triangle. A and B could be independently changed, but the angle stayed 90 degrees!

What I meant was to ask about a case where there aren't any other constraints that use a or b, and the expression is used in the only constraint that the solver needs to solve (say a dimension constraint). In that case the parameters are free, and the solver can choose any real value to arrive at a valid solution. With parameters defined in requests, the user has control over the parameter value (which can be edited, and inspected by the user), so the solver won't do anything surprising as long as the user chooses meaningful values. However, with dynamically generated params, the solver will be free to keep the initial value (which the user has no direct access to), resulting in the user fighting the solver when dragging, with no control over the result unless a named parameter request or a relation constraint is introduced. This feels a bit too magical for me (as an aside, I also dislike the fact that with dynamic parameter generation there are effectively two ways to define a parameter, with named parameter request being inferior in the current implementation because it does not allow an expression).

[snip]

We can do all of this without dynamic creation of new parameters in the parser, using text window definitions. But 1) it can be nice to just write them on a sketch. And 2) we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.

[snip]

It just occurred to me that if a stored constraint expression becomes invalid because a variable got deleted or renamed, we could simply not generate an equation for that constraint. The sketch would gain a degree of freedom, but nothing should be moved by the solver. We might have a way to inform the user that the constraint is broken and maybe indicate "err" for the group too. Would it be possible to do this with a generic equation like 1=0 ?

Why wouldn't the expression parse? We can store named parameter references as new expression nodes (lazily bound parameters, resolvable by name), and fix their references only during regeneration, when they are being solved. This is similar to a resolution step that is already taking place today during solving, where parameter handles in the expression are resolved to direct parameter pointers. That way we can fail at resolution time with a nice message about an unknown parameter, allowing the user to define it and retry, similar to how the solver can fail when a set of constraints is unsolvable, and the user can make changes to let the solver retry. To support falling back on the old value while the user fixes things, we can keep a shadow copy of named parameters when a user renames or deletes a parameter, which will be discarded after the first successful regeneration (as a successful regeneration means the operation didn't orphan any parameters that later expressions depend on).

But yes, parameters get saved into the file, although that's only for the benefit of assemblies. When loading a model into a sketch (as opposed to linking), everything gets regenerated from groups, requests, and constraints.

Parameters need to be saved in a file so the sketch has reasonable dimensions. When you draw a line the endpoints use 2 parameters each (x, y). The line entity has handles for the 2 point entities and then each point has handles for 2 parameters. All coordinate parameters have to be saved because of that. I was wondering if the constraint-created parameters are also saved, but I suspect the answer is yes for similar reasons. They also should not be discarded and recreated during sketch regeneration for the same reason. I am not very familiar with the whole regenerate process.

You noted in the old video a triangle gets a side "sqrt(aa+bb)" and the other sides are "a" and "b". We don't care which constraint those 2 parameters belong to, they could belong to the group. What matters is that all instances of "a" in that group refer to the same parameter. But notice the expression was used instead of "c". If it were called c then we'd have aa + bb = c*c - think pythagorous. After that expression was added it became a right triangle. A and B could be independently changed, but the angle stayed 90 degrees!

What I meant was to ask about a case where there aren't any other constraints that use a or b, and the expression is used in the only constraint that the solver needs to solve (say a dimension constraint). In that case the parameters are free, and the solver can choose any real value to arrive at a valid solution.

But it won't. When you type "a" into a dimension constraint (in the old demo), a parameter gets created and is substituted into an equation defining the distance between 2 points. That parameter value will change as you drag an end of the line and change its length. Currently this will also happen with my named parameters. If you define one in the text window of a group and then use it as a distance for the length of a line in that same group, you can still drag the line length and the parameter value in the text window will change. If the line is in a later group, the length will be fixed because the parameter is treated as constant since it's from a previous group.

I don't know if I like this behavior. We may want to "lock" the parameter to the typed value. Relation constraints will also allow writing "x=5" on the sketch, so locking in the text window would prevent that. For that reason I think there needs to be an option to lock them in the text window. The original issue #77 author suggested names could define a variable or a constant. I think one request type is enough, but a checkbox to lock it might be needed. I'm not sure what the internal mechanism would be for that - a request can generate a constraint, so that may work. I thought there might be a flag on parameters that prevent the solver from changing them but I don't know the solver well enough to be sure.

With parameters defined in requests, the user has control over the parameter value (which can be edited, and inspected by the user), so the solver won't do anything surprising as long as the user chooses meaningful values. However, with dynamically generated params, the solver will be free to keep the initial value (which the user has no direct access to), resulting in the user fighting the solver when dragging, with no control over the result unless a named parameter request or a relation constraint is introduced. This feels a bit too magical for me (as an aside, I also dislike the fact that with dynamic parameter generation there are effectively two ways to define a parameter, with named parameter request being inferior in the current implementation because it does not allow an expression).

We can do relation constraints without dynamic parameter creation. If we find a way to do it later it should be possible to add without breaking compatibility.

[snip]
We can do all of this without dynamic creation of new parameters in the parser, using text window definitions. But 1) it can be nice to just write them on a sketch. And 2) we will need to handle renaming and deletion somehow. Those operation will orphan a name in other parts of the sketch and the expressions won't parse. They can't just revert to a previous value either as there is no meaningful number in valA.
[snip]
It just occurred to me that if a stored constraint expression becomes invalid because a variable got deleted or renamed, we could simply not generate an equation for that constraint. The sketch would gain a degree of freedom, but nothing should be moved by the solver. We might have a way to inform the user that the constraint is broken and maybe indicate "err" for the group too. Would it be possible to do this with a generic equation like 1=0 ?

Why wouldn't the expression parse? We can store named parameter references as new expression nodes (lazily bound parameters, resolvable by name), and fix their references only during regeneration, when they are being solved. This is similar to a resolution step that is already taking place today during solving, where parameter handles in the expression are resolved to direct parameter pointers. That way we can fail at resolution time with a nice message about an unknown parameter, allowing the user to define it and retry, similar to how the solver can fail when a set of constraints is unsolvable, and the user can make changes to let the solver retry. To support falling back on the old value while the user fixes things, we can keep a shadow copy of named parameters when a user renames or deletes a parameter, which will be discarded after the first successful regeneration (as a successful regeneration means the operation didn't orphan any parameters that later expressions depend on).

The parser throws an error for parameter/constant/function names it doesn't recognize. We validate an expression in mouse.cpp when it is typed. We store the expression string in a constraint, but it is re-evaluated in constrainteq.cpp during regeneration. If the name is changed and the sketch regenerated there will be an expression that won't parse. This currently causes a crash. As I suggested in a previous comment, we could simply not create an equation for the constraint in that case, or even create an unsolvable one to trigger existing warnings. I'll try that tonight if I have time.

I've read everything but have not thought carefully. That said I want your opinion on what should happen in the following situation:

0. CTRL-N
1. Draw two "free" line segments.
2. Add dimension constraints to both with a value "x".

To clarify "x" is NOT defined in any of the "explicit" ways you are discussing.

Bonus questions:

• Change one constraint to "x=3"

• Change one constraint to "2*x"

• Add a second dimension constraint to one segment and make it "4" (yes it is possible in the parameters branch and I still have not looked into why).

• Change one constraint to "x*x"

• Change one constraint to "y=x+8"

I prevented the crash when a named parameter is deleted but still used in constraints. The red background will appear and then somehow it will figure out which are the bad constraints and delete them with a message box.

@ruevs as it is right now you can only use parameters that are defined in the text window, and parameters defined in the current group are not fixed value. So you can do the entire demo/video so long as you define all the names (x,y,a,b) in the text window of g002 and draw everything in g002. Dragging a line of length x will change the value of x - you can see that value change in the text window. Adding a second constraint to the length of a line and typing in a number will force the line to that length, and hence x will become that value as well.

Drawing a triangle and giving the sides length a, b, sqrt(a * a+b * b) will force legs a and b to be perpendicular but not of fixed length.

The only thing missing here is full scope rules. It does obey the rule that params defined in a previous group are held constant for the current group - the solver enforces that because these parameters are defined correctly. All we need now is for the list of parameters to include ONLY those that can be used in the current group (not future groups) and override the handles in the appropriate way if there are duplicate names from different groups.

So in expr.cpp and EditControlDone we need to pass in a "current" list of names.

Parameters need to be saved in a file so the sketch has reasonable dimensions. When you draw a line the endpoints use 2 parameters each (x, y). The line entity has handles for the 2 point entities and then each point has handles for 2 parameters. All coordinate parameters have to be saved because of that. I was wondering if the constraint-created parameters are also saved, but I suspect the answer is yes for similar reasons. They also should not be discarded and recreated during sketch regeneration for the same reason. I am not very familiar with the whole regenerate process.

Right, the constraint created param is also saved. During regeneration all of the parameters are solved again, although the values loaded from the file are used as initial guesses, so in practice the regeneration should converge to almost exactly the same values as in the file.

What I meant was to ask about a case where there aren't any other constraints that use a or b, and the expression is used in the only constraint that the solver needs to solve (say a dimension constraint). In that case the parameters are free, and the solver can choose any real value to arrive at a valid solution.

But it won't. When you type "a" into a dimension constraint (in the old demo), a parameter gets created and is substituted into an equation defining the distance between 2 points. That parameter value will change as you drag an end of the line and change its length. Currently this will also happen with my named parameters. If you define one in the text window of a group and then use it as a distance for the length of a line in that same group, you can still drag the line length and the parameter value in the text window will change. If the line is in a later group, the length will be fixed because the parameter is treated as constant since it's from a previous group.

The solver will generally try to keep the initial values if they still satisfy the constraints. However, when dragging a point of a line segment, it must change either the parameter in the length equation or the parameters of the other point, since the constraint isn't satisfied with the existing value (it has less effect on the dragged point, due to the dragged logic that limits it from making significant changes to dragged parameters, but otherwise it can still affect the dragged point as well), and that's when unexpected things may happen depending on the order of equations provided to the solver. I haven't tested the parameters branch, but you can see that in the old demo Evil-Spirit was forced to put a WHERE_DRAGGED constraint on the other point of the line segment in order to ensure that the solver only changes the constraint's free parameter.

I don't know if I like this behavior. We may want to "lock" the parameter to the typed value. Relation constraints will also allow writing "x=5" on the sketch, so locking in the text window would prevent that. For that reason I think there needs to be an option to lock them in the text window. The original issue #77 author suggested names could define a variable or a constant. I think one request type is enough, but a checkbox to lock it might be needed. I'm not sure what the internal mechanism would be for that - a request can generate a constraint, so that may work. I thought there might be a flag on parameters that prevent the solver from changing them but I don't know the solver well enough to be sure.

Locking parameters should be easy enough to do. All that is needed is for the generation code to mark them as known, which causes the solver to resolve them as constants. So if a "lock" check box is introduced for the named parameter request, it can easily be implemented within the existing regeneration framework.

The parser throws an error for parameter/constant/function names it doesn't recognize. We validate an expression in mouse.cpp when it is typed. We store the expression string in a constraint, but it is re-evaluated in constrainteq.cpp during regeneration. If the name is changed and the sketch regenerated there will be an expression that won't parse. This currently causes a crash.

Sure, but what I'm suggesting is to modify the parser to allow parsing such expressions without crashing. This can be done by introducing a new expression node (which I termed "lazily bound parameters") that can be resolved at evaluation time by passing a list of params to use for the resolution. This is an easy change to do, and is the most logical IMO. That way we also easily deal with renames and deletions, since resolution will be delayed until we actually evaluate the expression. It'll also allow us to bail at solution time and point directly to the broken constraint, without the need for hacks like 0 = 1 which the solver can only identify by brute force.

@phkahler I took your code and created an initial draft of the scoping rules that you described and my idea of the parser changes in a branch. Note that this branch does not yet persist named parameters to file, so any file that uses named parameters will be broken.

The main changes from the code in this PR are:

• Named parameters are a group property rather than requests (this makes it easier to reason about scoping, and avoids redundant iterations over the requests list to gather named parameters)
• The expression parser supports parsing named parameters without resolving them, allowing resolution to be deferred to when needed
• A new solver status for unresolved named parameters was added, and deletion of a named parameter (or referencing an undefined named parameter when editing a constraint) is handled with a descriptive message in the text window

@iscgar I'm going to need some time to review what you're doing. At a glance it seems more invase, but a lot of the changes are small.

Right now the version in this PR can save and load files - there was no new code needed for that. In mine, I see that diameters are not shown correctly.

@iscgar I'm going to need some time to review what you're doing. At a glance it seems more invase, but a lot of the changes are small.

Most of the invasiveness in my branch is due to also decoupling Expr from the sketch and making it standalone, which requires passing ParamList everywhere params need to be resolved. I did it mostly because it seemed a good opportunity to do so with the need to pass named parameter mapping for named parameter resolution, but we can do the decoupling at a later stage and reduce the scope of the changes.

Right now the version in this PR can save and load files - there was no new code needed for that.

That's indeed a plus with your implementation, though I don't think it justifies the added complexity with the request handling all over the place. Adding the ability to save and load named parameters shouldn't be too difficult, and I'll try to work on it during the weekend so the two approaches could be compared.

That's indeed a plus with your implementation, though I don't think it justifies the added complexity with the request handling all over the place. Adding the ability to save and load named parameters shouldn't be too difficult, and I'll try to work on it during the weekend so the two approaches could be compared.

I'm not sure what you mean by "request handling all over the place". The text window needs something - I do want to define them primarily in the text window, but in relation constraints and on-sketch expressions is a very nice to have. I'm going to move the creation of the "dictionary" into the group generation area next, so that should be cleaner and only in one place. None that means they couldn't belong to the group instead of a request, but groups (so far) have fixed number of parameters depending on the type of group.

One thing I hated about the original demo be evil-spirit was that he added names to the parameters themselves and saved those in files. Parameters in Solvespace were never meant to be any kind of object. They are just a double with a handle that can be used by the constraint solver. Objects (groups, requests, constraints) have parameters, not the other way around :-)

I hope to have time to finish my approach in the coming week as well.

I'm not sure what you mean by "request handling all over the place". The text window needs something - I do want to define them primarily in the text window, but in relation constraints and on-sketch expressions is a very nice to have. I'm going to move the creation of the "dictionary" into the group generation area next, so that should be cleaner and only in one place. None that means they couldn't belong to the group instead of a request, but groups (so far) have fixed number of parameters depending on the type of group.

What I meant was that with requests there is no easy way to find the group's parameters other than iterating over the entire request list, and that leads to things like the double iteration when showing the group info in the text window, or during regeneration. As you mentioned, though, my approach isn't without downsides (the biggest of which is the need to add new code for saving and loading parameters, whereas your approach Just Works by reusing the request serialisation logic).

None that means they couldn't belong to the group instead of a request, but groups (so far) have fixed number of parameters depending on the type of group.

In my implementation the named parameters use the upper half of possible group parameter values, in order to allow future groups to use the entire bottom half for fixed parameters (or other uses that may arise) if needed. the total allowed parameter range per group is 2^16, so that should leave plenty of room for future expansion, and the room for named parameters should also be enough, as I don't think users will ever need to define more than 32K named parameters.

Regardless of whether named parameters are requests or a group property, another question is whether name resolution should be performed at expression parsing time or deferred to expression evaluation time. Your implementation chooses the first option, whereas I went with the second one. Deferring resolution to evaluation time allows the solver to handle resolution failure as a first class citizen, clearly indicating the issue and pointing to the list of bad constraints, resulting in nicer error handling. I feel that requiring them named parameters to be resolved at parsing time is needlessly limiting, and leads to hacks like the unsolvable equation that you needed to add when resolution fails in order to allow the solver to find the bad constraint (which still isn't a nice solution, because the user has no way to tell why the constraint is bad).

I hope to have time to finish my approach in the coming week as well.

Sounds great. I'm looking forward to seeing the complete implementation.

I cleaned up the code a bit, dropped the decoupling of Expr from the sketch for now, and implemented named parameter persistency in a different branch. Please check it out and let me know what you think.

EDIT: I updated my implementation to include some quality of life improvements, such as showing the unresolved parameter names in the text window on resolution failure, as well as fixes to bugs that I discovered while testing.

I cleaned up the code a bit, dropped the decoupling of Expr from the sketch for now, and implemented named parameter persistency in a different branch. Please check it out and let me know what you think.

@phkahler @ruevs did you get a chance to test my branch? I would love to hear your thoughts and get your feedback regarding the approach I took.

@iscgar I played both with yours @phkahler's branches - as a "dumb user". Have not looked at the code of either and therefore have not commented yet.

@iscgar I have not had time yet. How does it handle names that only exist in an expression? Are they added to the group? At what index?

@ruevs my version still needs to inherit a list of names from the parent group to get the scope handled correctly, and also needs a toggle to lock a value in the text window (const vs varaible).

@iscgar I have not had time yet. How does it handle names that only exist in an expression? Are they added to the group? At what index?

No, you have to create named parameters on the group to use them in an expression. Any unresolved references result in an error in my version, with a clear explanation of the issue in the text window:

Once you get this error, you can either edit the expression if there was a typo, or select the group in the text window and add the missing named parameters in order to fix the issue.

[snip] also needs a toggle to lock a value in the text window (const vs varaible).

Is this really needed? I was under the impression you preferred to use the references group for "constants".

I implemented parameter locking functionality in my branch, and it's indeed very useful. While at it I also aligned parameter names and values in the text window to make it look more consistent with different lengths of parameter names and values:

I implemented parameter locking functionality in my branch, and it's indeed very useful. While at it I also aligned parameter names and values in the text window to make it look more consistent with different lengths of parameter names and values:

Off topic: @iscgar by adding tooltips you've touched the infamous TextWindow::Printf - our "GUI framework " :-) - which has inspired some... passionate comments... in the past. Having used it to implement parameters and even modified it to add tool tips - what do you think of it? Sincerely :-)

I'll pull and play with your latest changes tomorrow. Not sure if I'll find time to read the code.

Off topic: @iscgar by adding tooltips you've touched the infamous TextWindow::Printf - our "GUI framework " :-) - which has inspired some... passionate comments... in the past. Having used it to implement parameters and even modified it to add tool tips - what do you think of it? Sincerely :-)

Well, it's different from other GUI libraries I've used in the past. When I first encountered it I wasn't sure what I was looking at (the name Printf definitely didn't help, and the half-line logic felt like a quick and dirty patch to fix a special case). However, as a user, while I still find myself going back to the code in order to refresh my memory about the different modifers of %B, %F, and %L, I've mostly gotten used to it. As a devloper, the tool-tip feature already existed in the text window, so I'm not sure it's representative of actually implementing a new GUI component, since all I had to do was add the new format string and show the tool-tip on hover (I did have to debug the weird interaction with DrawOrHitTestIcons() though, so there's that). The fact that the GUI code is so bare-bones made it in some ways easier to add the tool-tip feature, because I only needed to keep track of all the ways the item matrix is used to make it functional.

While having a proper GUI toolkit abstraction would probably make the code easier to interact with and understand, I'm not sure it justifies the effort that it would require. On the one hand you'd be able to add things like text boxes, and maybe even fancier controls, like combo boxes and proper radio button groups. On the other hand, you'd then need to deal with things like scrollable containers and Z-order, which currently simply aren't a thing.

@iscgar How did you implement the lock/unlock icons? I searched and couldn't find those in the unicode BMP :-) Also, how are you handling units in your parameters branch? Or are you? I like your version of the text window. Also glad you found the value in locking - sometimes you want to have a parameter with a fixed value and just use it in the current group. My intent for the references group would be for "global" or high level parameters to be defined there, and also to reflect those in linking group.

@iscgar How did you implement the lock/unlock icons? I searched and couldn't find those in the unicode BMP :-)

That's because they aren't standard BMP glyphs :)
The way custom icons in the text window are handled is by assigning them to the Private Use Area:

They are then referenced using raw UTF-8 bytes in the strings handed to TextWindow::Printf.

I simply added two new PNG files for the lock states, and assigned them to the next empty slots in the PUA.

Also, how are you handling units in your parameters branch? Or are you?

The same way it's handled in your branch, with the unit multiplier saved in valA of the constraint.

Any updates on this PR? The discussion here sounds to me like you guys are almost there!

Any updates on this PR? The discussion here sounds to me like you guys are almost there!

@TerryGeng We've gone in slightly different directions. With holiday vacations coming up I'm planning to pull this together in the next few weeks. My plan is to use my original concept of having requests for parameters in the text window and internally using a per-group dictionary copied from the parent group and extended by new requests (or values overwritten for duplicate names). This avoids adding any new members to C++ classes or anything new to files, just some new request types. I'll use @iscgar layout of the text window because it's clean and he added nice locks. We'll use 2 request types for locked or unlocked parameters (the UI can just change the type). Actually for future-proof (linked sketches) there will be 2 more reserved parameter request types but that stuff will be transparent to the user. That much I want to start on Dec 20th and get done ASAP. After that is in place and working I'll hope to turn @dgramop loose on bringing relation constraints on top of this (if still interested ;-). We will not be allowing new names to be defined in relation constraints yet - that's going to be a little bit tricky (meaning I'm not sure the best way to create the parameter handles yet, but it will be possible to add).

I'd like to get version 3.2 final out this year, and then immediately start putting the named parameters stuff in. If all goes well there could be a 3.3 with that stuff by spring - I'm happy to have a release just for this feature. I'm never as fast as I hope, but we are very close with this like you said. This was first prototyped when? In 2016? so 10 years. Ugh, it's time.