diego1996 / gamekit

Automatically exported from code.google.com/p/gamekit
0 stars 0 forks source link

gkIfNode with CMP_OR statement is blocked incorrectly. #38

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
It does not make sense to block an OR-IF node if one of the input socket is 
blocked. I would like to have it blocked if both inputs are blocked...

To solve it, we can make the gkLogicNode::block method as virtual and 
redefine  the blocking logic in gkIfNode::block. Something like this could 
solve the problem:

void gkIfNode::block(bool truth)
{
    if(mStatement == CMP_OR && truth)
    {
        truth = mSockets[0].isBlocked() && mSockets[1].isBlocked();
    }

    gkLogicNode::block(truth);
}

Original issue reported on code.google.com by silveira.nestor@gmail.com on 15 Feb 2010 at 9:11

GoogleCodeExporter commented 9 years ago
Question, do you find the node system useful even without a supported GUI ? To 
me
it's a very tedious process, to manually build nodes. As much as I find it 
useful
there is no blender support anymore. Therefor I think it should be put on hold 
and
reimplemented later on with the nodal system being developed. In the mean time
continue adding logic brick support.

I'm asking this, because if it's useful to you or anyone else. Support can be
continued and bugs including this one should be fixed.

Original comment by snailr...@gmail.com on 15 Feb 2010 at 3:52

GoogleCodeExporter commented 9 years ago
Yes I agree with you. It is tedious and it will not 100% useful till we have a 
GUI.
But it is a very powerful mechanism to add logic by code. And it is very easy 
to add 
new nodes in order to have new functionality. In my case I added some extra 
functionality like Arcball-control for the camera in a very short time ( I 
guess it 
will be difficult to do the same using just logic-bricks!)

Original comment by silveira.nestor@gmail.com on 15 Feb 2010 at 4:13

GoogleCodeExporter commented 9 years ago
Yes It's very difficult to do more complex things with logic bricks, simply 
because
were dealing with Boolean values only. Being that you have made used of it, I 
will
update support in the newer code base and we can work out some of these bugs.

Original comment by snailr...@gmail.com on 15 Feb 2010 at 4:25

GoogleCodeExporter commented 9 years ago
Thanks! (By the way the code you are doing is awesome! Very nice job!)

Original comment by silveira.nestor@gmail.com on 15 Feb 2010 at 4:41

GoogleCodeExporter commented 9 years ago
Thanks, this node system has issues though! 
First thing is nodes need to be sorted for proper execution. If manually 
created,
execution depends on creation order. When originally written I think I took a 
short
cut and let blender sort them for me. 

Were you able to even get the if node to behave properly ? So far by my account 
It's
completely broken.

Original comment by snailr...@gmail.com on 15 Feb 2010 at 6:44

GoogleCodeExporter commented 9 years ago
I had the same problem with several nodes: "if node" and "motion node" 
included. I 
realized the problem was when you create the subtree for io blocking (contained 
in 
gkLogicSocket::mNodes member variable). Basically the problem is inside 
gkLogicTree::findSubNodeForward method. gkLogicTree::findSubNodeForward is a 
recursive method called from gkLogicTree::pushSocketTree with the skip 
parameter as 
true, but in the first recursive call you changed the skip parameter to 
false... this 
will cause adding to the blocking list (mNodes) also the nodes linked to the 
input 
sockets...
At the end what you have in gkLogicSocket::mNodes are all the nodes related to 
the 
output socket and for them (first recursive call) also all the nodes related to 
input 
and output socket and so on.(second , third recursive call,...)
Conclusion: when a node blocks itself, then, blocks also all the nodes linked 
to the 
output sockets (so far so good, first level of the tree) and (the problem) all 
the 
nodes linked to input and output sockets for the following levels of the tree.

For example to have "test13" working I had to change 
gkLogicTree::findSubNodeForward 
to not include the input related nodes:

 Changing:
          findSubNodeForward(root, rfnode, to, 0, false);
    to
          findSubNodeForward(root, rfnode, to, 0, skip);

After this I realized that some nodes like OR-IF cannot be blocked if some of 
the 
inputs are not blocked...(same for motion).

(As you say the blocking mechanism needs some fixing and issue 37 and 38 are 
very 
related...)

I hope this will help!

Original comment by silveira.nestor@gmail.com on 16 Feb 2010 at 9:14

GoogleCodeExporter commented 9 years ago
I'm wondering if blocking should be removed all together. the findSubNodeForward
function is a complete nightmare and hard to understand. Right now, the local 
update
I have works great without any blocking whatsoever. If I remember correctly, 
blocking
was an attempt to speed up execution time and nothing more.

Locally, now that I have the nodes sorted. My thoughts are to execute the first 
node
depth only. If any sockets need to pass data through a link, push its outputs 
on to a
stack and recursively do the same until there are no remaining nodes. But keep
execution depth / level based. So for all nodes that have a depth of 2
do the normal evaluation, then push depth 1 nodes, and so on.

Original comment by snailr...@gmail.com on 16 Feb 2010 at 2:26

GoogleCodeExporter commented 9 years ago
I was wondering the same too. All this blocking stuff has also a performance 
cost...! 
If the evaluate function is fast enough I do not think we will have a big boost 
in 
performance keeping all this blocking stuff...

Original comment by silveira.nestor@gmail.com on 16 Feb 2010 at 2:39

GoogleCodeExporter commented 9 years ago
Thanks for confirming, I was going to hold off on removing it in case you had 
some
code dependency on it. But will, remove it along with the next update.

Original comment by snailr...@gmail.com on 16 Feb 2010 at 2:52

GoogleCodeExporter commented 9 years ago
any more issues with if nodes ? 

Original comment by snailr...@gmail.com on 17 Feb 2010 at 5:49