Skip to content

Commit

Permalink
fix(VariableNode): causing infinite loops, Node::eval() deleted, is n…
Browse files Browse the repository at this point in the history
…ow in VM
  • Loading branch information
berdal84 committed Apr 16, 2022
1 parent 3cb5230 commit b0ef607
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 87 deletions.
1 change: 0 additions & 1 deletion src/core/include/nodable/core/Node.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ namespace Nodable {
bool is_dirty()const;

virtual UpdateResult update();
virtual bool eval() const;

const IInvokable* get_connected_operator(const Member* _localMember); // TODO: weird, try to understand why I needed this
bool has_wire_connected_to(const Member *_localMember);
Expand Down
1 change: 0 additions & 1 deletion src/core/include/nodable/core/VariableNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ namespace Nodable
inline bool is_declared()const { return m_is_declared; }
const char* get_name()const { return m_name.c_str(); };
Member* get_value()const { return m_value; }
bool eval()const override;
std::shared_ptr<const Token> get_type_token() const { return m_type_token; }
std::shared_ptr<const Token> get_assignment_operator_token() const { return m_assignment_operator_token; }
std::shared_ptr<const Token> get_identifier_token() const { return m_identifier_token; }
Expand Down
3 changes: 1 addition & 2 deletions src/core/src/GraphNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,9 @@ UpdateResult GraphNode::update()
{
if (each_node->is_dirty())
{
each_node->eval();
each_node->update();
each_node->set_dirty(false);
changed |= true;
changed = true;
}
}
if ( changed )
Expand Down
24 changes: 0 additions & 24 deletions src/core/src/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,30 +135,6 @@ int Node::get_output_wire_count()const
return count;
}

bool Node::eval() const
{
// copy values (only if connection is "by copy")
for(Member* each_member : m_props.by_id())
{
Member* input = each_member->get_input();

if( input
&& !each_member->is_connected_by_ref()
&& each_member->get_type() != type::null
&& input->get_type() != type::null )
{
*each_member->get_variant() = *input->get_variant();
}
}

// update
if(has<InvokableComponent>())
{
return get<InvokableComponent>()->update();
}
return true;
}

UpdateResult Node::update()
{
if(has<DataAccess>())
Expand Down
11 changes: 0 additions & 11 deletions src/core/src/VariableNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,3 @@ void VariableNode::set_name(const char* _name)

set_label(label.c_str(), short_label);
}

bool VariableNode::eval() const
{
Variant* variant = m_value->get_variant();
if(variant->is_initialized() && !variant->is_defined() )
{
variant->flag_defined();
return Node::eval();
}
return true;
}
60 changes: 47 additions & 13 deletions src/core/src/VirtualMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <nodable/core/VariableNode.h>
#include <nodable/core/Log.h>
#include <nodable/core/Scope.h>
#include <nodable/core/InvokableComponent.h>
#include "nodable/core/String.h"

using namespace Nodable;
Expand Down Expand Up @@ -174,11 +175,7 @@ bool VirtualMachine::_stepOver()
{
advance_cursor();
auto* variable = const_cast<VariableNode*>( next_instr->push.var ); // hack !
NODABLE_ASSERT_EX(!variable->get_value()->get_variant()->is_initialized(),
"We should not push an initialized variable, check if push_stack_frame did or did not.");
variable->get_value()->get_variant()->ensure_is_initialized(true);
// TODO: push variable to the future stack

variable->get_value()->get_variant()->ensure_is_initialized(false);
success = true;
break;
}
Expand All @@ -189,29 +186,66 @@ bool VirtualMachine::_stepOver()
auto* variable = const_cast<VariableNode*>( next_instr->push.var ); // hack !
NODABLE_ASSERT_EX(variable->get_value()->get_variant()->is_initialized(),
"Variable should be initialized since it should have been pushed earlier!");
variable->get_value()->get_variant()->reset_value();
variable->get_value()->get_variant()->ensure_is_initialized(false);
// TODO: pop variable to the future stack
success = true;
break;
}

case opcode::push_stack_frame: // ensure variable declared in this scope are unitialized before/after scope
case opcode::pop_stack_frame:
{
auto scope = next_instr->pop.scope;
for(auto each_variable : scope->get_variables())
{
each_variable->get_value()->get_variant()->ensure_is_initialized(false);
}
advance_cursor();
success = true;
break;
}

case opcode::eval_node:
{
auto node = const_cast<Node*>( next_instr->eval.node ); // hack !
node->eval();
bool transfer_inputs = true;
auto node = const_cast<Node*>( next_instr->eval.node ); // hack !

auto transfer_input_values = [](Node* _node)
{
for(Member* each_member : _node->props()->by_id())
{
Member* input = each_member->get_input();

if( input
&& !each_member->is_connected_by_ref()
&& each_member->get_type() != type::null
&& input->get_type() != type::null )
{
*each_member->get_variant() = *input->get_variant();
}
}
};

if( auto variable = node->as<VariableNode>())
{
Variant* variant = variable->get_value()->get_variant();
if( !variant->is_initialized() )
{
variant->ensure_is_initialized();
variant->flag_defined();
}
else
{
transfer_inputs = false;
}
}

if( transfer_inputs )
{
transfer_input_values(node);
}

// evaluate Invokable Component, could be an operator or a function
if(auto invokable = node->get<InvokableComponent>())
{
invokable->update();
}

node->set_dirty(false);
advance_cursor();
success = true;
Expand Down
35 changes: 0 additions & 35 deletions tests/src/specs/graph.specs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,38 +142,3 @@ TEST_F( graph_node_fixture, create_and_delete_relations)
EXPECT_EQ(n2->inputs().size(), 0);
EXPECT_EQ(graph.get_relation_registry().size(), 0);
}

TEST_F( graph_node_fixture, by_reference_assign)
{
// we will create this graph manually
// "double b = 6;"
// "b = 5;"
// "b;";

// prepare
Node* program = graph.create_root();

// create b
VariableNode* var_b = graph.create_variable<double>("b", program->get<Scope>());
var_b->set(6.0);

// create assign operator
Signature* sig = Signature
::from_type<int(double&, double)>
::as_operator(language.find_operator("=", Operator_t::Binary));

Node* assign = graph.create_function(language.find_operator_fct(sig));

// connect b
auto props = assign->props();
graph.connect(var_b->get_value(), props->get_input_at(0) );

props->get_input_at(1)->set(5.0);

ASSERT_DOUBLE_EQ((double)*var_b->get_value(), 6.0 );

// apply
assign->eval();

ASSERT_DOUBLE_EQ((double)*var_b->get_value(), 5.0 );
}

0 comments on commit b0ef607

Please sign in to comment.