big subtle bug :D

Forums 💬 NodeCanvas ⚙️ Support big subtle bug :D

Viewing 4 posts - 1 through 4 (of 4 total)
  • Author
    Posts
  • #18288
    bcristian
    Participant

      Hello
      Just found a very insidious bug – easy to fix though.
      The children nodes are sorted (in the parent’s children collection) by the X coord of their centers.
      Yet they also auto-size.
      So if arranging the nodes like a step ladder, it is possible to unknowingly change their order by doing anything that changes their label, such as renaming variables :D.

      I’ve changed in EDITOR_Node.cs:746 from “c.targetNode.nodeRect.center.x” to “c.targetNode.nodeRect.xMin”.

      #18291
      Gavalakis
      Keymaster

        Hey,

        I see what you mean. I don’t think that qualifies as bug though 🙂 More like a design choice. 🙂
        Please let me explain why it’s made to make use of center rather than X.

        Here is a screenshot of how it works now. Visually, the connection lines and the position they are touching the nodes, indicate which node is first and which one is second.
        [attachment file=”PosXCenter.png”]

        If we change the sorting to be done by the nodeRect.x rather than center.x, then the connections (in some cases) no longer reflect the order of the nodes and can lead to visually being criss-crossed, which I think can be quite confusing.
        I really believe that in the following image, the top action node should be first in order 🙂
        [attachment file=”PosXMin.png”]

        Is such a layout that you have and mean by “step ladder”?

        #18290
        bcristian
        Participant

          Something like that, yes.
          I see your point about the lines, haven’t considered that.
          Yet, I still consider that renaming variables causing execution order changes is a bug 😀
          Maybe a better fix in this case would be to change resizing so that it leaves the top-center fixed, instead of the top-left corner?

          #18289
          Gavalakis
          Keymaster

            I see your point too here 🙂
            I will investigate this further. Your suggested solution (scale from center) should work. I will test it out and see
            Thanks 🙂

          Viewing 4 posts - 1 through 4 (of 4 total)
          • You must be logged in to reply to this topic.