armory3d / armory

3D Engine with Blender Integration
https://armory3d.org/engine
zlib License
3.07k stars 316 forks source link

Logic node improvements #1835

Closed MoritzBrueckner closed 4 years ago

MoritzBrueckner commented 4 years ago

Hi,

this issue is a continuation of the discussion started in https://github.com/armory3d/armory/issues/1786. In my opinion, there a quite a few things about logic nodes that need discussion and very likely some improvements. I'm collecting them in this issue:

What do you think?

knowledgenude commented 4 years ago

This can give more ideas to split Action and Value:

Transform - everything that is related to transforms interaction except variables (including set rotation, get rotation, scale, translate, separate transform, etc.)

Variable - everything that is considered a variable (with an input and output)

Output Variable (i don't know a better name) - everything that is considered a variable(?) without output , like Dynamic, Global Object, Mesh, etc.

Condition - Gate, Branch, Is True, Is False, etc.

Animation - As it don't have tendency to be bigger and bigger, can include from object animation to bone animation and frames related.

Materials - Everything related to materials

Set Camera Properties and Set Active Camera i think is an intuitive name, and follow the Blender nomenclatures. Set Camera is very vague, like "set camera of what?" . I never used Arrays, but by "Remove Array" and "Remove Array Value" i don't think in nothing. Maybe "Array Clear" and "Array Remove" should be better.

Note: In Array is out of alphabetic order

I think the alphabetic order should not be considered. Instead we can put more intuitive names and use the list name as reference. But with almost 300 nodes it will not be a fast work :)

Maybe is better to create all lists names and after this, move the nodes to it slowly. We can define a limit of list names (and divide all nodes between these lists. (20 lists should fit good in the screen).

niacdoial commented 4 years ago

The one thing I really have to say about this is that, if we want to have a custom menu (with separators, subdirectories and a partial disregard of alphabetical order), we need to build its API in a way that doesn't compromise user-extendability, which means that we need an easy way to register both new nodes and new node categories.

Talking about the contents of #1786, I think the node replacement system could benefit from individual nodes declaring a version number, in order to "upgrade" them.

With the re-organized menu, I think there will also be less of a need to consolidate small nodes into bigger, more generic ones. Fusions like "Keyboard"+"OnKeyboard" would still be good, but the simple "Sleep" node doesn't need to be fully replaced by the "Generic Timer" thing, for example.

Another change I would like to see personally is a "full" revamp of the way rotations are treated, with a special socket type, and a way to construct/decompose them to/from quaternions, eulers or axis-angles. Honestly though, it might need to wait for pretty much everything else to be done.

Another thing to consider is dynamic socket number/type/name for some nodes (for example, have a single output to the "Vector Math" node, that can change name and type depending on the node's operation). Should we do this more, or would it be considered too unintuitive (especially for changing the name of inputs, since the change itself might be hidden by the node's settings) ?

I might add things (probably by editing this post), but right now it's somewhat late. Also feel free to tell me if something wasn't clear.

MoritzBrueckner commented 4 years ago

Thanks for your ideas!

Transform - everything that is related to transforms interaction except variables (including set rotation, get rotation, scale, translate, etc. separate, etc.)

Sounds very good.

Variable - everything that is considered a variable (with an input and output)

Output Variable (i don't know a better name) - everything that is considered a variable(?) without output , like Dynamic, Global Object, Mesh.

I don't think that it would help to split the Variable category any more, it has a good size in my opinion. The same goes for the Logic category. In my opinion, "Value" would be a much better name for the "Variable" category but that is already taken^^

Materials - Everything related to materials

+1

Set Camera Properties and Set Active Camera i think is an intuitive name, and follow the Blender nomenclatures. Set Camera is very vague, like "set camera of what?"

Properties sounds good. I think this doesn't need much discussion, I will open a PR later :)

I never used Arrays, but by "Remove Array" and "Remove Array Value" i don't think in nothing. Maybe "Array Clear" and "Array Remove" should be better.

I would call it "Remove Array by Index" and "Remove Array by Value". Or would it make more sense to combine the nodes and add a switch to toggle between the nodes? This would break compatibility.

Note: In Array is out of alphabetic order

What do you think of "Array Contains"?


My ideas for categories so far (unordered, changed/new categories only):

It's far from perfect, there are so many nodes that don't fit well in categories...

MoritzBrueckner commented 4 years ago

The one thing I really have to say about this is that, if we want to have a custom menu (with separators, subdirectories and a partial disregard of alphabetical order), we need to build its API in a way that doesn't compromise user-extendability, which means that we need an easy way to register both new nodes and new node categories.

Absolutely! There should be an API that allows adding categories (with icons and automatic icon usage of nodes), registering nodes etc. Armory would benefit a lot from that.

Talking about the contents of #1786, I think the node replacement system could benefit from individual nodes declaring a version number, in order to "upgrade" them.

Yes, the things you mentioned on discord look a lot like you're working on something like that? It would be awesome to have such functionality, even better if it is easy to maintain. If you didn't know it, there is a replacement system that's no longer used in nodes_logic.py, maybe you can combine it with your metaclass magic which I don't understand anything of^^

With the re-organized menu, I think there will also be less of a need to consolidate small nodes into bigger, more generic ones. Fusions like "Keyboard"+"OnKeyboard" would still be good, but the simple "Sleep" node doesn't need to be fully replaced by the "Generic Timer" thing, for example.

+1

Another change I would like to see personally is a "full" revamp of the way rotations are treated, with a special socket type, and a way to construct/decompose them to/from quaternions, eulers or axis-angles. Honestly though, it might need to wait for pretty much everything else to be done.

That would be awesome!

Another thing to consider is dynamic socket number/type/name for some nodes (for example, have a single output to the "Vector Math" node, that can change name and type depending on the node's operation). Should we do this more, or would it be considered too unintuitive (especially for changing the name of inputs, since the change itself might be hidden by the node's settings) ?

That's a good question. It should always be as clear as possible (or at least explained somewhere in the wiki). It might be problematic if you are new to Armory and see a screenshot but the node in your logic tree looks different than on the screenshot. Maybe that should be decided case-by-case.

knowledgenude commented 4 years ago

"Array Contains" sounds good, reminds me of Get Child "Contains".

If we have some lists well separated, it will be useful for new nodes. Maybe if we create a new list for example to the "Output Variables", currently there is not a need for this, but what about the possibilites of nodes becoming for this list in the future? Anyway in this case i don't see too much need for this specific list. The truth is that currently we have a good amount of nodes, we can do almost everything. Is a good time to organize them. I would like to help in what i can and i have too much free time this year. Unfortunately i am not able to send some kind of PRs properly, but i can send files to another one send the PR.

Just because you will rename some nodes, maybe you would like to rename Cast Physics Ray to Ray Cast too? I can do it but i can't edit the .py/.hx files (is the case where i can't delete files in PRs)

knowledgenude commented 4 years ago

@niacdoial In the case of On Keyboard and Keyboard nodes, i couldn't see why we have both? Keyboard looks more complete and i don't see any performance difference between them.

niacdoial commented 4 years ago

@knowledgenude That was exactly what I meant. Those two nodes can be fused with each other without any problem. It is just that some other fusions are less needed now that node categories are more clear.

MoritzBrueckner commented 4 years ago

I did a quick concept of which nodes could go into which category (only the new proposed categories are written down): https://pastebin.com/raw/7kKNsCiu (Old version: https://pastebin.com/raw/9JmQ3MXk) What do you think? I separated nodes in one category which could fit into one section (delimited with separators like in the screenshot in the first post) with a blank line, Nodes with a ? before them might fit but also might not fit into the category. At the bottom is a list of all nodes from the Action and Value categories that currently don't fit in.

@luboslenco Are you fine with such big changes? This should not destroy your work and the thoughts you put in. If you have ideas, let us know :)


Also, I thought a bit about the node menu and the API @niacdoial mentioned. Currently, a node is registered like this:

add_node(SpawnCollectionNode, category='Scene')

I would propose to add another parameter section to that call:

add_node(SpawnCollectionNode, category='Scene', section='collections')

This section parameter puts the node in one section (like explained above) to group similar nodes visually in the menu.

To add categories and sections, there could be functions like this which would be called in __init__.py of the respective node package:

# Adds a section of categories called 'Data' to the menu to group multiple categories together visually
# (the section name is not visible from the UI, it's just for internal usage).
add_category_section('data')

# Adds a category named 'Scene' to the menu under the section 'Data'
# If no section is given, there should be a default section
add_category('Scene', section='data', icon='SCENE_DATA')

# Adds a section of nodes in the 'Scene' category that is delimited by separators
# The section name is not visible from the UI again, its just the section ID
add_node_section('Scene', section='collections')

Should the sections be ordererd alphabetically or in order of declaration? We would need to make sure that Armory's sections and categories are registered first but I think it would be good to give node creators that freedom.

knowledgenude commented 4 years ago

The list you do is very intuitive and organized, for me is almost perfect. :1st_place_medal:

About your doubts, i have some suggestions:

Yet to sort in: From the Action category:

  • Call Function - Logic
  • Call Node Group Logic
  • Node Group Output Logic
  • Print native?
  • Send Event (Event category)
  • Send Global Event (Event category)
  • Set Light Color (Light category?) Properties
  • Set Light Strength Properties
  • Set Parent Bone Animation
  • Set Property Properties
  • Set Time Scale Properties
  • Set Variable (difference between variables and properties should be made clear!) Variables
  • Sleep Logic
  • Timer Logic
  • Write Storage Native

From the Value category:

  • Compare Variables (note here: days using armory and i don't even know about the existence of this node)
  • Display Info Properties
  • Get Distance Properties
  • Get Property Properties
  • Get World Properties
  • None Variables
  • Read Storage Native
  • Screen To World Space Numbers
  • Separate RGB Properties
  • Time Native
  • Volume Trigger Variables
  • Window Info Properties
  • World To Screen Space Numbers

Properties category include properties from objects/scene/etc. These are not specific

Also i would like to recommend the change of Logic category name, because everything inside the tree is a logic part? I don't have any idea about a name for this.

About searching for nodes manually, i think in some cases the alphabetical names hinders me. I always search for "Set Canvas Text" and after, i realize that the correct is "Canvas Set Text". I don't know if this happens just to me, i am just punctuating this fact.

niacdoial commented 4 years ago

I am looking at the node list right now, (and wondering about hypothetical "World" and "App" categories for the remaining nodes).

Just to clarify, would the "categories" be the "directories" of the new menu while the "sections" would be groups of nodes inside those categories? (like in this image)

Whether I got that right or wrong, I think that if the name isn't visible in the UI, then the order should be the one in which the things are registered

MoritzBrueckner commented 4 years ago

Thanks for your answers again :)

Parse float fits good in Numbers as you said.

The question is if it still fits if we rename Numbers to Maths as I would suggest. Numbers sounds too vague but ParseFloat doesn't really fit into the Maths category.

Regarding the Logic category: the name is adequate because it represents logic as a field in mathematics but I agree that it might be a bit irritating for beginners. I think they will get used to this^^

About searching for nodes manually, i think in some cases the alphabetical names hinders me. I always search for "Set Canvas Text" and after, i realize that the correct is "Canvas Set Text". I don't know if this happens just to me

I totally agree on this, this happens all the time to me as well. It's Blender's fault, but we could try to implement our own search operator. But, as you can see in the screenshot above, it doesn't seem to be possible to remove the default operator.

Regarding the Volume Trigger node: There is a "On Volume Trigger" node that is in the event category, we should keep them together but obviosly the first one doesn't fit in that category. I would say it fits into Physics or Transform, internally it uses transform stuff so maybe we should put it there.

Maybe the "Properties" category should be "Miscellaneous" or "Other"?

wondering about hypothetical "World" and "App" categories for the remaining nodes

Also pretty good ideas! Window Info, Time etc. would fit very well into an "App" category.

Just to clarify, would the "categories" be the "directories" of the new menu while the "sections" would be groups of nodes inside those categories?

Exactly :) Sorry, I was a bit unclear about that.

knowledgenude commented 4 years ago

@MoritzBrueckner , Miscellaneous is better in my opinion

MoritzBrueckner commented 4 years ago

It works :)

node_menu_preview

The best thing: it's fully dynamic and the nodes now even automatically use the icons from their categories. The icons are far from perfect and it's just for demonstration (you can see the old categories are still there).

What doesn't work anymore is the search operator and I don't know why. Need to do some research about that.

knowledgenude commented 4 years ago

This is better than i expected :+1:

I found others nodes that maybe need to be renamed: Set Gravity and Set Gravity Enabled. Is not better Set World Gravity and Set Object Gravity respectively?

luboslenco commented 4 years ago

@luboslenco Are you fine with such big changes?

Sure! If it's for the better we should not hold back, and I am happy other parts of Armory are moving forward while I focus on the lower level internals.

niacdoial commented 4 years ago

Oh, I think I understood what the add_category_section, add_category, and add_node_section do. It took me an embarrassingly long time to realize there were both sections of categories and sections of nodes. er... do you think it would make sense to rename the "section" parameters of add_category and add_node to "category_section" and "node_section" respectively? Or do you think it would make it more difficult to remember?

Also, I think the order of parameters could be more similar between add_category_section and add_node_section. what do you think of this?

add_category_section(name='data')
add_node_section(name='collections', category='Scene') # note that I replaced the "section" parameter by the "name" parameter

Also, a little late, but +1 on the "capitalization" of sections when compared to the "Capitalization" of categories.

knowledgenude commented 4 years ago

@MoritzBrueckner , what about the possibility to add some kind of description file for what each node? User can edit the description for what is better to understand and when a new node is created, the author can write some description instead of going to the wiki.

The descriptions can be ported to Armory wiki, then when any node is added/removed, no need to update the wiki.

MoritzBrueckner commented 4 years ago

do you think it would make sense to rename the "section" parameters of add_category and add_node to "category_section" and "node_section" respectively?

Yes, definitely. The current parameter names can cause a lot of confusion.

Also, I think the order of parameters could be more similar between add_category_section and add_node_section.

I'm not sure what exactly you mean, for me it looks pretty similar? Or do you mean the fact that I used the parameter in add_category_section as a positional one and in add_node_section as a named one?

A parameter called name would be very good too, I think this keeps the confusion level low.

@knowledgenude I'm not sure what exatly you mean by that. Yes, it is possible to document the nodes directly in the code with a detailed docstring and it even would be possible with a bit of work to automatically update the wiki according to this (apart from the screenshots).

It would also be very helpful to display the first sentence of the docstring as a tooltip but I'm not sure yet how to achieve this, maybe we need to add a new operator with a dynamic description that calls the add_node operator.

knowledgenude commented 4 years ago

@MoritzBrueckner , As a noob, i mean that like we have .hx/.py files, maybe an additional file dedicated to description linked to .py file should be interesting. A file that can be viewed inside Blender and ported to the wiki without too much work. Probably you know a better way to do this.

niacdoial commented 4 years ago

I'm not sure what exactly you mean, for me it looks pretty similar? Or do you mean the fact that I used the parameter in add_category_section as a positional one and in add_node_section as a named one?

er... I think I kinda mixed a lot of things in what I wanted to say. Sorry. I guess I wanted to make the default order of the arguments of add_category more similar to the rest (the thing being registered comes in first, its position comes in second) I have no idea how to only allow a given argument as a keyword argument / only allow it as an ordered argument, and I was assuming we wouldn't do that, so I tried to showcase both the default order and the names of the arguments.

MoritzBrueckner commented 4 years ago

@MoritzBrueckner , As a noob, i mean that like we have .hx/.py files, maybe an additional file dedicated to description linked to .py file should be interesting. A file that can be viewed inside Blender and ported to the wiki without too much work. Probably you know a better way to do this.

I would indeed use docstrings for this. If combined with https://github.com/armory3d/armory/issues/1820 it would be easy to open the documentation. Automatically pushing changes to the wiki is possible but it would require a bit of work and probably lubos help for the github actions and a bot. This would also allow to add another context menu entry "Open wiki page" that jumps directly to the documentation of the selected node. But I think it's a bit too much for now^^

I guess I wanted to make the default order of the arguments of add_category more similar to the rest (the thing being registered comes in first, its position comes in second)

Let's do that :)

I have no idea how to only allow a given argument as a keyword argument / only allow it as an ordered argument

Me neither (maybe with **kwargs), and we shouldn't do that you're right, this was just my misunderstanding of the post above.

@ all:

I found out why the search menu doesn't work anymore. If you don't properly register a node category to nodeitems_utils.py (which is an undocumented Blender script) the nodes won't appear in the search menu. The problem is: we no longer use the category registering process used by that module. There are two solutions:

  1. Find a good but hacky way to add the node categories to the internal data structure (it's accessible via Python, so that would work). It's not the nicest solution but it's easy.

  2. Write a custom search operator. This makes it possible for example to also let the user search for categories and we could implement a search that doesn't depend on the word order (see the last paragraph of this comment: https://github.com/armory3d/armory/issues/1835#issuecomment-687223966). I don't think it is possible to easily overide the "Add node" menu so there would probably be the case of two search entries like in the screenshot in the first post. Maybe there is a way to somehow get around that but I'm not sure.

Edit: I think I got a custom add node menu working. Need to test it a bit more... Edit 2: Nah it's not good this would also require us to override the node editor header... It's easy but do we really want so much overriding of Blender stuff? we have to keep it up to date with Blender then... Edit 3: Found a good solution (overriding the menu works)

MoritzBrueckner commented 4 years ago

I got everything working (currently with Blender's default search but a custom one shouldn't be that difficult to create), nodes are now in subpackages based on their category which makes the logicnode package a lot clearer. How do you think the files should be named? Currently it's like logic_is_true.py for example. Should it be logic/is_true.py, logic/is_true_node.py or logic/node_is_true.py (or something else?) Should we keep it the way it is? Renaming the files will not break compatibility.

Now to the category icons. This is the current state (Action/Value/Variable will be removed or somehow replaced): logicnode_icons

What do you think? You can activate the Development: Icon Viewer add-on to get an overview over all available icons (use the search menu and search for Icon). The icons should be similar in regards to line thickness/amount of white and details so that no category steps out of line (for example, this was the case with the Input category icon in the screenshot a few posts above).

A few alternatives:

knowledgenude commented 4 years ago

My suggestions:

MoritzBrueckner commented 4 years ago

Thanks!

Regarding the last three categories in your list: those are likely to be removed when all nodes are sorted in, but lets see.

@niacdoial What do you think? Also, do you have ideas regarding a naming scheme for Python node files (see post above)?

knowledgenude commented 4 years ago

For Logic category i don't have any insight. I would recommend a icon that reminds a branch, like this one: Screenshot_20200907-175806~2.png

MoritzBrueckner commented 4 years ago

In logic nodes, there is a "Variable" functionality: certain nodes can function as a variable and store values for the current trait. Even if they only have an output, it's possible to set their values via the Set Variable node. With our reassignment of categories, some variable nodes got into other categories, so I have a proposal of how to ensure that this functionality isn't forgotten (apart from documenting it).

The proposal is to add another socket type (or better: a socket variation) for variables. Instead of the current way of adding sockets

self.outputs.new('NodeSocketBool', 'Bool')

there could be two new methods in the ArmLogicTreeNode type:

def add_input(self, socket_type: str, socket_name: str, default_value: Any = None, is_var: bool = False) -> bpy.types.NodeSocket:
def add_output(self, socket_type: str, socket_name: str, default_value: Any = None, is_var: bool = False) -> bpy.types.NodeSocket:

The default_value parameter would make the code more readable as it replaces often used constructs like this:

self.inputs.new('NodeSocketFloat', 'Strength')
self.inputs[-1].default_value = 2.0

# or
self.inputs.new('NodeSocketFloat', 'Strength').default_value = 2.0

with a parameter. Also, the new is_var parameter would add a dot to the sockets so that they are recognized as variables:

VariableSocket

What do you think?

knowledgenude commented 4 years ago

There is another way to do this (better and i will say why): this variation of the output is not like "hey i am a variable" because of its size (zoom is something you don't wanna do in nodes).

Another way to do this is custom colors for each node type. This can be done manually by the user, by enabling Color checkbox. For example, lets take blueprints as example:

images.jpeg

As you can see, each blueprint has some color depending of its type and Blender nodes looks more readable in my opinion. I really would like to see the result on this :)

knowledgenude commented 4 years ago

But your idea can be applied in the unused sockets (leaving the middle of unused sockets as they are "empties"):

images (1)~2.jpeg

MoritzBrueckner commented 4 years ago

Another way to do this is custom colors for each node type. This can be done manually by the user, by enabling Color checkbox. For example, lets take blueprints as example:

I'm not sure if this is the best approach, this would require setting a defined color which could conflict with the user's theme (setting the header color isn't possible, Blender only allows to color the entire node different). To counteract against that, we would need to add a color setting in the preferences.

But the bigger problem is that the variable system internally works with sockets. The question then is, how should we color nodes that work with variables but aren't variables themselves? How do we tell the user that one input uses a variable and one doesn't? Take the Set Variable node for example: the Variable parameter takes in a variable (ok, in this case it's obvious due to the socket name) and the Value parameter doesn't, yet they have the same socket.

But your idea can be applied in the unused sockets (leaving the middle of unused sockets as they are "empties"):

It's a great idea but this cannot really be achieved (we would need to listen for each node connection change in the code) and even then it would probably be confusing for beginners as Blender doesn't have such a system.

Instead of a dot in the center we could use a diamond shape or a quadrate (if that's what you mean with zooming, the dot is too small?). Would one of those be better? Other options don't exist in Blender, it isn't possible to define other shapes for sockets.

If other people read this, it would be nice to hear your opinion too (on this and on the other things discussed here). Only 2-3 people might come to results that don't reflect a broader preference.

knowledgenude commented 4 years ago

@MoritzBrueckner , indeed. I think in the node output filled or not is more a feature to Blender Foundation implement themselves. Maybe sometime they care more about nodes design.

If there is some parameter we can put inside the .py file about what color the node is created, i can do it manually for each one.

About the Value socket color, I don't think changing its color to another would keep the "pattern" (as it is the only one with this color). Except if it uses the same color as property in Set Property. I also get confuse with both Value and Variable is green.

If you are not safe that everyone will like it, you can show for them in the forum. I personally don't think someone will not like it. This will make everything better and more user friendly.

MoritzBrueckner commented 4 years ago

If there is some parameter we can put inside the .py file about what color the node is created, i can do it manually for each one.

Nodes have a parameter like this but the problems mentioned above still exist. And you're right, changing the socket color is a bad idea because then the information about its type is lost. That's why I was talking about its shape - you can change it to a rhomb/diamond or quadrate like in this image. Or is the dot sufficient enough?

Except if it uses the same color as property in Set Property. I also get confuse with both Value and Variable is green.

Grey like in Set Property means that it is a string, the node expects a property name (for a dictionary-like access). Light green (actually it is the Shader socket) means that the type is Dynamic which we want for the Set Variable node, as it should work with different data types. So the question is how we mark the variable sockets without loosing the type information (color).

knowledgenude commented 4 years ago

@MoritzBrueckner , in this case the only solution is to mark with another color inside as you send above (but there is a way to make it bigger?). I also liked changing its shape, but more for design than funcionality (as color is more easy to recognize than shapes).

MoritzBrueckner commented 4 years ago

but there is a way to make it bigger?

I fear not, this would be awesome though. A dot in the center it is, then.

knowledgenude commented 4 years ago

@MoritzBrueckner , there is some changes to make the nodes more polished (changes that must happens in the .hx and .py files to keep everything clean):

There is Collision Groups and Collision Masks. Ray Cast searches for Collision Groups, then:

Cast Physics Ray - > Ray Cast Collision Filter Mask -> Collision Groups - I also think this must start with 1 instead of 0, because there is not a group 0.

Mask -> Collision Groups - The only usage of this node is in Ray Cast groups -> Groups

Get Visible -> Get Object Visible

Set Visible -> Set Object Visible

Self - > Self Object (as we have Self Trait, Self Scene, etc.)

Parse float - > Parse Float

Load Url (Browser only) - > Load URL (Browser Only) or Load URL?

In Array - > Array Contains

Scene Root - > Scene Objects? Note that if there is an object in the scene called Root, any logic over this object will affect the entire scene.

Eveything that contains Trait - > Tree? Because trait is referencing the Node Tree.

Nodes to be removed: On Mouse, On Keyboard, On Surface, On Gamepad, On Virtual Button.

Nodes to be merged: Show Mouse and Set Mouse Lock. My opinion is that nobody wants the mouse to be displayed if it is locked, and nobody wants the mouse to be not displayed if it is not locked.

These are nodes that i can't see any usage for them: Expression, Play/Pause/Stop speaker.

There have approximately 18 tasks to be done related to nodes. From new nodes to implement new functionality to existent nodes. I will group all of them and create a new issue to track these and close the old issues. I will focus on this, because i really want to learn something from this.

The only functionality that will lack on Armory after all of this is related to rotations and control over parents.

Did i forgot something?

MoritzBrueckner commented 4 years ago

Self, Parse float, In Array, Cast Physics Ray and Collision Filter Mask are renamed now, thanks again :)

I wouldn't rename Mask as it just returns an integer that might be used elsewhere (and also might have its use case in the future). Because nodes are now sorted in other categories and show an icon based on their category, the same goes for Get/Set Visible, I think it's sufficient enough, otherwise we would have to rename the getter/setter nodes for meshes, parents etc. too.

In my opinion Scene Root is a good name as this is what it's called. Scene Objects would suggest that the returned value is an array.

Also, traits don't need to be node trees, they can be scripts and canvas traits too.

About the merging of the mouse nodes: I have no opinion on that other than there might be cases where only one of those nodes is used.

The removal of deprecated nodes should wait until we have a proper node replacement system in place, @niacdoial seems to work on one. In the meantime, we could mark them as @:deprecated in the Haxe code so that they raise warnings on export.

The Expression node can have is use cases (even if rarely used), for example if you have more complicated conditions/equations and you don't want to use a bunch of nodes for that. Regarding the Speaker node, I don't know about how complete the support for speakers is in Armory but we should keep it.

@ everyone:

If you want to test the current changes, check out this branch: https://github.com/MoritzBrueckner/armory/tree/logic-nodes. It would be great to get some feedback especially about how the sections are divided.

Thanks!

knowledgenude commented 4 years ago

Show Mouse is not better as Set Mouse Visible?

MoritzBrueckner commented 4 years ago

I have no opinion on that, if you want you can open a PR when this issue is resolved :)

I plan to open the PR for this issue later today, the work is finally complete (also, nodes have tooltips now!). Only the wiki part is still missing. Do you think a separate page per category is useful?

knowledgenude commented 4 years ago

@MoritzBrueckner , okay. I will open a new pr later with some renames. What kind of page you are talking? There is some concept?

Because we are talking about nodes organization, I don't know what is better for Armory in this case: a node with all rigid body atributes (suck as mass, friction, mask, etc) or a single node for each atribute?

MoritzBrueckner commented 4 years ago

What kind of page you are talking?

The logic node reference in the wiki, as mentioned in the first post above. Currently, everything is one big messy page. This has the advantage that you can use Ctrl + F to easily search for nodes but its a pain to scroll through that page. My proposal would be to add a table of content that links to sub-pages for each category. On the "index"-page there would then be a small introduction to logic nodes, sockets, variables etc.

I don't know what is better for Armory in this case: a node with all rigid body atributes (suck as mass, friction, mask, etc) or a single node for each atribute?

In this case, I would prefer a node for all of those settings like Set RigidBody Properties (it should be easy to understand so a big node is okay in my opinion), but there is one problem: in the most cases you want to only set one parameter. If the node has a bunch of sockets, there must be a way to enable those or otherwise you'll have to know the current values to "re-set" them in order to not change them.

If you want to add such a node, I can try to implement a socket setting that adds a checkbox next to an input socket. I'm not sure if it works but it should be doable.

knowledgenude commented 4 years ago

@MoritzBrueckner , first i need to understand what is happening that Bullet Physics don't listen to the properties i set.

This is not exactly related to the purpose of this topic, but why there is this line is in every node: if (object == null) return; is there some case that the object is null?

knowledgenude commented 4 years ago

About the page layout, is not more handy if there is a page per node category? Because clicking to open each node page can be a pain.

MoritzBrueckner commented 4 years ago

first i need to understand what is happening that Bullet Physics don't listen to the properties i set.

I'll take a look but I cannot guarantee that I know why this doesn't work, I'm a big physics noob^^

The null-check is done because the object might be null if the user doesn't enter an object for example. Then there is no value so null is used.

Because clicking to open each node page can be a pain.

Indeed, that's why I'm really unsure about that... But the sheer amount of nodes and screenshots slows down the page. If there were some kind of spoiler/expandable sections that would be awesome.

knowledgenude commented 4 years ago

@MoritzBrueckner , in my case i would not go to the wiki to "study" what each node do, but to clarify any doubt i have over a specific node. If this looks common for you, maybe there is no problem to have each node per page.

MoritzBrueckner commented 4 years ago

Should the logic nodes in the wiki be sorted alphabetically in their category or also respect the section layout?

knowledgenude commented 4 years ago

@MoritzBrueckner , in my opinion alphabetical order is more organized and easy to search. There is some specific cases not, like two nodes that Get and Set the same value. If there is a way to sort in alphabetic order by the second word for these cases... ^^

MoritzBrueckner commented 4 years ago

@luboslenco I'm currently working on a script that automatically generates the logic node reference page based on docstrings (with a syntax similar to javadoc), where would you put that script (if that's ok for you)? It should be editable so the wiki repository is not a good place, the wiki image repo might be a bit different topic-wise and I don't know if it fits somewhere in the Armory repo.

So instead of editing the wiki, users can edit the docstrings and the wiki would be updated accordingly (if we would combine this with a Github action in the future, that should be possible. Currently, one has to copy the changes to the wiki by hand). There is also a script in the wiki image repo that takes/updates all node screenshots but it unfortunately has to be executed manually now and then (link).

luboslenco commented 4 years ago

👍 Maybe the https://github.com/armory3d/armory_tools repo would do?