Reduce development effort to add point-and-click modeling operations and sketch tools

[franknoirot]

Currently it takes quite a bit of codebase knowledge to implement the point-and-click UX for new modeling operations (ex. Extrude, Revolve, Fillet) and sketching tools (ex. Rectangle, Circle, Arc) in Modeling App. We need to reduce this considerably if we're ever going to have a larger team of people able to contribute major features.

The feature availability pipeline goes something like this:

1. Requirements as GitHub discussion (ex: Sweep)
2. Engine
3. API
4. KCL
5. Point-and-Click

So we should acknowledge that often times we discover new requirements as we implement our way down that list, and need to walk back up it to get those requirements met. This issue is not about tightening that feedback loop although that is another very worthwhile effort. This issue is strictly to track reducing the time and effort it takes to implement the point-and-click version of a feature as a single developer or small cohort.

The current process

Based on my experience implementing the Rectangle tool, here is the current process as I understand it:

1. Write the code mod that you would like your point-and-click tool/operation to generate (like here)
2. Write a code mod to allow you to update values in this code mod found within the AST (like here)
3. Add a tool or operation to the modelingMachine where appropriate (like here), perhaps most-easily done using the visual XState editor provided by the XState VSCode plugin
   • If this is a sketch tool, it should originate from the Sketch.SketchIdle state via an event with the name "Equip [insert tool name]". It can have any internal states and events that it needs, and should connect to other sketch tools via equip and unequip events.
   • If it is a modeling operation, it should connect to the "Idle" state via an event with the operation's name.
   • Either way, the tool or operation should have a guard that validates when it should be available
4. Implement the business logic for the point-and-click user flow
   • If this is a sketch tool, that business logic comes in the form of setup and update methods on the clientSideScene class (like here), which are invoked as actions on the equip and internal states in modelingMachine (like here)
   • If this is a modeling operation like Extrude, that business logic for getting the user input you need for an operation lives as a Command config in modelingCommandConfigs (like here), and the action to actually perform the code mod lives as an action on modelingMachine (like here). There is currently no way to create overlay UI related to your operation in the 3D scene (ex. an extrude preview or distance handle)
5. Add an icon (like here) and a Toolbar button to invoke your new operation or equip/unequip your new tool (like here)
6. Write E2E tests to ensure your user flow works (like here)

What's hard about it

1. There are really different processes for adding tools to sketch vs operations to modeling
2. There are a lot of places where business logic and rules need to be slotted in.
3. Working with modelingMachine basically requires the XState VSCode extension to understand
4. The developer must switch between declarative/imperative programming, class-based/React context and hook-based, and into things that feel unrelated like the CommandBar, while implementing a feature.
5. The developer must understand our AST pretty deeply to create a code mod and the related code mod updater function.
6. The developer must touch THREEjs pretty directly if they are implementing a sketch tool.

How we will know it's getting better

1. Number of files needed to touch drops
2. The development flow for both sketch tools and modeling operations feel more similar
3. The developer has to interact less with systems that aren't named "modeling"
   • command bar
   • toolbar
   • clientSideScene
   • modifyAST
   • queryAST
4. There are fewer and fewer places where business logic lives

[Irev-Dev]

A start might be to figure out how to get the toolbar to auto-populate when a new tool is added, and probably also automatically getting added to the command bar might be part of it or the next step, but I'm definitely less familiar with that.

I don't really see a problem with things living in modifyASt and queryAST since those files are filled with nothing but pure functions so they should both be easy to test and high value to test them. (which I noticed, we probably should have added unit tests for the rectangle tool mods). We could even add coverage rules for just those files to hit some high % value.

But sure we can also split up the utils in those files and put them closer to what ever encompasses the tool.

I'm pretty happy what was needed to be done to SceneEntities for the rectangle tool in that most of that logic is just adding new mouse listeners for that tool, because it use existing three.js tools (straight segments) didn't really need to be touched (that would not be the case for a totally new type of segment like a spline for example, but that should be rarer). But the method in the sceneEntities logic really doesn't need to live on that class if we didn't want it too. It probably a little bit side-effectty atm so probably keeping it on the class makes sense. The way forward could be to put the three.js side effect stuff into a neat abstracted corner and make the rest more data-based, so instead setupDraftRectangle calling setupSketch directly itself, it returns all the data needed for our abstraction to setup the client side scene correctly for the draft rectangle. Would end up being a bit more complicated than I'm making out, but think it could be done.

[franknoirot]

I don't really see a problem with things living in modifyASt and queryAST since those files are filled with nothing but pure functions so they should both be easy to test and high value to test them. (which I noticed, we probably should have added unit tests for the rectangle tool mods). We could even add coverage rules for just those files to hit some high % value.

Yeah you're right, queryAST and modifyAST make sense to touch sometimes. As we add more tools and operations I would hope we have to touch these less, and then they represent kit that any new code mods can make use of. Does that seem right?

Ah yeah I can make an issue for rectangle tool code mod unit tests. That coverage idea is good.