## Summary
PlotWidget.moveTo() and PlotWidget.position() called via the REST API resolve to the empty EventInterface implementations instead of the working Item
implementations. This makes it impossible to programmatically position or re-select PlotWidgets after creation.
Discovered while building a REST API automation layer for Minsky. Happy to submit a PR with the fix if preferred.
Minsky version: 3.23.0
## Reproduction
```bash
# Create a plot and move it
curl -s http://localhost:9999/minsky/canvas/addPlot
curl -s -X PUT -d '[0,0]' http://localhost:9999/minsky/canvas/getItemAt
curl -s -X PUT -d '["Test"]' http://localhost:9999/minsky/canvas/item/title
# moveTo appears to succeed (no error)
curl -s -X PUT -d '[400,200]' http://localhost:9999/minsky/canvas/item/moveTo
# But position hasn't changed
curl -s http://localhost:9999/minsky/canvas/item/x # returns 0
curl -s http://localhost:9999/minsky/canvas/item/y # returns 0
Compare with a Variable (works correctly):
curl -s -X PUT -d '["v","parameter"]' http://localhost:9999/minsky/canvas/addVariable
curl -s -X PUT -d '[0,0]' http://localhost:9999/minsky/canvas/getItemAt
curl -s -X PUT -d '[400,200]' http://localhost:9999/minsky/canvas/item/moveTo
curl -s http://localhost:9999/minsky/canvas/item/x # returns 400
Root Cause
Diamond inheritance with conflicting moveTo and position methods.
PlotWidgetSuper (plotWidget.h:41) inherits from both ItemT<plotwidget> and RenderNativeWindow:</plotwidget>
EventInterface Item
moveTo() {} [empty] moveTo() [sets m_x, m_y correctly]
position() {0,0} [stub] x(), y() [reads m_x, m_y with zoom]
| |
RenderNativeWindow ItemT<plotwidget>
| |
+---------- PlotWidgetSuper -----+
|
PlotWidget</plotwidget>
The classdesc REST binding resolves to EventInterface::moveTo (empty stub) rather than Item::moveTo (which correctly updates m_x/m_y).
Source locations
┌────────────────────────┬─────────┬──────────────────────────────────────┬───────────────────────────────────────────┐
│ File │ Line │ Method │ Behaviour │
├────────────────────────┼─────────┼──────────────────────────────────────┼───────────────────────────────────────────┤
│ model/eventInterface.h │ 38 │ EventInterface::moveTo(float, float) │ Empty body — does nothing │
├────────────────────────┼─────────┼──────────────────────────────────────┼───────────────────────────────────────────┤
│ model/eventInterface.h │ 40 │ EventInterface::position() │ Returns {0, 0} always │
├────────────────────────┼─────────┼──────────────────────────────────────┼───────────────────────────────────────────┤
│ model/item.cc │ 256-273 │ Item::moveTo(float, float) │ Correctly sets m_x, m_y with group zoom │
├────────────────────────┼─────────┼──────────────────────────────────────┼───────────────────────────────────────────┤
│ model/plotWidget.h │ 41-42 │ PlotWidgetSuper │ Inherits from both — creates the conflict │
└────────────────────────┴─────────┴──────────────────────────────────────┴───────────────────────────────────────────┘
Why only PlotWidget is affected
Variables and Operations inherit from ItemT<> only — no RenderNativeWindow in the chain. PlotWidget is the only item type with both parents, creating the vtable
ambiguity.
Impact
Suggested Fix
Override in PlotWidgetSuper or PlotWidget to delegate to the Item versions:
// plotWidget.h
void moveTo(float x, float y) override {
Item::moveTo(x, y);
}
std::vector<float> position() const override {
return {Item::x(), Item::y()};
}</float>
Alternatively, if classdesc resolution can be controlled, ensure the REST binding dispatches through the Item vtable when the selected item is a PlotWidget.
Anonymous
Good catch! I was wondering why this didn't bite us with Canvas event handling, but in that case, the PlotWidgets are manipulated through the Item interface, so it resolves correctly. But as you point out, with classdesc, the latest declaration trumps any earlier ones in the declaration stack, which means that the RenderNativeWindow intrerface (with the empty implementations) trumps the Item implementation. I think your proposed solution is the simplest, and probably most correct solution. Another that comes to mind would be to derive Item from EventInterface, and arrange for virtual inheritance, but not only is that more complex to reason about, virtual inheritance tends to create more problems than it solves, almost to the point that it is considered a code smell.
Happy to take a PR for this!
Just a brief note to say I've approved the PR, but I stuffed up the CI
pipeline in generating a Windows release. I ran out of time yesterday
trying to find out a solution. I'll merge the PR once master is green again.
I going to have to create a new releases branch, so that fixes for
quirks on particular platforms are quarantined in that and not break
the build for the master branch. I've learned my lesson.
Cheers
--
Dr Russell Standish Phone 0425 253119 (mobile)
Principal, High Performance Coders hpcoder@hpcoders.com.au
http://www.hpcoders.com.au