Skip to content

Add support for binding callback function pipelines#50

Open
MelanTech wants to merge 1 commit intoHotariTobu:mainfrom
MelanTech:main
Open

Add support for binding callback function pipelines#50
MelanTech wants to merge 1 commit intoHotariTobu:mainfrom
MelanTech:main

Conversation

@MelanTech
Copy link
Copy Markdown

Here's a prototype implementation of callback function binding, which allows for functions to be triggered after data updates. Users can bind multiple custom callback functions using the with statement. This functionality is particularly useful for data storage, as users can save data via callback functions once it has been updated. Below is an example of its usage:

var _data = BindingSource.new(Data.new())
var _config_file = ConfigFile.new()

func _ready():
	_data.bind(&"count").using(_get_label).with(_on_data_changed).with(_on_data_changed_print)

func _on_button_pressed():
	_data.count += 1

func _on_data_changed(name, value):
	_config_file.set_value("section_name", name, value)
	_config_file.save("user://settings.cfg")

func _on_data_changed_print(name, value):
	prints(name, value)

class Data:
	var count: int = 0

Copy link
Copy Markdown
Owner

@HotariTobu HotariTobu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD]
Change with_pipeline to callback_pipeline.

[SHOULD]
From the perspective of a with user, it's not clear from the code whether a callback function registered with with is called when the source changes or when the target changes.
Make it possible to specify whether the callback function is called on source changes or target changes when it is registered.
For example:

func with(callback: Callback, on_source_changed: bool = false):

[MUST]
Write tests.

[MUST]
Change the branch name to feat/MelanTech-callback-pipeline.
You'll need to recreate the PR to change the merge source branch name. Please finish your revisions and then create a new pull request.

converter_pipeline: BindingConverterPipeline,
target_value_change_signal
target_value_change_signal,
with_pipeline: BindingWithPipeline = null,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD]
Do not pass null as a default value, just as you would with other arguments.

Suggested change
with_pipeline: BindingWithPipeline = null,
with_pipeline: BindingWithPipeline,


for with_func in _with_pipeline.get_pipeline():
assert(with_func.is_valid(), "The with function must be a valid Callable.")
with_func.call(_source_property, next_source_value)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[SHOULD]
Since source_property is known when the callback function is registered, passing it during the callback is redundant.
Instead, pass the value before the change.

Suggested change
with_func.call(_source_property, next_source_value)
with_func.call(source_value, next_source_value)

@HotariTobu
Copy link
Copy Markdown
Owner

Thank you for contributing to this add-on.
I have a few corrections I'd like you to make.

@HotariTobu HotariTobu added the enhancement New feature or request label Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants