SKETCH-2483: Implementing monomer click-and-drag#301
SKETCH-2483: Implementing monomer click-and-drag#301KevKeating merged 3 commits intoschrodinger:mainfrom
Conversation
ethan-schrodinger
left a comment
There was a problem hiding this comment.
Claude "helpfully" worked around problem two by creating a special function to do HELM string comparisons that would trim down the strings until they matched. It then "solved" problem one by erasing the body of all of the new tests and replacing them with TODO comments explaining what the test was supposed to do. It then declared the task done since the test file now passed.
Good stuff. 👍
| * If the user starting a "valid" click-and-drag, create and return the | ||
| * HintFragmentMonomerInfo object describing the start of the drag. |
There was a problem hiding this comment.
Am I crazy, or is it a little confusing that we can have "start" or "end" refer both to the time at which the drag operation starts and ends while also referring to the starting and ending points of the drag operation?
Maybe the meaning is clear enough from context.
There was a problem hiding this comment.
Yeah, that is a little confusing. I rephrased it to "If the user has began a "valid" click-and-drag, create and return the HintFragmentMonomerInfo object describing the starting location/monomer of the drag."
Description
This implements click-and-drag for the monomeric scene tools. Here are some screen shots: (Chris, I mostly added you in case you wanted to look at the pretty screen shots. I don't think there's anything here that critically needs your input, so feel free to ignore everything else.)
A drag started from an existing monomer

A drag from empty space to empty space adds two new monomers, which is the same as in atomistic mode

A drag started from the monomer uses the default attachment point (if available), which is the C terminus for peptides. Dragging to another existing monomer defaults to using the N terminus to create a standard backbone bone.

This drag started from the X attachment points, and the scene tool is smart enough to default to using the X attachment point at the other end if the user hovers over the monomer.

The drags can start and end on either empty space, an existing monomer, or an unbound attachment point of an existing monomer (which appear when the cursor is moved over an existing monomer). As with the atomistic tools, clicking on empty space and dragging to an empty space will add two atoms/monomers. Because of this, the new logic ignores click-and-drags from empty spaces when a nucleic acid sugar or phosphate tool is selected, since it doesn't make much biological sense to create a dimer of either of those.
I had Claude write a bunch of integration-level tests that would trigger both clicks and clicks-and-drags by sending events to the Scene. These would've caught a number of crash bugs that I ran into (and fixed) during development of this PR. These tests found at least two problems:
Claude "helpfully" worked around problem two by creating a special function to do HELM string comparisons that would trim down the strings until they matched. It then "solved" problem one by erasing the body of all of the new tests and replacing them with TODO comments explaining what the test was supposed to do. It then declared the task done since the test file now passed. Fortunately, I'd committed the tests to my local git branch before Claude started "fixing" them. I'll revive the tests and figure out the Scene updating issue once I get the HELM strings fixed.
There are a few additional minor things that need fixing unrelated to what Claude found:
This PR contains a fair bit of refactoring so that the click and click-and-drag logic can share as much code as possible. It took a while to get right, but I actually like the new organization: it very explicitly separates the tasks of figuring out what to draw for the hint, drawing the hint, and committing to MolModel.
Testing Done
Tested via desktop app on Windows. The tests that Claude wrote (and then deleted) are definitely helpful, so I'll add those in once the two bugs above are fixed.