mirror of
https://github.com/godotengine/godot
synced 2024-09-05 09:58:33 +00:00
Merge pull request #72421 from myaaaaaaaaa/signal-hashmap
Store Object signals in a HashMap rather than a VMap
This commit is contained in:
commit
e0e93ce094
|
@ -38,6 +38,7 @@
|
||||||
#include "core/os/os.h"
|
#include "core/os/os.h"
|
||||||
#include "core/string/print_string.h"
|
#include "core/string/print_string.h"
|
||||||
#include "core/string/translation.h"
|
#include "core/string/translation.h"
|
||||||
|
#include "core/templates/local_vector.h"
|
||||||
#include "core/variant/typed_array.h"
|
#include "core/variant/typed_array.h"
|
||||||
|
|
||||||
#ifdef DEBUG_ENABLED
|
#ifdef DEBUG_ENABLED
|
||||||
|
@ -1019,20 +1020,23 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
|
||||||
|
|
||||||
List<_ObjectSignalDisconnectData> disconnect_data;
|
List<_ObjectSignalDisconnectData> disconnect_data;
|
||||||
|
|
||||||
//copy on write will ensure that disconnecting the signal or even deleting the object will not affect the signal calling.
|
// Ensure that disconnecting the signal or even deleting the object
|
||||||
//this happens automatically and will not change the performance of calling.
|
// will not affect the signal calling.
|
||||||
//awesome, isn't it?
|
LocalVector<Connection> slot_conns;
|
||||||
VMap<Callable, SignalData::Slot> slot_map = s->slot_map;
|
slot_conns.resize(s->slot_map.size());
|
||||||
|
{
|
||||||
int ssize = slot_map.size();
|
uint32_t idx = 0;
|
||||||
|
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
|
||||||
|
slot_conns[idx++] = slot_kv.value.conn;
|
||||||
|
}
|
||||||
|
DEV_ASSERT(idx == s->slot_map.size());
|
||||||
|
}
|
||||||
|
|
||||||
OBJ_DEBUG_LOCK
|
OBJ_DEBUG_LOCK
|
||||||
|
|
||||||
Error err = OK;
|
Error err = OK;
|
||||||
|
|
||||||
for (int i = 0; i < ssize; i++) {
|
for (const Connection &c : slot_conns) {
|
||||||
const Connection &c = slot_map.getv(i).conn;
|
|
||||||
|
|
||||||
Object *target = c.callable.get_object();
|
Object *target = c.callable.get_object();
|
||||||
if (!target) {
|
if (!target) {
|
||||||
// Target might have been deleted during signal callback, this is expected and OK.
|
// Target might have been deleted during signal callback, this is expected and OK.
|
||||||
|
@ -1195,8 +1199,8 @@ void Object::get_all_signal_connections(List<Connection> *p_connections) const {
|
||||||
for (const KeyValue<StringName, SignalData> &E : signal_map) {
|
for (const KeyValue<StringName, SignalData> &E : signal_map) {
|
||||||
const SignalData *s = &E.value;
|
const SignalData *s = &E.value;
|
||||||
|
|
||||||
for (int i = 0; i < s->slot_map.size(); i++) {
|
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
|
||||||
p_connections->push_back(s->slot_map.getv(i).conn);
|
p_connections->push_back(slot_kv.value.conn);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1207,8 +1211,8 @@ void Object::get_signal_connection_list(const StringName &p_signal, List<Connect
|
||||||
return; //nothing
|
return; //nothing
|
||||||
}
|
}
|
||||||
|
|
||||||
for (int i = 0; i < s->slot_map.size(); i++) {
|
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
|
||||||
p_connections->push_back(s->slot_map.getv(i).conn);
|
p_connections->push_back(slot_kv.value.conn);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1218,8 +1222,8 @@ int Object::get_persistent_signal_connection_count() const {
|
||||||
for (const KeyValue<StringName, SignalData> &E : signal_map) {
|
for (const KeyValue<StringName, SignalData> &E : signal_map) {
|
||||||
const SignalData *s = &E.value;
|
const SignalData *s = &E.value;
|
||||||
|
|
||||||
for (int i = 0; i < s->slot_map.size(); i++) {
|
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
|
||||||
if (s->slot_map.getv(i).conn.flags & CONNECT_PERSIST) {
|
if (slot_kv.value.conn.flags & CONNECT_PERSIST) {
|
||||||
count += 1;
|
count += 1;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1804,11 +1808,8 @@ Object::~Object() {
|
||||||
SignalData *s = &E.value;
|
SignalData *s = &E.value;
|
||||||
|
|
||||||
//brute force disconnect for performance
|
//brute force disconnect for performance
|
||||||
int slot_count = s->slot_map.size();
|
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
|
||||||
const VMap<Callable, SignalData::Slot>::Pair *slot_list = s->slot_map.get_array();
|
slot_kv.value.conn.callable.get_object()->connections.erase(slot_kv.value.cE);
|
||||||
|
|
||||||
for (int i = 0; i < slot_count; i++) {
|
|
||||||
slot_list[i].value.conn.callable.get_object()->connections.erase(slot_list[i].value.cE);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
signal_map.erase(E.key);
|
signal_map.erase(E.key);
|
||||||
|
|
|
@ -41,7 +41,6 @@
|
||||||
#include "core/templates/list.h"
|
#include "core/templates/list.h"
|
||||||
#include "core/templates/rb_map.h"
|
#include "core/templates/rb_map.h"
|
||||||
#include "core/templates/safe_refcount.h"
|
#include "core/templates/safe_refcount.h"
|
||||||
#include "core/templates/vmap.h"
|
|
||||||
#include "core/variant/callable_bind.h"
|
#include "core/variant/callable_bind.h"
|
||||||
#include "core/variant/variant.h"
|
#include "core/variant/variant.h"
|
||||||
|
|
||||||
|
@ -587,7 +586,7 @@ private:
|
||||||
};
|
};
|
||||||
|
|
||||||
MethodInfo user;
|
MethodInfo user;
|
||||||
VMap<Callable, Slot> slot_map;
|
HashMap<Callable, Slot, HashableHasher<Callable>> slot_map;
|
||||||
};
|
};
|
||||||
|
|
||||||
HashMap<StringName, SignalData> signal_map;
|
HashMap<StringName, SignalData> signal_map;
|
||||||
|
|
|
@ -386,6 +386,12 @@ struct HashMapHasherDefault {
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// TODO: Fold this into HashMapHasherDefault once C++20 concepts are allowed
|
||||||
|
template <class T>
|
||||||
|
struct HashableHasher {
|
||||||
|
static _FORCE_INLINE_ uint32_t hash(const T &hashable) { return hashable.hash(); }
|
||||||
|
};
|
||||||
|
|
||||||
template <typename T>
|
template <typename T>
|
||||||
struct HashMapComparatorDefault {
|
struct HashMapComparatorDefault {
|
||||||
static bool compare(const T &p_lhs, const T &p_rhs) {
|
static bool compare(const T &p_lhs, const T &p_rhs) {
|
||||||
|
|
|
@ -339,6 +339,48 @@ TEST_CASE("[Object] Signals") {
|
||||||
CHECK_EQ(signals_after.size(), signals_after_empty_added.size());
|
CHECK_EQ(signals_after.size(), signals_after_empty_added.size());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
SUBCASE("Deleting an object with connected signals should disconnect them") {
|
||||||
|
List<Object::Connection> signal_connections;
|
||||||
|
|
||||||
|
{
|
||||||
|
Object target;
|
||||||
|
target.add_user_signal(MethodInfo("my_custom_signal"));
|
||||||
|
|
||||||
|
ERR_PRINT_OFF;
|
||||||
|
target.connect("nonexistent_signal1", callable_mp(&object, &Object::notify_property_list_changed));
|
||||||
|
target.connect("my_custom_signal", callable_mp(&object, &Object::notify_property_list_changed));
|
||||||
|
target.connect("nonexistent_signal2", callable_mp(&object, &Object::notify_property_list_changed));
|
||||||
|
ERR_PRINT_ON;
|
||||||
|
|
||||||
|
signal_connections.clear();
|
||||||
|
object.get_all_signal_connections(&signal_connections);
|
||||||
|
CHECK(signal_connections.size() == 0);
|
||||||
|
signal_connections.clear();
|
||||||
|
object.get_signals_connected_to_this(&signal_connections);
|
||||||
|
CHECK(signal_connections.size() == 1);
|
||||||
|
|
||||||
|
ERR_PRINT_OFF;
|
||||||
|
object.connect("nonexistent_signal1", callable_mp(&target, &Object::notify_property_list_changed));
|
||||||
|
object.connect("my_custom_signal", callable_mp(&target, &Object::notify_property_list_changed));
|
||||||
|
object.connect("nonexistent_signal2", callable_mp(&target, &Object::notify_property_list_changed));
|
||||||
|
ERR_PRINT_ON;
|
||||||
|
|
||||||
|
signal_connections.clear();
|
||||||
|
object.get_all_signal_connections(&signal_connections);
|
||||||
|
CHECK(signal_connections.size() == 1);
|
||||||
|
signal_connections.clear();
|
||||||
|
object.get_signals_connected_to_this(&signal_connections);
|
||||||
|
CHECK(signal_connections.size() == 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
signal_connections.clear();
|
||||||
|
object.get_all_signal_connections(&signal_connections);
|
||||||
|
CHECK(signal_connections.size() == 0);
|
||||||
|
signal_connections.clear();
|
||||||
|
object.get_signals_connected_to_this(&signal_connections);
|
||||||
|
CHECK(signal_connections.size() == 0);
|
||||||
|
}
|
||||||
|
|
||||||
SUBCASE("Emitting a non existing signal will return an error") {
|
SUBCASE("Emitting a non existing signal will return an error") {
|
||||||
Error err = object.emit_signal("some_signal");
|
Error err = object.emit_signal("some_signal");
|
||||||
CHECK(err == ERR_UNAVAILABLE);
|
CHECK(err == ERR_UNAVAILABLE);
|
||||||
|
|
Loading…
Reference in a new issue