nomeata / incredible

The Incredible Proof Machine
MIT License
358 stars 36 forks source link

Turn up snapLinks behavior (it's good, you don't have to hold back, you're allowed to have more) #144

Open makoConstruct opened 1 month ago

makoConstruct commented 1 month ago

You currently have snapLinks: true, in graph-interaction.js. This is good, it means the player doesn't have to be so precise about reaching the in ports, which allows them to move faster. I tried setting snapLinks: { radius: 300 }, and imo that was even better and enabled even more speed. I'd recommend trying this out!

There's one slight inelegance that this exacerbates a little bit, where links drawn out from blocks that have an in port will immediately wrap around and connect to themselves. Once a player is used to the game, they wont really notice this, because as soon as they start dragging from the outport they'll already be moving the mouse towards the in port that they want. For a new player it could be an aggravation, but it usually isn't, because the game starts out with out blocks that don't have an in ports. But you could fix this by returning false if vs === vt in validateConnection

  validateConnection: function (vs, ms, vt, mt, e, vl) {
      if(ms && mt && ms.getAttribute('direction') == mt.getAttribute('direction')){
        return false;
      }
      if(vs === vt){
        return false;
      }
      return true;
    }

But this would make it impossible to self-connect. In most jointjs applications that wouldn't be desirable (instead you'd want something nuanced where it doesn't self-connect with radius 300 but does at smaller radii), but it's possible that here there are no self connections in incredible? I'm not sure, are there any?

It would also be lovely if the user could start dragging from an out node without having to click down precisely on it, instead just clicking down nearest to the outport that they want. That would interfere a lot with canvas clicking to drag the view, but middle click also provides that function (and we can distinguish middle clicks from left clicks with event.button == 1), so it might be worth it. I've tried to trial this, but the elements that I'm getting in this.model.getElements() in the paper.on('blank:pointerdown', function (e, x, y) { scope don't have a getPorts method, so I'm unable to seek the nearest port on click. They have a ports field, but it's an empty object. I'm at a complete loss as to why.

nomeata commented 1 month ago

That's a great suggestion, the usability could certainly be improved.

Disabling self-links isn't particularly desirable (it's always a bad proof, but one that users should be able to try), but may be worth given the usability improvements.

I should try and experiment. Currently on a holiday trip, so not sure how soon I'll get to it.

makoConstruct commented 1 month ago

I don't think the self-links are too much of a problem.

I'm happy to try to implement this a little more today if you can offer any hints for finding the missing getPorts

nomeata commented 1 month ago

Sorry, but I haven't touched this code in almost a decade or so, by this time you know it better than I do. But the more the reason for you to keep digging :-)

makoConstruct commented 1 month ago

Presumably the reason jointjs didn't have getPorts was that it was ancient, so I updated it to 3.6.3 (first one I could find in a cdn resource search engine), and now getPorts is there, but this hasn't fixed the problem, there still seem to be no ports in the elements! And I'm pretty sure the elements correspond to the elements in the graph, since the number of them is always correct.

It's also silently broken the link deletion functionality (I can no longer delete links)

https://github.com/makoConstruct/incredible

I give up for now.

nomeata commented 1 month ago

Thanks a lot for trying! Personally I'm scared of updating that library, and hope I'm never forced to do it :-). Maybe someday I'll feel inspired or someone else has more luck than us…. Until then, thanks to nix, it at least keeps working.

makoConstruct commented 1 month ago

Oh, I should emphasize, then, we now have no reason to doubt that the ports object in the older version of jointjs probably works as well as the getPorts() method in the newer version (which also doesn't work) so I don't know if there's a need to update.