Search Unity

  1. Looking for a job or to hire someone for a project? Check out the re-opened job forums.
    Dismiss Notice
  2. Good news ✨ We have more Unite Now videos available for you to watch on-demand! Come check them out and ask our experts any questions!
    Dismiss Notice

Feature Request Keyboard shortcuts are an essential and missing feature.

Discussion in 'Shader Graph' started by DigitalSalmon, Mar 23, 2020.

  1. DigitalSalmon

    DigitalSalmon

    Joined:
    Jul 29, 2013
    Posts:
    62
    This has been brought up many many times.

    Could we please have some keyboard shortcuts, like every other big boy shader editor since 2010?

    1,2,3,4,5 -> Vectors/Colours
    M - Mulitply
    O - One Minus
    S - Subtract
    A - Add
    D - Divide

    Are we waiting for integration with a new input package?
     
  2. Elizabeth_LeGros

    Elizabeth_LeGros

    Unity Technologies

    Joined:
    Jan 7, 2020
    Posts:
    50
  3. DigitalSalmon

    DigitalSalmon

    Joined:
    Jul 29, 2013
    Posts:
    62
    Do you guys not think this is an important feature? There is so much amazing work going into making SG really powerful, then the first thing i'm asked by every new artist on the team is "where are the keyboard shortcuts?"

    I'm just baffled that something like this would be considered a roadmap consideration, it's a bit like someone suggesting supporting normal maps or a multiply node...
     
    hampus_unity628 and Ng0ns like this.
  4. tweedie

    tweedie

    Joined:
    Apr 24, 2013
    Posts:
    303
    +1. It's pretty absurd Shadergraph doesn't have this, given how pervasive 1,2,3,4 are.

    That roadmap doesn't appear to have a place where we can view all the open suggestions, nor can I lend my support to an existing request. It would be nicer if there was a page where they could all be seen in one place and upvoted, to avoid duplicates, as well as discuss and converge on better versions of similar ideas. A bit like a forum.
     
    hampus_unity628 likes this.
  5. Elizabeth_LeGros

    Elizabeth_LeGros

    Unity Technologies

    Joined:
    Jan 7, 2020
    Posts:
    50
    I would say it is certainly a feature we want to support; its more a question of what we should be focusing on based on time/scheduling. This is something that has been discussed before, and unfortunately the solution is not something that can be accomplished quickly. So, then it becomes another task that we have to base priority on versus our other features/bugs we want to tackle. I say you should put it on the timeline because we are watching that timeline to see what features are most important to the community, and if this is something the community needs then we take that into consideration.

    You can vote on things on our roadmap, as well as add your own ideas! click on one of the things you think of as important and you can say how important it is to have!
     
  6. tweedie

    tweedie

    Joined:
    Apr 24, 2013
    Posts:
    303
    Thanks, I missed that. I still think it's a bit limiting, as I might like to upvote an issue to indicate its important to me, but explain why, or comment on details about a topic. For instance, a huge limitation of the built in pipeline is the inability for UI renderers to use MaterialPropertyBlocks, which was a huge frustration for many users. It would be good to be able to indicate on the UI roadmap item that this was a previous flaw we'd like to avoid. Either way I appreciate feedback on the feedback website isnt exactly relevant to this thread nor your responsibility :)

    Can you elaborate on why? Unless you're allowing remapping, or plan to integrate with / depend on another system in Unity like the new Shortcuts, this seems very trivial. I'm guessing there are good reasons why it isn't.
     
    hippocoder and Elizabeth_LeGros like this.
  7. DigitalSalmon

    DigitalSalmon

    Joined:
    Jul 29, 2013
    Posts:
    62
    Do you have any more information around why the solution can't be accomplished with relative ease? The MaterialGraphEditWindow has a reference to the GraphData, which contains an AddNode method which looks very well put together. I don't mean to over-simplify, and I appreciate how self-entitled I will be coming across, I'm just really struggling to see why this wasn't one of the first things to get done.

    I hate using UE4, but their material editor is crushing Unity, and it's not because of advanced features, it's because they have got the quality of life features like this.
     
  8. Elizabeth_LeGros

    Elizabeth_LeGros

    Unity Technologies

    Joined:
    Jan 7, 2020
    Posts:
    50
    When you vote on something, you get a chance to explain why its important to you too :)

    Building the system cleanly is just not as simple as it looks from the top level, unfortunately. There's a bunch of quirks about the back end to account for, and only so many engineers on the team to be able to work. Not being vague to be mean, just trying not to break any NDAs or anything :D.

    The roadmap is the best way to help inform us of what the community wants and gives us the right info to prioritize tasks for shadergraph
     
  9. DigitalSalmon

    DigitalSalmon

    Joined:
    Jul 29, 2013
    Posts:
    62
    I really appreciate you taking the time.

    Respectfully, I think I have to simply agree to disagree.

    Line 332 of GraphEditorView.cs

    Code (CSharp):
    1.  
    2. if (evt.keyCode == KeyCode.M) {
    3.     if (m_Graph != null) {
    4.         m_Graph.AddNode(new MultiplyNode());
    5.     }
    6. }
    7.  
    When you press M, a new multiply node is created. It functions as expected, and saves and loads properly (As far as I can test).

    It's clear the GraphView/RegisterCallback approach might not enjoy the combination of hold keyboard button + mouse press. I also fully understand that whacking in some lines of code isn't the same as developing and following a robust and consistent approach.

     
    Elizabeth_LeGros likes this.
  10. Elizabeth_LeGros

    Elizabeth_LeGros

    Unity Technologies

    Joined:
    Jan 7, 2020
    Posts:
    50
    You're not wrong that this is one approach that works; its the backend side that we find the quirks/complexity.

    That being said, this is a great way to add that functionality locally and if you are finding you need it now this is a good solution!

    BUT modifying package code does mean upgrades may not be fun later, just something to keep in mind.
     
  11. DigitalSalmon

    DigitalSalmon

    Joined:
    Jul 29, 2013
    Posts:
    62
    Code (CSharp):
    1.  
    2. internal class KeyboardShortcutHelper
    3.     {
    4.         private MaterialGraphView GraphView { get; }
    5.         private GraphData GraphData { get; }
    6.  
    7.         public KeyboardShortcutHelper(MaterialGraphView graphView, GraphData graph)
    8.         {
    9.             GraphView = graphView;
    10.             GraphData = graph;
    11.  
    12.             GraphView.RegisterCallback<KeyDownEvent>(OnKeyDown);
    13.         }
    14.  
    15.         void OnKeyDown(KeyDownEvent evt) {
    16.             if (GraphData == null) return;
    17.  
    18.             Vector2 pos = evt.originalMousePosition;
    19.  
    20.             switch (evt.keyCode) {
    21.                 case KeyCode.O:
    22.                     CreateNode(() => new OneMinusNode(), pos);
    23.                     break;
    24.                 case KeyCode.M:
    25.                     CreateNode(() => new MultiplyNode(), pos);
    26.                     break;
    27.                 case KeyCode.L:
    28.                     CreateNode(() => new LerpNode(), pos);
    29.                     break;
    30.                 case KeyCode.D:
    31.                     CreateNode(() => new DivideNode(), pos);
    32.                     break;
    33.                 case KeyCode.S:
    34.                     CreateNode(() => new SubtractNode(), pos);
    35.                     break;
    36.                 case KeyCode.A:
    37.                     CreateNode(() => new AddNode(), pos);
    38.                     break;
    39.                 case KeyCode.F:
    40.                     CreateNode(() => new FractionNode(), pos);
    41.                     break;
    42.                 case KeyCode.Alpha1:
    43.                     CreateNode(() => new Vector1Node(), pos);
    44.                     break;
    45.                 case KeyCode.Alpha2:
    46.                     CreateNode(() => new Vector2Node(), pos);
    47.                     break;
    48.                 case KeyCode.Alpha3:
    49.                     CreateNode(() => new Vector3Node(), pos);
    50.                     break;
    51.                 case KeyCode.Alpha4:
    52.                     CreateNode(() => new Vector4Node(), pos);
    53.                     break;
    54.                 case KeyCode.Alpha5:
    55.                     CreateNode(() => new ColorNode(), pos);
    56.                     break;
    57.             }
    58.         }
    59.  
    60.         private void CreateNode(Func<AbstractMaterialNode> createNode, Vector2 pos) {
    61.             AbstractMaterialNode multiplyNode = createNode();
    62.  
    63.             var drawState = multiplyNode.drawState;
    64.             Vector2 p = GraphView.contentViewContainer.WorldToLocal(pos);
    65.  
    66.             drawState.position = new Rect(p.x, p.y, drawState.position.width, drawState.position.height);
    67.             multiplyNode.drawState = drawState;
    68.             GraphData.AddNode(multiplyNode);
    69.         }
    70.     }
    71.  
    Add this to the top of GraphEditorView.cs

    Code (CSharp):
    1. KeyboardShortcutHelper shortcutHelper = new KeyboardShortcutHelper(graphView, graph);
    Add this to the end of the constructor for GraphEditorView

    -

    Any early warnings on which quirks this is going to hit would be appreciated so I can ahead of them before they bite me!

    This, if it continues to work, makes ShaderGraph fundamentally better. Would be totally down for cleaning it up + PR.
     
    Last edited: Mar 24, 2020
    curiouspers and florianBrn like this.
  12. Elizabeth_LeGros

    Elizabeth_LeGros

    Unity Technologies

    Joined:
    Jan 7, 2020
    Posts:
    50
    Nice solution! Like I said, this works great on a local user case.

    Part of the reason why we can't do this is simply because shadergraph is an official part of the Unity ecosystem, and so we need to keep consistent behavior with other systems. Take VFX graph: the solution here would need to work for both teams and so couldnt just be thrown in GraphData. Then we need to make sure these settings are exposed for accessibility and decisions need to be made about where those settings live (per graph, per project, etc). We then also need to decide how a node gets its own keyboard shortcut - can users specify one themselves? If so, what happens if they want a pipeline specific node to have a hotkey, and a different pipeline gets chosen? Etc etc

    We want to make shadergraph into the best tool it can possibly be, and that sometimes means not doing the quick code fix in favor of a total and robust fix later. Thats why I am so heavily encouraging adding this to our timeline - so we can give the problem the time it needs for a robust and great solution that meets community and Unity standards
     
  13. DigitalSalmon

    DigitalSalmon

    Joined:
    Jul 29, 2013
    Posts:
    62
    I think it's excellent that decisions are being thought out before action being taken, and that lots of use cases are being considered - It's a great approach to ensure ongoing commitments don't become technical debt.

    I think the goal of maintaining parity with VFX Graph is admirable, but the list of differences at present is testament to the fact that ultimately, the focus is on feature set before consistency. This is infinitely truer in the context of Unity's wider code base.

    If a feature can get a massive amount of the way towards covering 90% of users/use cases, without actively impeding your ability to extend or exchange it later, then I believe there is a strong case to be made that the decision Unity have made is the wrong one. One day of developer time to save countless hundreds of hours in the short-medium term is amazing value.

    I'll stop pushing! I appreciate I don't have the full picture. Thank you very much for taking so much time. Stay safe!
     
    Last edited: Mar 24, 2020
    qoobit likes this.
  14. edwon

    edwon

    Joined:
    Apr 24, 2011
    Posts:
    218
    How is there still not a save shortcut in the unity shader graph? This is essential
     
  15. curiouspers

    curiouspers

    Joined:
    Aug 7, 2014
    Posts:
    17
    +1 for this feature, also thanks DigitalSalmon for workaround
     
unityunity