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

Loop refactor #2887

Merged
merged 16 commits into from
Nov 6, 2018
Merged

Loop refactor #2887

merged 16 commits into from
Nov 6, 2018

Conversation

johnhaddon
Copy link
Member

This follows on from #2880 and #2812, refactoring Loop from being a funky templated "mixin" class, into being a more straightforward single generic node.

@andrewkaufman
Copy link
Contributor

Travis won't let me restart the failed test for some reason...

Any chance you want to add a shiny rmb->setup menu item to drop down the necessary dots that are usually created manually to accompany any loop ?

Otherwise it's a bit fiddly to make the dots manually and get the nodules on the right edges. The resulting layout is a little, shall we say, jaunty, but I'd like to leave that as a future improvement for StandardGraphLayout. We could force the node positions to what we think they should be, but then we're missing the collision detection that StandardGraphLayout performance, and we're vulnerable to any changes in size/location of NodeGadgets and nodules.
@johnhaddon
Copy link
Member Author

Any chance you want to add a shiny rmb->setup menu item to drop down the necessary dots that are usually created manually to accompany any loop ?

Done in 628bffc. I've also pushed two more commits with minor fixes/enhancements.

@andrewkaufman
Copy link
Contributor

Jaunty dots! I'm not even going to download and test first. Lets just merge and be surprised.

@andrewkaufman andrewkaufman merged commit 3da7139 into GafferHQ:master Nov 6, 2018
@johnhaddon johnhaddon deleted the loopRefactor branch November 7, 2018 16:37
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