Skip to content

Commit

Permalink
GH-630 Enhance VariableGet with validation mode
Browse files Browse the repository at this point in the history
  • Loading branch information
Naros committed Aug 1, 2024
1 parent 26d8395 commit 214c184
Show file tree
Hide file tree
Showing 9 changed files with 174 additions and 7 deletions.
2 changes: 2 additions & 0 deletions doc_classes/OScriptNodeVariableGet.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<description>
The [OScriptNodeVariableGet] node is used to return the value of an orchestration variable.
The output pin returns the value currently assigned in the orchestration variable. If the variable has not been assigned, it will return the variable type's default value, which may be [code]null[/code].
[b]Return Value Validation:[/b][br]Variables that derive from [Object] will return a [code]null[/code] value if they have not been assigned. The [OScriptNodeVariableGet] node can provide validation for [code]null[/code] automatically by right-clicking the node's titlebar and select [param Make Validated]. In validated mode, the node will have two execution output pins and the node exits one of those based on the variable's current state. If the variable is not [code]null[/code], the node exits the [param Is Valid] output pin; otherwise the node exits the [param Is Not Valid] output pin. This validation mode reduces the need to add extra validation branch conditions to the graph.
To convert the node back to its pure mode, select [param Make Pure] in the node's context-menu.
</description>
<tutorials>
<link title="Node reference">https://docs.cratercrash.com/orchestrator/nodes/variables#get-nodes</link>
Expand Down
8 changes: 8 additions & 0 deletions src/editor/graph/graph_edit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -679,16 +679,23 @@ void OrchestratorGraphEdit::_drop_data(const Vector2& p_position, const Variant&
}
else
{
const bool validated = get_orchestration()->get_variable(variable_name)->get_variable_type() == Variant::OBJECT;

// Create context-menu handlers
Ref<OrchestratorGraphActionHandler> get_handler(memnew(OrchestratorGraphNodeSpawnerVariableGet(variable_name)));
Ref<OrchestratorGraphActionHandler> set_handler(memnew(OrchestratorGraphNodeSpawnerVariableSet(variable_name)));
Ref<OrchestratorGraphActionHandler> get_validated_handler(memnew(OrchestratorGraphNodeSpawnerVariableGet(variable_name, true)));

// Create context-menu to specify variable get or set choice
_context_menu->clear();
_context_menu->add_separator("Variable " + variable_name);
_context_menu->add_item("Get " + variable_name, CM_VARIABLE_GET);
if (validated)
_context_menu->add_item("Get " + variable_name + " with validation", CM_VARIABLE_GET_VALIDATED);
_context_menu->add_item("Set " + variable_name, CM_VARIABLE_SET);
_context_menu->set_item_metadata(_context_menu->get_item_index(CM_VARIABLE_GET), get_handler);
if (validated)
_context_menu->set_item_metadata(_context_menu->get_item_index(CM_VARIABLE_GET_VALIDATED), get_validated_handler);
_context_menu->set_item_metadata(_context_menu->get_item_index(CM_VARIABLE_SET), set_handler);
_context_menu->reset_size();
_context_menu->set_position(get_screen_position() + p_position);
Expand Down Expand Up @@ -1642,6 +1649,7 @@ void OrchestratorGraphEdit::_on_context_menu_selection(int p_id)
switch (p_id)
{
case CM_VARIABLE_GET:
case CM_VARIABLE_GET_VALIDATED:
case CM_VARIABLE_SET:
case CM_PROPERTY_GET:
case CM_PROPERTY_SET:
Expand Down
1 change: 1 addition & 0 deletions src/editor/graph/graph_edit.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class OrchestratorGraphEdit : public GraphEdit
enum ContextMenuIds
{
CM_VARIABLE_GET,
CM_VARIABLE_GET_VALIDATED,
CM_VARIABLE_SET,
CM_PROPERTY_GET,
CM_PROPERTY_SET,
Expand Down
23 changes: 23 additions & 0 deletions src/editor/graph/graph_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "script/nodes/editable_pin_node.h"
#include "script/nodes/functions/call_function.h"
#include "script/nodes/functions/call_script_function.h"
#include "script/nodes/variables/variable_get.h"
#include "script/script.h"

#include <godot_cpp/classes/button.hpp>
Expand Down Expand Up @@ -495,6 +496,16 @@ void OrchestratorGraphNode::_show_context_menu(const Vector2& p_position)
if (multi_selections)
_context_menu->set_item_tooltip(_context_menu->get_item_index(CM_VIEW_DOCUMENTATION), "Select a single node to view documentation.");

Ref<OScriptNodeVariableGet> variable_get = _node;
if (variable_get.is_valid() && variable_get->can_be_validated())
{
_context_menu->add_separator("Variable Get");
if (variable_get->is_validated())
_context_menu->add_item("Make Pure", CM_MAKE_PURE_GETTER);
else
_context_menu->add_item("Make Validated", CM_MAKE_VALIDATED_GETTER);
}

_context_menu->set_position(get_screen_position() + (p_position * (real_t) get_graph()->get_zoom()));
_context_menu->reset_size();
_context_menu->popup();
Expand Down Expand Up @@ -749,6 +760,18 @@ void OrchestratorGraphNode::_handle_context_menu(int p_id)
get_graph()->emit_signal("expand_node", _node->get_id());
break;
}
case CM_MAKE_PURE_GETTER:
{
Ref<OScriptNodeVariableGet> getter = _node;
getter->set_validated(false);
break;
}
case CM_MAKE_VALIDATED_GETTER:
{
Ref<OScriptNodeVariableGet> getter = _node;
getter->set_validated(true);
break;
}
#if GODOT_VERSION >= 0x040300
case CM_TOGGLE_BREAKPOINT:
{
Expand Down
2 changes: 2 additions & 0 deletions src/editor/graph/graph_node.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ class OrchestratorGraphNode : public GraphNode
CM_COLLAPSE_FUNCTION,
CM_EXPAND_NODE,
CM_RESIZABLE,
CM_MAKE_PURE_GETTER,
CM_MAKE_VALIDATED_GETTER,
CM_NODE_ACTION = 1000
};

Expand Down
4 changes: 4 additions & 0 deletions src/editor/graph/graph_node_spawner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,10 @@ void OrchestratorGraphNodeSpawnerVariableGet::execute(OrchestratorGraphEdit* p_g
OScriptNodeInitContext context;
context.variable_name = _variable_name;

Dictionary data;
data["validation"] = _validation;
context.user_data = data;

p_graph->spawn_node<OScriptNodeVariableGet>(context, p_position);
}

Expand Down
5 changes: 4 additions & 1 deletion src/editor/graph/graph_node_spawner.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,14 @@ class OrchestratorGraphNodeSpawnerVariableGet : public OrchestratorGraphNodeSpaw
static void _bind_methods() { }

protected:
bool _validation{ false };

OrchestratorGraphNodeSpawnerVariableGet() = default;

public:
OrchestratorGraphNodeSpawnerVariableGet(const StringName& p_variable_name)
OrchestratorGraphNodeSpawnerVariableGet(const StringName& p_variable_name, bool p_validation = false)
: OrchestratorGraphNodeSpawnerVariable(p_variable_name)
, _validation(p_validation)
{
}

Expand Down
115 changes: 109 additions & 6 deletions src/script/nodes/variables/variable_get.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,60 @@ class OScriptNodeVariableGetInstance : public OScriptNodeInstance
{
DECLARE_SCRIPT_NODE_INSTANCE(OScriptNodeVariableGet);
StringName _variable_name;
bool _validated{ false };

public:
int step(OScriptExecutionContext& p_context) override
{
Variant value;
if (!p_context.get_runtime()->get_variable(_variable_name, value))
OScriptVirtualMachine::Variable* variable = p_context.get_runtime()->get_variable(_variable_name);
if (!variable)
{
p_context.set_error(vformat("Variable '%s' not found.", _variable_name));
return -1;
}

if (!p_context.set_output(0, &value))
p_context.set_output(0, &variable->value);
if (_validated)
{
p_context.set_error("Failed to set variable value on output stack.");
return -1;
}

if (variable->type == Variant::OBJECT && !Object::cast_to<Object>(variable->value))
return 1;
}
return 0;
}
};

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

void OScriptNodeVariableGet::_get_property_list(List<PropertyInfo>* r_list) const
{
r_list->push_back(PropertyInfo(Variant::BOOL, "validated", PROPERTY_HINT_NONE, "", PROPERTY_USAGE_STORAGE));
}

bool OScriptNodeVariableGet::_get(const StringName& p_name, Variant& r_value) const
{
if (p_name.match("validated"))
{
r_value = _validated;
return true;
}

// todo: GodotCPP expects this to be done by the developer, Wrapped::get_bind doesn't do this
return OScriptNodeVariable::_get(p_name, r_value);
}

bool OScriptNodeVariableGet::_set(const StringName& p_name, const Variant& p_value)
{
if (p_name.match("validated"))
{
_validated = p_value;
return true;
}

// todo: GodotCPP expects this to be done by the developer, Wrapped::set_bind doesn't do this
return OScriptNodeVariable::_set(p_name, p_value);
}

void OScriptNodeVariableGet::_upgrade(uint32_t p_version, uint32_t p_current_version)
{
if (p_version == 1 and p_current_version >= 2)
Expand Down Expand Up @@ -80,7 +111,15 @@ void OScriptNodeVariableGet::_variable_changed()

void OScriptNodeVariableGet::allocate_default_pins()
{
if (_validated)
{
create_pin(PD_Input, PT_Execution, PropertyUtils::make_exec("ExecIn"));
create_pin(PD_Output, PT_Execution, PropertyUtils::make_exec("is_valid"))->set_label("Is Valid");
create_pin(PD_Output, PT_Execution, PropertyUtils::make_exec("is_invalid"))->set_label("Is Invalid");
}

create_pin(PD_Output, PT_Data, PropertyUtils::as("value", _variable->get_info()))->set_label(_variable_name, false);

super::allocate_default_pins();
}

Expand All @@ -102,5 +141,69 @@ OScriptNodeInstance* OScriptNodeVariableGet::instantiate()
OScriptNodeVariableGetInstance *i = memnew(OScriptNodeVariableGetInstance);
i->_node = this;
i->_variable_name = _variable->get_variable_name();
i->_validated = _validated;
return i;
}

void OScriptNodeVariableGet::initialize(const OScriptNodeInitContext& p_context)
{
if (p_context.user_data)
{
const Dictionary& data = p_context.user_data.value();
if (data.has("validation"))
_validated = data["validation"];
}

super::initialize(p_context);
}

bool OScriptNodeVariableGet::can_be_validated()
{
PropertyInfo property = _variable->get_info();
if (property.type == Variant::OBJECT)
return true;
return false;
}

void OScriptNodeVariableGet::set_validated(bool p_validated)
{
if (_validated != p_validated)
{
_validated = p_validated;

if (!_validated)
{
// Disconnect any control flow pins, if they exist
Ref<OScriptNodePin> exec_in = find_pin("ExecIn", PD_Input);
if (exec_in.is_valid())
exec_in->unlink_all();

Ref<OScriptNodePin> is_valid = find_pin("is_valid", PD_Output);
if (is_valid.is_valid())
is_valid->unlink_all();

Ref<OScriptNodePin> is_not_valid = find_pin("is_not_valid", PD_Output);
if (is_not_valid.is_valid())
is_not_valid->unlink_all();
}

// Record the connection before the change
Ref<OScriptNodePin> connection;
Ref<OScriptNodePin> value = find_pin("value", PD_Output);
if (value.is_valid() && value->has_any_connections())
{
connection = value->get_connections()[0];
value->unlink_all();
}

_notify_pins_changed();

if (connection.is_valid())
{
// Relink connection on change
value = find_pin("value", PD_Output);
if (value.is_valid())
value->link(connection);
}
}
}
21 changes: 21 additions & 0 deletions src/script/nodes/variables/variable_get.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ class OScriptNodeVariableGet : public OScriptNodeVariable
static void _bind_methods() { }

protected:
bool _validated{ false }; //! Whether to represent get as validated get

// //~ Begin Wrapped Interface
void _get_property_list(List<PropertyInfo>* r_list) const;
bool _get(const StringName& p_name, Variant& r_value) const;
bool _set(const StringName& p_name, const Variant& p_value);
// //~ End Wrapped Interface

//~ Begin OScriptNode Interface
void _upgrade(uint32_t p_version, uint32_t p_current_version) override;
//~ End OScriptNode Interface
Expand All @@ -41,7 +49,20 @@ class OScriptNodeVariableGet : public OScriptNodeVariable
String get_node_title() const override;
bool should_draw_as_bead() const override { return true; }
OScriptNodeInstance* instantiate() override;
void initialize(const OScriptNodeInitContext& p_context) override;
//~ End OScriptNode Interface

/// Return whether the node can be validated
/// @return true if the node can be validated, false otherwise
bool can_be_validated();

/// Return whether the variable is validated
/// @return true if the node is rendered as a validated node, false otherwise.
bool is_validated() const { return _validated; }

/// Change whether the node is rendered as a validated get
/// @param p_validated when true, rendered as validated get
void set_validated(bool p_validated);
};

#endif // ORCHESTRATOR_SCRIPT_NODE_VARIABLE_GET_H

0 comments on commit 214c184

Please sign in to comment.