sverweij / state-machine-cat

write beautiful state charts :scream_cat:
https://state-machine-cat.js.org
MIT License
811 stars 45 forks source link

Nested states that transition to themselves look weird #28

Closed LevelbossMike closed 6 years ago

LevelbossMike commented 6 years ago

Thanks for this great project and sorry for the strange title. When nested states reference themself the rendered outpout looks weird and it's hard to see what is happening in the statechart:

bildschirmfoto 2018-10-18 um 19 17 40

Expected Behavior

I would expect that a transition from a nested state to itself looks the same as a non nested-state to itself.

Current Behavior

The transition of a nested state to itself is drawn inside of the nested state which is easy to miss when looking at the diagram.

Possible Solution

Not sure

Steps to Reproduce (for bugs)

This is the diagram I am drawing:

initial,
"media player off",

"media player on" {
  stopped, playing, paused;

  stopped => playing : play;
  playing => stopped : stop;
  playing => paused  : pause;
  paused  => playing : pause;
  paused  => stopped : stop;
};

initial            => "media player off";
"media player off" => stopped           : power;
"media player on"  => "media player off" : power;
"media player off" => "media player off";
"media player on" => "media player on";

Context

I'm trying to build a visualizer for an ember-addon https://github.com/LevelbossMike/ember-statecharts that I am working on and would like to use state-machine-cat as the project that creates the visual output.

Your Environment

I'm using the web-editor.

Thanks again for this great project! I'm using your diagramming tool nearly every day and it makes my dev-life much easier 🍻

sverweij commented 6 years ago

@LevelbossMike thanks for raising this issue, and yes, that needs to be improved. (also the self-transitions from non-nested entities have become a bit too tiny for my taste since the last major change). I'll try to tweak the output into submission.

Your add-on looks cool and very useful 👍 - wish something like this would exist for angular...

(And thanks for the compliments 😊 )

sverweij commented 6 years ago

Intermediate update: below you see the current situation and what I came up with after some graphviz trickery:

ns {
    s => s: hi;
};

ns => ns: hi;

before after

LevelbossMike commented 6 years ago

this looks great! Is this already released? I still see the old behavior in the web-editor

bildschirmfoto 2018-10-22 um 09 50 08
namibj commented 6 years ago

I found a solution for this issue, but it seems to only be applicable to fdp. You need to use splines=compound instead of splines=true, and the state transitions/edges have to start/end on the "cluster_" node instead of the invisible helper node inside the cluster, as is currently done. The lhead="cluster_" inside the brackets has to/should be removed. It might be necessary to also remove rankdir=LR and add overlap=scale, at least those seem to have solved it for me.

sverweij commented 6 years ago

Thanks @namibj! - I'll tweak the fdp output specifically to include you recommendations in a separate PR. (I wish we could do the same for dot!)

For the dot (and probably the osage) algorithm I'm afraid we're stuck with another helper node to guide the self transition outside the clustered node...

sverweij commented 6 years ago

@LevelbossMike I haven't released it yet - have to do some juggling in the dot reporter to get to the result. I'll let you know in this thread when I have something testable ready.

sverweij commented 6 years ago

hi @LevelbossMike - a ping as promised: the PR above tries to fix the issue. I've published a first beta version:

I've already found two kinks to iron out (see the PR - @namibj already pointed out one of em) - but am curious if this works well enough for your scenario(s).

sverweij commented 6 years ago

Update: just deployed a fix for the first of the two kinks (with direction left-right and right-left self transitions to composite states look sorta ok now as well) and published them on npm (state-machine-cat@4.1.5-beta-2) and on https://state-machine-cat.js.org/dev.

LevelbossMike commented 6 years ago

@sverweij looks great :beers: 👍

sverweij commented 6 years ago

@LevelbossMike I've merged the PR, released to npm as 4.1.5 and to the main branch on state-machine-cat.js.org.

Thanks again for raising the issue and for the prompt feedback!