Consider output error for connectability#1725
Consider output error for connectability#1725RunDevelopment wants to merge 2 commits intochaiNNer-org:mainfrom
Conversation
|
In general i think the idea is good, but yeah i can see how it might "trap" users. Maybe we could have a popup instead that says it's invalid, but give them the option to cancel or remove the other connections making it invalid? |
|
Any more thoughts on what we should do with this? |
|
The user experience with the feature is not good. It's too restrictive. Instead of disallowing connections that will cause errors, I think it might be better to allow it, but show a warning. Not sure about how though. |
|
We could highlight the handle and edge with red maybe? We specifically haven't used red yet so it could be used for error stuff |
d0e4684 to
5007ca9
Compare
Resolves #1673.
If PR makes it so that inputs are not connectable of the assigned type would cause an output error. So if the target node is valid (no output errors), but the new connections would create output errors, then we don't allow that connection. Examples:
While this enforces the general idea "if it's connectable, it's valid," it also makes it a little harder to edit chains.
E.g. in the Guided Upscale example, I initially had source and guide the other way around by accident. Chainner would not let me assign the other input with the intended value until I removed the old (invalid) connection.
Frankly, I don't know whether I like this. A new user might not realize that they have to remove old invalid connections before they can connect their intended value. This might "trap" new users and lead to frustration. But maybe I'm overthinking here.
Please test this feature and see whether this feature is desirable.