Got the basic tool -> select workflow working (#121) #492
Conversation
|
Add "figure out a better way to define the constraint conditions" to the list. And "rename constraint to command to make it more general". Anyone have any advice? |
|
Thank you for the contribution. The approach I had in mind for #121 was to make the state machine associated with each command (which can sometimes get pretty complex) encapsulated into its own class. I feel like the ad-hoc approach you have here, while certainly possible to get to work, would not be nearly as easy to maintain in the long run. |
|
That makes sense. Were you thinking something like having a common interface to each state machine class, then instantiating the one for the selected command when it's clicked, then calling it with every selection change? What about the development of this feature - should all commands go in at once or can each command be done in a separate commit/PR? The original issue talks about having to hit enter or something to confirm. What are your thoughts on this? |
Pretty much this.
I'm not sure, really. I'm also afraid that right now I don't have the bandwidth for reviewing a huge new feature like this, and I also have a large backlog of fixes in SolveSpace that will have to be reviewed first. |
|
No worries, it's just something that I'd like to be available. I know you're busy, so take your time. I'd also like to apologize for being so heavy handed in my first PR to this project, #359. I'd like to think that I've learned some things about life and project development in the past year. That being said, what do you think about the changes that I made to constraint.cpp for the sake of my IDE? Are those valid, and would it be a good idea to break it out into its own pull request? |
I would like to avoid, in general, radically changing code structure to work around any single IDE or other tool. What are the symptoms of the problem? Maybe it can be worked around in some other way |
phkahler
force-pushed
the
master
branch
2 times, most recently
from
dfb96de
to
3f09eaf
Compare
|
I closed 121. We aren't going to make these kinds of changes to the workflow. |
As a first stab at #121, I've implemented a prototype of the tool -> select workflow. I'm uploading it now for feedback, even though just the one command works. I duplicated the reference button, then changed that duplicate to set a command flag in the graphics window, then wait for a valid selection. When that occurs, I attempt to create the constraint. I have only implemented the 2 points coincident at the moment, because I'm sure I'm doing something wrong and want some feedback before I do anything else. I'm not too sure about the naming, or how to display what the currently selected command is, or how to use a key binding, or if I'm even approaching this problem the right way.
I also broke out the part of constraint.cpp that wasn't
#ifdef LIBRARYguarded into its own file; the guards made my IDE (CLion) really confused.Things I Need to add: