It’s time to figure out next moves. Small steps, always, but do we need to bend the path a bit? If so, tending where?
My list of concerns includes:
- Additional Pieces. There are more parts than we have now, rocker arms, lifting arms, expansion links, I don’t know what all.
- Refactoring. Much of the code is how it is because it got that way, and I’d like it to be a bit more communicative and pleasant to work with.
- Renaming. We found a diagram giving the linkage parts their official names, and this could be a good time to make our parts’ names correspond.
- Angles. It turns out that almost all our locomotives have their pistons, not horizontal, but slightly angled. We need to account for that in their definition, and we might want to explore whether there is a better approach to solving them than what we have.
- Testing. Our individual unit tests are good, and give confidence, but testing a linkage of more than two pieces is difficult, because hand-calculating the results is at best tedious and quite often, well, I just don’t want to.
- Visualization. We can draw static SVG pictures of a linkage at a given setting, but that, too, is tedious and far to slow to provide rapid feedback on whether things are working. Luau is so well sandboxed that I can’t see how to get to a more dynamic display of what’s going on.
- Expression. It’s early days for this, but if this thing is to be useful, we need to be able to quickly get from a diagram, or an SL build, to a collection of linkage pieces, correctly wired up. So far, each piece has its own idiosyncrasies, which isn’t ideal. I think we need to express each one with root position and planar rotation angle.
- Points of Interest. Each piece has points of interest, at least a point of connection to its parent, and usually at least one “output” point, such as the opposite end of the ConnectingRod, the part that pushes the PistonRod. These positions can be at any offset in the object that we might represent by root and angle. We dare not be too general with this idea, since these key points are accessed during every calculation of the linkage arrangement.
- Real-time or Puppeteer? Will we embed the linkage code in the vehicle, or will we use it to produce a list of positions and rotations that can be processed with a faster routine that just interpolates between some number of known points of the linkage. The latter will work fairly well, though it looks odd if you pay close attention, and can make the linkage look “loose” in operation.
Wow. I didn’t realize that I had so much on my mind.
This is turning out to be one of those projects where you thought you had a few days’ work and it turns into a lifetime profession. So far, it’s kind of fun.
Today
It’s Saturday, about 1015, and I think I’ll just do some easy stuff. Let’s begin by renaming things to match this picture:
I’ll do this with Sublime’s multi-select, or with its replace. Let’s try replace. Run the tests and commit the code first.
The thing I’ve been calling CouplingRod is SideRod. I’ll try a replace. The tests all run. This might work.
What I called ConnectingRod is MainRod. There are two in the code ConnectingRod and GeneralConnectingRod. I think one rename should do it. Done, tests all run. I could be committing the code on each rename but I won’t.
Curiously enough, the PistonRod is called PistonRod. The only other class I’ve invented is called DriveWheel and the diagram has no opinion about that.
Rename complete. Tests pass. Commit and push the code.
So, that was probably worth doing. What now? There are tests using shorthand names, like this:
_:test("story test", function()
local wheel = DriveWheel{x=10, y=2}
local con_rod = MainRod{parent=wheel, length=6, radius=1}
local piston_rod = PistonRod{
parent=con_rod, position=vector(6,2), length=4}
wheel:move(0)
con_rod:calculate()
_:expect(con_rod:position(),"con").is(vector(8,2))
_:expect(con_rod:drive_point(),"drive").is(vector(5,2))
piston_rod:calculate()
_:expect(piston_rod:position(),"piston").is(vector(3,2))
end)
Ideally, we’d rename those locals to match:
_:test("story test", function()
local wheel = DriveWheel{x=10, y=2}
local main_rod = MainRod{parent=wheel, length=6, radius=1}
local piston_rod = PistonRod{
parent=main_rod, position=vector(6,2), length=4}
wheel:move(0)
main_rod:calculate()
_:expect(main_rod:position(),"con").is(vector(8,2))
_:expect(main_rod:drive_point(),"drive").is(vector(5,2))
piston_rod:calculate()
_:expect(piston_rod:position(),"piston").is(vector(3,2))
end)
I think we’ll do that as we encounter the places. We do not revisit tests often.
What’s next? I’ll scan the railroad classes for code that could use improvement. It’s all pretty simple, looks decent. This class seems a bit raggedy to me:
MainRod = class()
function MainRod:init(parms)
self._parent = parms.parent or error('expected parent')
self._length = parms.length or error('expected length')
self._offset = vector(parms.radius or error('expected radius'), 0)
self._position = vector(0,0)
self:calculate()
end
function MainRod:calculate()
local base_pos = self._parent:position()
local rotated = self._parent:adjusted_offset(self._offset)
self:compute_center(base_pos, rotated)
end
function MainRod:compute_center(base_pos, rotated)
local wheel_end = base_pos + rotated
self._position = self:center_position(base_pos, wheel_end)
end
function MainRod:center_position(base_pos, wheel_end)
return wheel_end + self:center_delta(base_pos, wheel_end)
end
function MainRod:center_delta(base_pos, wheel_end)
local delta_y = base_pos.y - wheel_end.y
local delta_x =
- math.sqrt(self._length*self._length - delta_y*delta_y)
self._driven_end = wheel_end + vector(delta_x, delta_y)
return vector(delta_x, delta_y) / 2
end
function MainRod:drive_point()
return self._driven_end
end
function MainRod:position()
return self._position
end
My main objection here is that the driven end value is saved right in the middle of the center_delta method, sort of as an afterthought.
What should really happen, it seems to me, is that both _position and _driven_end should be computed at the same time, both based on the vector(delta_x, delta_y).
The flow should be: set up the base position and rotated position (which is the offset position of the crank on the wheel). Then compute <delta_x,delta_y>, then use it twice to compute the root and effector (driven_end) values.
Let’s make that happen. I’ll try to refactor in tiny steps, keeping the code working. It is tempting just to go ahead and make all the changes, copying, cutting, pasting, until it works. Small step refactoring is better.
We start here:
function MainRod:center_delta(base_pos, wheel_end)
local delta_y = base_pos.y - wheel_end.y
local delta_x =
- math.sqrt(self._length*self._length - delta_y*delta_y)
self._driven_end = wheel_end + vector(delta_x, delta_y)
return vector(delta_x, delta_y) / 2
end
The first two lines of this compute delta_x and delta_y. We can Extract Method to do this. First, we’ll return them separately to make the center_delta method happy:
function MainRod:center_delta(base_pos, wheel_end)
local delta_x, delta_y = self:compute_deltas(base_pos, wheel_end)
self._driven_end = wheel_end + vector(delta_x, delta_y)
return vector(delta_x, delta_y) / 2
end
function MainRod:compute_deltas(base_pos, wheel_end)
local delta_y = base_pos.y - wheel_end.y
local delta_x =
- math.sqrt(self._length*self._length - delta_y*delta_y)
return delta_x, delta_y
end
This should continue to pass the tests. Yes. Could commit. Now center_delta really wants the vectors anyway. Change it to expect a vector and change compute_deltas to provide it:
function MainRod:center_delta(base_pos, wheel_end)
local delta_xy = self:compute_deltas(base_pos, wheel_end)
self._driven_end = wheel_end + delta_xy
return delta_xy / 2
end
function MainRod:compute_deltas(base_pos, wheel_end)
local delta_y = base_pos.y - wheel_end.y
local delta_x =
- math.sqrt(self._length*self._length - delta_y*delta_y)
return vector(delta_x, delta_y)
end
Tests run. Commit again if you wanna.
I think compute_deltas is the wrong name. It is the vector from the driven end to the driving end. It is the effector offset. The driven_end_offset. Yes. Rename:
function MainRod:center_delta(base_pos, wheel_end)
local full_offset = self:driven_end_offset(base_pos, wheel_end)
self._driven_end = wheel_end + full_offset
return full_offset / 2
end
function MainRod:driven_end_offset(base_pos, wheel_end)
local delta_y = base_pos.y - wheel_end.y
local delta_x =
- math.sqrt(self._length*self._length - delta_y*delta_y)
return vector(delta_x, delta_y)
end
Better, I think. Tests run. Now I’m going to move the calculation of the actual _driven_end up to calculate, by calling driven_end_offset from there, and using it. I’ll remove the setting from the center_delta method, of course.
function MainRod:calculate()
local base_pos = self._parent:position()
local rotated = self._parent:adjusted_offset(self._offset)
local wheel_end = base_pos + rotated
local full_offset = self:driven_end_offset(base_pos, wheel_end)
self._driven_end = wheel_end + full_offset
self:compute_center(base_pos, rotated)
end
function MainRod:center_delta(base_pos, wheel_end)
local full_offset = self:driven_end_offset(base_pos, wheel_end)
return full_offset / 2
end
Tests run. Now calculate has the info it needs to compute _position as well. Make it so.
function MainRod:calculate()
local base_pos = self._parent:position()
local rotated = self._parent:adjusted_offset(self._offset)
local wheel_end = base_pos + rotated
local full_offset = self:driven_end_offset(base_pos, wheel_end)
self._driven_end = wheel_end + full_offset
self._position = wheel_end + full_offset / 2
end
Tests pass. compute_center, center_position, and center_delta are no longer referenced. Remove them.
Tests still pass. Here’s the new code:
function MainRod:calculate()
local base_pos = self._parent:position()
local rotated = self._parent:adjusted_offset(self._offset)
local wheel_end = base_pos + rotated
local full_offset = self:driven_end_offset(base_pos, wheel_end)
self._driven_end = wheel_end + full_offset
self._position = wheel_end + full_offset / 2
end
function MainRod:driven_end_offset(base_pos, wheel_end)
local delta_y = base_pos.y - wheel_end.y
local delta_x =
- math.sqrt(self._length*self._length - delta_y*delta_y)
return vector(delta_x, delta_y)
end
A few articles back, I argued that calculate, which wasn’t much more complicated than it is now, could and should be split into setting up and computing. We could do that. Right now, I’m not feeling it, though at six lines this is getting pretty long. I think we’ll call time right there.
Looking Back
We renamed our objects to match what seems to be official train terminology. Can’t hurt, might help.
We then reduced 23 lines of code to 15, untangling a bit of oddness along the way. We did it with tiny steps, almost all no more than moving one or two lines fo code to a new location. I don’t think I typed any full lines other than function definitions. And after each tiny change, we ran the tests, ensuring that we had not broken anything. Had a test failed, we would have known exactly what went wrong, since we were just a couple of lines different from what worked.
This is the path: a very safe, simple, and smooth path. It can be tricky to think of the tiny change, but it’s always worth it, in my book.
Safe paths!