Skip to content

Add ability to configure graph rendering#80

Closed
codesoap wants to merge 2 commits intoqmuntal:masterfrom
codesoap:master
Closed

Add ability to configure graph rendering#80
codesoap wants to merge 2 commits intoqmuntal:masterfrom
codesoap:master

Conversation

@codesoap
Copy link
Copy Markdown
Contributor

@codesoap codesoap commented Jul 10, 2024

In my use of stateless' ToGraph method I have found that the graphs sometimes get confusing. Coloring some "special" triggers differently has helped me make the graphs less confusing. For some scenarios (especially ignored transitions), I can even imagine that omitting such special triggers could be useful.

This pull request adds the ability to configure the rendering of ignored, reentrant and internal transitions.

Demo 1: Default rendering

When the new config is not set, the graph generated in example_test.go looks as usual:
demo1

Demo 2: Coloring internal transitions

With phoneCall.SetGraphConfiguration(stateless.GraphConfiguration{InternalTransitionColor: "blue"}) the internal transitions are accentuated:
demo2

Demo 3: Omitting internal transitions

With phoneCall.SetGraphConfiguration(stateless.GraphConfiguration{OmitInternalTransitions: true}) the internal transitions are omitted from the graph:
demo3

@codesoap
Copy link
Copy Markdown
Contributor Author

codesoap commented Jul 16, 2024

I've now added another commit, which changes the color's types from string to color.Color. I think this is a little more idiomatic, although admittedly a little more cumbersome, since color names are only defined in another, external dependency: golang.org/x/image/colornames

Demo 2 would now work like this:

blue := color.NRGBA{R: 0, G: 0, B: 0xff, A: 0xff}
phoneCall.SetGraphConfiguration(stateless.GraphConfiguration{InternalTransitionColor: blue})

@qmuntal
Copy link
Copy Markdown
Owner

qmuntal commented Aug 21, 2024

Thanks for the sugestion. I'm reluctant to add this complexity without a good reason. On the other hand,I agree that the current way if rendering multiple internal transitions is confusing, as they tend to overlap.

Would it solve this issue for you improving how multiple internal transitions are rendered? I'm thinking on combining them in a single reentring line while keeping a each transition name in a separate line. Thoughts?

@qmuntal
Copy link
Copy Markdown
Owner

qmuntal commented Aug 23, 2024

See #84.

@codesoap
Copy link
Copy Markdown
Contributor Author

Thanks for your response. The change from #84 looks very cool, but I'm afraid it doesn't solve my problem or even makes it slightly worse. I probably didn't explain it well enough, so let me try with a better example:

Let's take this small statemachine:

var (
	triggerEncounterProblem = "EncounterProblem"
	triggerCry              = "Cry"
	triggerResolveProblems  = "ResolveProblems"
	stateHappy              = "Happy"
	stateUnhappy            = "Unhappy"
)
mood := stateless.NewStateMachine(stateHappy)
mood.Configure(stateHappy).
	Permit(triggerEncounterProblem, stateUnhappy)
mood.Configure(stateUnhappy).
	Permit(triggerResolveProblems, stateHappy).
	PermitReentry(triggerEncounterProblem).
	Ignore(triggerCry)

From it, this graph can be generated:
mood_demo

When the mood is Unhappy there is a self-loop for the reentrant trigger triggerEncounterProblem and the ignored trigger triggerCry. The two different types of triggers cannot be distinguished. I think the difference between a reentrant trigger and an ignored trigger can be very significant; that's why I want to display them in different styles.

In some scenarios, with big and confusing graphs, I could even imagine omitting the (somewhat unimportant) ignored triggers altogether.

@qmuntal
Copy link
Copy Markdown
Owner

qmuntal commented Aug 24, 2024

Your example is clarifying. I get your point that there is value on differentiating ignored triggers from reentrant triggers.

I will make a bit more push back here, though. IMO drawing both trigger types in a way that can be easily differentiated should be the default behavior, not something that one get by customizing the graph (which would then need some legend to know what is what).

One solution would be to group triggers by category under a header that names the trigger type. Another would be to prepend the trigger type -possibly using using an abbreviation- to the trigger name. I'm open to ideas.

I'm not closing the door to having a way to configure the graph, I like the idea having a knob to hide the ignored transitions, for example.

@codesoap
Copy link
Copy Markdown
Contributor Author

codesoap commented Aug 25, 2024

Sound's good! I had prematurely assumed, that you would be opposed to a general change on how graphs are rendered. I have now opened #86 where I implement your suggestion to use headers for trigger groups.

Omitting ignored transitions is not yet possible, but this has a lower priority for me and could still be added later.

@codesoap
Copy link
Copy Markdown
Contributor Author

Closing because of #86.

@codesoap codesoap closed this Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants