Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New design for reset symbols in diagrams #1241

Merged
merged 8 commits into from
Jul 1, 2022
Merged

Conversation

a-sr
Copy link
Collaborator

@a-sr a-sr commented Jun 22, 2022

Fixes use of unicode symbols in state variables and revises reset trigger figure.
Closes #1219

Result:
reset
for

reactor R (param:int(0)){
    state s:int(0)
    reset state rs:int(0)
    reaction(reset) {==}
}

@a-sr a-sr added the diagrams Problems with diagram synthesis label Jun 22, 2022
@a-sr a-sr requested a review from lhstrh June 22, 2022 12:44
@lhstrh
Copy link
Member

lhstrh commented Jun 22, 2022

This looks very cool! One concern I have is that the reset arrow might be hard to spot on the state. Would it make sense to wrap the reset arrow all the way around the bullet rather than have it dangle off the side?

@a-sr
Copy link
Collaborator Author

a-sr commented Jun 23, 2022

I also thought about this but my feeling is that this would make the "bullet point" too large, resulting in a very inconsistent appearance with others.
Furthermore, I did not want to draw too much attention to it because, same as state variables and parameters in general, its relevance on the diagram level is rather limited because we do not show reactions bodies where this would have an effect.

@lhstrh
Copy link
Member

lhstrh commented Jun 24, 2022

The aspect ratio between the thickness of the arrow and the size of the dot is not quite as I meant, but here is a mock up of what it could look like:

image

I'm basically suggesting that the outer circle around the filled dot can be replaced by an arrow. That way, it would take up about as much space as an ordinary state icon.

@a-sr
Copy link
Collaborator Author

a-sr commented Jun 24, 2022

Ah, now I get it. Yes, this is a good a idea. I will redesign it accordingly.

@a-sr
Copy link
Collaborator Author

a-sr commented Jun 28, 2022

Adjusted appearance. Now it looks like this:
reset2

@edwardalee
Copy link
Collaborator

Maybe lose the dot in the middle of the reset bullet? This might make it look cleaner and distinguish it better (and also match the event icon better).

@a-sr
Copy link
Collaborator Author

a-sr commented Jun 29, 2022

Ok. Without the dot in the middle, it now looks like this:
reset3

@lhstrh
Copy link
Member

lhstrh commented Jun 29, 2022

Hm, I'm unsure about this one. I agree that the new version relates more obviously to the reset trigger, but it looks less like a state variable, too, which might be confusing. My idea was for the bullet to represent the notion of "state variable" and the ring around it to indicate whether it gets reset (broken, with arrow) or not (solid).

@a-sr
Copy link
Collaborator Author

a-sr commented Jun 30, 2022

I agree with @lhstrh, in the previous version the similarities between the sate variables were stronger, which also was my initial intention, to make the reset "only" a minor modification.
I can revert the last change, if we want to go with the previous design.

@edwardalee
Copy link
Collaborator

Yes, I see this point... let's revert.

@a-sr
Copy link
Collaborator Author

a-sr commented Jun 30, 2022

Ok, I reverted the commit. We are back to the previous appearance.

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@lhstrh lhstrh merged commit 2a8ef20 into master Jul 1, 2022
@lhstrh lhstrh deleted the diagram-reset-symbols branch July 1, 2022 02:53
@lhstrh lhstrh changed the title Adjust reset symbols in diagrams Redesign reset symbols in diagrams Jul 7, 2022
@lhstrh lhstrh added the enhancement Enhancement of existing feature label Jul 7, 2022
@lhstrh lhstrh changed the title Redesign reset symbols in diagrams New design for reset symbols in diagrams Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagrams Problems with diagram synthesis enhancement Enhancement of existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minor issues with diagrams of modal reactors
3 participants