Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callback API #3224

Open
monokotech opened this issue May 12, 2024 · 7 comments
Open

Callback API #3224

monokotech opened this issue May 12, 2024 · 7 comments
Milestone

Comments

@monokotech
Copy link

This is what I learned from the conversation with aleck099.

By adding this "registration" method, we can achieve more customization, which can reduce redundant code and minimize the impact on the EasyRPG Player.

This part can refer to the usage of RegisterHandler in game_multiplayer.cpp
Example: easyrpg.RegisterHandler(what, ...);

The idea is only mentioned at the moment.

@Ghabry
Copy link
Member

Ghabry commented May 12, 2024

Do you mean you want callbacks at certain parts of the code to be notified about changes?

@monokotech
Copy link
Author

Correct

@Ghabry
Copy link
Member

Ghabry commented May 12, 2024

Well if this makes it easier for you to keep this huge patch up-to-date: Sure. You only have to list the locations where you need them.

@monokotech
Copy link
Author

Well if this makes it easier for you to keep this huge patch up-to-date: Sure. You only have to list the locations where you need them.

I will try this before the next merge.

@Ghabry Ghabry changed the title Something like RegisterHandler Callback API May 18, 2024
@Ghabry
Copy link
Member

Ghabry commented Jun 10, 2024

@monokotech wrote:

Could the request_id be directly stored in the listeners? This way, there is no need to add the FileRequestBinding in the class.

std::vector<std::pair<FileRequestBindingWeak, std::function<void(FileRequestResult*)> > > listeners;

to:

std::vector<std::pair<int, std::function<void(FileRequestResult*)> > > listeners;

And I am starting to work on the callback API.

I will add the following lines to the namespace of Player:

std::vector<std::function<void(int dir)>> main_player_moved_listeners;
...
more listeners

That callback API we use for the FileRequest stuff is not very well designed. Is just really old code.

I actually wrote a callback Api because you asked about it but forgot to share it:

File callback.h:

/*
 * This file is part of EasyRPG Player.
 *
 * EasyRPG Player is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * (at your option) any later version.
 *
 * EasyRPG Player is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with EasyRPG Player. If not, see <http://www.gnu.org/licenses/>.
 */

#ifndef EP_CALLBACK_H
#define EP_CALLBACK_H

#include <lcf/scope_guard.h>
#include <vector>
#include <functional>
#include <utility>
#include <algorithm>

template</*typename Ret, */typename ...Args>
class Callback {
public:
	using Ret = void;
	using Fn = std::function<Ret(Args...)>;
	using BindingScopeGuard = lcf::ScopeGuard<std::function<void()>>;

	template<typename T>
	int Bind(Ret (T::*func)(Args...), T* that, Args... args) {
		Fn f = std::bind(std::mem_fn(func), that, std::placeholders::_1, args...);
		return Bind(f);
	}

	int Bind(Fn func) {
		listeners.push_back({next_id, func});
		return next_id++;
	}

	template<typename T>
	BindingScopeGuard BindWithGuard(Ret (T::*func)(Args...), T* that, Args... args) {
		int id = Bind(func, that, args...);

		return lcf::ScopeGuard<std::function<void()>>([this, id=id]() {
			Unbind(id);
		});
	}

	BindingScopeGuard BindWithGuard(Fn func) {
		int id = Bind(func);

		return lcf::ScopeGuard<std::function<void()>>([this, id]() {
			Unbind(id);
		});
	}

	void Unbind(int id) {
		auto it = std::find_if(listeners.begin(), listeners.end(), [id](auto& listener){
			return listener.first == id;
		});
		assert(it != listeners.end());

		listeners.erase(it);
	}

	void Call(Args... args) {
		for (auto& listener: listeners) {
			listener.second(std::forward<Args>(args)...);
		}
	}

	void Clear() {
		listeners.clear();
	}

private:
	std::vector<std::pair<int, Fn>> listeners;

	int next_id = 1;
};

#endif

Creating a new callback:

To game_variables.h add:

	int GetMaxDigits() const;

	Callback<int, int> VariableChangedCallback;
private:

To game_variables.cpp add (in SetOp):

	v = Utils::Clamp(value, _min, _max);
	VariableChangedCallback.Call(variable_id, v);
	return v;

Attaching to the callback:

// The function returns an id, you can manually call Unbind(id) to detach
Main_Data::game_variables->VariableChangedCallback.Bind([](int a, int b) {
	Output::Warning("{} = {}", a, b);
});

// Attaching with a scope guard (the callback will automatically Unbind when the
// guard is not in scope anymore.
// That similiar to this RequestBindingWeak thing but a better solution.
auto guard = Main_Data::game_variables->VariableChangedCallback.BindWithGuard([](int a, int b) {
	Output::Warning("X{} = {}", a, b);
});

// Bind a function in the current class that has signature (int, int)
Main_Data::game_variables->VariableChangedCallback.Bind(&MyClass::MyMember, this) {
	Output::Warning("{} = {}", a, b);
});

The unbinding is necessary when the callback function is registred to an object that is destroyed. To prevent a use after free here you can do something like this:

class MyClassThatRegistersACallbackFunction {
public:
	MyClassThatRegistersACallbackFunction() {
		// GOOD: When MyClassThatRegisters... is destroyed the binding automatically unbinds
		// because the scope guard is destroyed
		guard = Main_Data::game_variables->VariableChangedCallback.BindWithGuard(&MyClassThatRegistersACallbackFunction::HandlerFunction, this);
	}

	void HandlerFunction(int, int) {}

private:
	BindingScopeGuard guard;
}

@carstene1ns
Copy link
Member

This callback api is really useful, thinking of steam integration and such.
Maybe we should ship this somewhere in player, since that makes it easier to include for forks.

@Ghabry
Copy link
Member

Ghabry commented Jun 10, 2024

Yeah my plan was to upstream this very soon. Could be also used for some debug features (e.g. Variable trace) as @florianessl adds so many currently.

Just waiting for some "real life" usage by e.g. @monokotech to see if there are any shortcomings of this API I didn't think of.

Also possible to add "event handlers" for Windows, e.g. "OnDecision", "OnCancel" etc.

@Ghabry Ghabry added this to the 0.8.1 milestone Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants