From c14aa0744205eff034d5d66d172d243123df6a80 Mon Sep 17 00:00:00 2001 From: silverice Date: Sun, 13 Mar 2016 21:21:09 +0300 Subject: [PATCH] cosmetics --- .../src/collections/dyn_form_watcher.h | 7 ++- .../src/collections/dyn_form_watcher.hpp | 23 ++++++--- JContainers/src/collections/item.h | 50 +++---------------- JContainers/src/skse/skse.cpp | 2 +- JContainers/src/util/stl_ext.h | 8 +-- 5 files changed, 31 insertions(+), 59 deletions(-) diff --git a/JContainers/src/collections/dyn_form_watcher.h b/JContainers/src/collections/dyn_form_watcher.h index b0e67e1c..db55d3d8 100644 --- a/JContainers/src/collections/dyn_form_watcher.h +++ b/JContainers/src/collections/dyn_form_watcher.h @@ -60,12 +60,12 @@ namespace form_watching { }; class form_ref { - form_entry_ref _watched_form; public: explicit form_ref(form_entry_ref&& entry) : _watched_form(std::move(entry)) {} + explicit form_ref(const form_entry_ref& entry) : _watched_form(entry) {} form_ref& operator = (form_ref&& entry) { if (this != &entry) { @@ -75,8 +75,7 @@ namespace form_watching { } form_ref() = default; - - explicit form_ref(const form_entry_ref& entry) : _watched_form(entry) {} + form_ref& operator = (const form_ref &) = default; form_ref(FormId id, form_observer& watcher); form_ref(const TESForm& form, form_observer& watcher); @@ -154,7 +153,7 @@ namespace form_watching { // "Stupid" form_ref comparison functions: // the functions don't care whether the @form_refs are really equal or not - - // really equal form_refs have equal @_watched_form field + // really equal form_refs have equal @_watched_form fields // The comparison is NOT stable namespace comp { template diff --git a/JContainers/src/collections/dyn_form_watcher.hpp b/JContainers/src/collections/dyn_form_watcher.hpp index 9469a44f..d1a17fe4 100644 --- a/JContainers/src/collections/dyn_form_watcher.hpp +++ b/JContainers/src/collections/dyn_form_watcher.hpp @@ -171,18 +171,20 @@ namespace collections { auto formId = fh::form_handle_to_id(handle); { + // Since it's impossible that two threads will delete the same form simultaneosly + // we can skip some thread safe stuff auto itr = _watched_forms.find(formId); if (itr != _watched_forms.end()) { - // read and write std::lock_guard guard{ spinlock_for(formId) }; auto watched = itr->second.lock(); if (watched) { watched->set_deleted(); - log("flag form-entry %" PRIX32 " as deleted", formId); + itr->second.reset(); + + log("flagged form-entry %" PRIX32 " as deleted", formId); } - itr->second.reset(); } } } @@ -253,8 +255,6 @@ namespace collections { return nullptr; } - log("querying form-entry %" PRIX32, fId); - { std::lock_guard guard{ spinlock_for(fId) }; @@ -262,15 +262,24 @@ namespace collections { if (itr != _watched_forms.end()) { auto watched = itr->second.lock(); if (!watched) { - // what if two threads trying assign?? - // both threads are here or one is here and another performing @on_form_deleted func. + // form-entry and real form has been deleted (recently) + // watch the form again, create entry, assuming that a new form with such ID exists + + // this code assumes that @watch_form tries to watch real existing form + // rather than the one from JSON itr->second = watched = form_entry::make(fId); + + log("queried, re-created form-entry %" PRIX32, fId); + } + else { + log("queried form-entry %" PRIX32, fId); } return watched; } else { auto watched = form_entry::make(fId); _watched_forms[fId] = watched; + log("queried, created new form-entry %" PRIX32, fId); return watched; } } diff --git a/JContainers/src/collections/item.h b/JContainers/src/collections/item.h index b7a351cb..c2f75ca8 100644 --- a/JContainers/src/collections/item.h +++ b/JContainers/src/collections/item.h @@ -56,9 +56,11 @@ namespace collections { } } - item() {} + item() = default; + item(const item& other) = default; + item& operator = (const item& other) = default; + item(item&& other) : _var(std::move(other._var)) {} - item(const item& other) : _var(other._var) {} item& operator = (item&& other) { if (this != &other) { @@ -67,11 +69,6 @@ namespace collections { return *this; } - item& operator = (const item& other) { - _var = other._var; - return *this; - } - const variant& var() const { return _var; } @@ -85,7 +82,7 @@ namespace collections { } item_type type() const { - return item_type(_var.which() + 1); + return item_type(_var.which() + item_type::none); } template T* get() { @@ -114,7 +111,6 @@ namespace collections { explicit item(SInt32 val) : _var(val) {} explicit item(int val) : _var((SInt32)val) {} explicit item(bool val) : _var((SInt32)val) {} -// explicit item(FormId id) : _var(weak_form_id(id)) {} explicit item(const form_ref& id) : _var(id) {} explicit item(form_ref&& id) : _var(std::move(id)) {} @@ -124,10 +120,6 @@ namespace collections { explicit item(std::string&& val) : _var(std::move(val)) {} // the Item is none if the pointers below are zero: -/* - explicit item(const TESForm *val) { - *this = val; - }*/ explicit item(const char * val) { *this = val; } @@ -140,10 +132,6 @@ namespace collections { explicit item(const object_stack_ref &val) { *this = val.get(); } - /* - explicit Item(const BSFixedString& val) : _var(boost::blank()) { - *this = val.data; - }*/ item& operator = (unsigned int val) { _var = (SInt32)val; return *this; } item& operator = (int val) { _var = (SInt32)val; return *this; } @@ -157,18 +145,6 @@ namespace collections { item& operator = (boost::none_t) { _var = boost::blank(); return *this; } item& operator = (object_base& v) { _var = &v; return *this; } -/* - item& operator = (FormId formId) { - // prevent zero FormId from being saved - if (formId != FormId::Zero) { - _var = weak_form_id{ formId }; - } - else { - _var = blank(); - } - return *this; - }*/ - template item& _assignPtr(T *ptr) { if (ptr) { @@ -193,17 +169,6 @@ namespace collections { return _assignPtr(val); } -/* - item& operator = (const TESForm *val) { - if (val) { - _var = weak_form_id{ *val }; - } - else { - _var = blank(); - } - return *this; - }*/ - object_base *object() const { if (auto ref = boost::get(&_var)) { return ref->get(); @@ -218,7 +183,7 @@ namespace collections { else if (auto val = boost::get(&_var)) { return *val; } - return 0; + return 0.f; } SInt32 intValue() const { @@ -242,8 +207,7 @@ namespace collections { } TESForm * form() const { - auto frmId = formId(); - return frmId != FormId::Zero ? skse::lookup_form(frmId) : nullptr; + return skse::lookup_form(formId()); } FormId formId() const { diff --git a/JContainers/src/skse/skse.cpp b/JContainers/src/skse/skse.cpp index 0bc8c22b..cf724cad 100644 --- a/JContainers/src/skse/skse.cpp +++ b/JContainers/src/skse/skse.cpp @@ -200,7 +200,7 @@ namespace skse { } TESForm* lookup_form(FormId handle) { - return g_current_api->lookup_form(handle); + return handle != FormId::Zero ? g_current_api->lookup_form(handle) : nullptr; } const char * modname_from_index(uint8_t idx) { diff --git a/JContainers/src/util/stl_ext.h b/JContainers/src/util/stl_ext.h index b439372a..47596383 100644 --- a/JContainers/src/util/stl_ext.h +++ b/JContainers/src/util/stl_ext.h @@ -4,7 +4,7 @@ namespace util { - namespace { + namespace aux { struct tree_erase_if_eraser { template typename TreeContainer::iterator operator()(TreeContainer& cnt, typename TreeContainer::const_iterator itr) const { @@ -13,7 +13,7 @@ namespace util { }; } - template + template void tree_erase_if(TreeContainer&& container, Predicate&& pred, Eraser&& eraser = Eraser{}) { for (auto itr = container.begin(); itr != container.end();) { if (pred(*itr)) { @@ -25,7 +25,7 @@ namespace util { } } - namespace { + namespace aux { template struct copy_const_qual { @@ -46,7 +46,7 @@ namespace util { template< typename Enum, - typename Number = typename copy_const_qual < Enum, typename std::underlying_type::type >::type + typename Number = typename aux::copy_const_qual < Enum, typename std::underlying_type::type >::type > inline auto to_integral_ref(Enum & e) -> Number & { return reinterpret_cast(e);