diff --git a/src/proxy/plugin_python_en.ts b/src/proxy/plugin_python_en.ts index 99302dd..4a3814e 100644 --- a/src/proxy/plugin_python_en.ts +++ b/src/proxy/plugin_python_en.ts @@ -14,48 +14,48 @@ - + ModOrganizer path contains a semicolon - + Python DLL not found - + Invalid Python DLL - + Initializing Python failed - - + + invalid problem key %1 - + The path to Mod Organizer (%1) contains a semicolon. <br>While this is legal on NTFS drives, many softwares do not handle it correctly.<br>Unfortunately MO depends on libraries that seem to fall into that group.<br>As a result the python plugin cannot be loaded, and the only solution we canoffer is to remove the semicolon or move MO to a path without a semicolon. - + The Python plugin DLL was not found, maybe your antivirus deleted it. Re-installing MO2 might fix the problem. - + The Python plugin DLL is invalid, maybe your antivirus is blocking it. Re-installing MO2 and adding exclusions for it to your AV might fix the problem. - + The initialization of the Python plugin DLL failed, unfortunately without any details. diff --git a/src/proxy/proxypython.cpp b/src/proxy/proxypython.cpp index 035063c..0e43c36 100644 --- a/src/proxy/proxypython.cpp +++ b/src/proxy/proxypython.cpp @@ -169,7 +169,12 @@ QList ProxyPython::load(const QString& identifier) if (!m_Runner) { return {}; } - return m_Runner->load(identifier); + + auto plugins = m_Runner->load(identifier); + for (auto* plugin : plugins) { + plugin->setParent(this); + } + return plugins; } void ProxyPython::unload(const QString& identifier) diff --git a/src/runner-pybind11/pybind11_all.h b/src/runner-pybind11/pybind11_all.h new file mode 100644 index 0000000..93e8e08 --- /dev/null +++ b/src/runner-pybind11/pybind11_all.h @@ -0,0 +1,15 @@ +#ifndef PYTHON_PYBIND11_ALL_H +#define PYTHON_PYBIND11_ALL_H + +#include +#include +#include +#include + +#include "pybind11_utils/functional.h" + +#include "pybind11_qt/pybind11_qt.h" + +#include "pybind11_utils/shared_cpp_owner.h" + +#endif diff --git a/src/runner-pybind11/pybind11_qt/details/pybind11_qt_sip.h b/src/runner-pybind11/pybind11_qt/details/pybind11_qt_sip.h index a01f4bf..d1a29c0 100644 --- a/src/runner-pybind11/pybind11_qt/details/pybind11_qt_sip.h +++ b/src/runner-pybind11/pybind11_qt/details/pybind11_qt_sip.h @@ -9,6 +9,8 @@ #include #include +#include "../pybind11_qt_holder.h" + namespace pybind11::detail::qt { /** @@ -96,6 +98,12 @@ namespace pybind11::detail::qt { if (data) { if constexpr (is_pointer) { value = reinterpret_cast(data); + + // transfer ownership + sipAPI()->api_transfer_to(src.ptr(), Py_None); + + // tie the py::object to the C++ one + new pybind11::detail::qt::qobject_holder(value); } else { value = *reinterpret_cast(data); diff --git a/src/runner-pybind11/pybind11_qt/pybind11_qt.h b/src/runner-pybind11/pybind11_qt/pybind11_qt.h index eafaffb..4a33b07 100644 --- a/src/runner-pybind11/pybind11_qt/pybind11_qt.h +++ b/src/runner-pybind11/pybind11_qt/pybind11_qt.h @@ -4,6 +4,7 @@ #include "pybind11_qt_basic.h" #include "pybind11_qt_containers.h" #include "pybind11_qt_enums.h" +#include "pybind11_qt_holder.h" #include "pybind11_qt_objects.h" #include "pybind11_qt_qflags.h" diff --git a/src/runner-pybind11/pybind11_qt/pybind11_qt_holder.h b/src/runner-pybind11/pybind11_qt/pybind11_qt_holder.h new file mode 100644 index 0000000..6438c8c --- /dev/null +++ b/src/runner-pybind11/pybind11_qt/pybind11_qt_holder.h @@ -0,0 +1,47 @@ +#ifndef PYTHON_PYBIND11_QT_HOLDER_HPP +#define PYTHON_PYBIND11_QT_HOLDER_HPP + +#include + +#include + +namespace pybind11::detail::qt { + + class qobject_holder : public QObject { + object p_; + + public: + template + qobject_holder(U* p) : p_{reinterpret_borrow(cast(p))} + { + setParent(p); + } + + ~qobject_holder() + { + gil_scoped_acquire s; + p_ = std::move(none()); + } + }; + +} // namespace pybind11::detail::qt + +namespace pybind11::qt { + + template + class qholder { + using type = Type; + + type* qobj_; + + public: + qholder(type* qobj) : qobj_{qobj} { new detail::qt::qobject_holder(qobj_); } + + type* get() { return qobj_; } + }; + +} // namespace pybind11::qt + +PYBIND11_DECLARE_HOLDER_TYPE(T, ::pybind11::qt::qholder) + +#endif diff --git a/src/runner-pybind11/pybind11_utils/functional.h b/src/runner-pybind11/pybind11_utils/functional.h new file mode 100644 index 0000000..d23be5a --- /dev/null +++ b/src/runner-pybind11/pybind11_utils/functional.h @@ -0,0 +1,7 @@ +#ifndef PYTHON_PYBIND11_FUNCTIONAL_H +#define PYTHON_PYBIND11_FUNCTIONAL_H + +// TODO +#include + +#endif diff --git a/src/runner-pybind11/pybind11_utils/shared_cpp_owner.h b/src/runner-pybind11/pybind11_utils/shared_cpp_owner.h new file mode 100644 index 0000000..23d2e0d --- /dev/null +++ b/src/runner-pybind11/pybind11_utils/shared_cpp_owner.h @@ -0,0 +1,98 @@ +#ifndef PYTHON_PYBIND11_SHARED_CPP_OWNER_H +#define PYTHON_PYBIND11_SHARED_CPP_OWNER_H + +// pybind11 has some issues when a Python classes extend a C++ wrapper since the Python +// object is not kept alive alongside the returned object +// +// there is a pybind11 branch called "smart_holder" that tries to solve this in a very +// complicated way (with many other features) +// +// here, we simply use a custom type_caster<> for the classes we need - see the actual +// definition in mo2::python::detail below +// +// IMPORTANT: this only works for classes that are managed by shared_ptr on the C++ +// side, not Qt object +// + +// TODO: WIP for Qt object + +namespace mo2::python::detail { + + template + struct shared_cpp_owner_caster + : pybind11::detail::copyable_holder_caster { + + // note that the actual holder type might be different in term of constness + using type = Type; + using holder_type = SharedType; + + using base = pybind11::detail::copyable_holder_caster; + using base::holder; + using base::value; + + // in load, we use the default type_caster<> to extract the shared pointer, then + // we replace it by a custom one + // + // the custom shared_ptr<> holds the py::object BUT does not really manage the + // C++ object because it will ref-count but not delete it + // + // this should work because here it's how it works: + // - the Python object holds a standard shared_ptr<> for the C++ object -> the + // C++ object remains alive as long as the Python one remains alive + // - the C++ object holds a shared_ptr<> that manages the python object -> the + // Python object remains alive as-long as there is a shared_ptr<> on the C++ + // side + // + bool load(pybind11::handle src, bool convert) + { + namespace py = pybind11; + + if (!base::load(src, convert)) { + return false; + } + + holder.reset(holder.get(), [pyobj = py::reinterpret_borrow( + src)](auto*) mutable { + py::gil_scoped_acquire s; + pyobj = std::move(py::none()); + + // we do NOT delete the object here - if this was the last reference to + // the Python object, the Python object will delete it + }); + + return true; + } + + // cast simply forward to the original type_caster<> + // + static pybind11::handle cast(const holder_type& src, + pybind11::return_value_policy policy, + pybind11::handle parent) + { + return base::cast(src, policy, parent); + } + }; + +} // namespace mo2::python::detail + +#define MO2_PYBIND11_SHARED_CPP_HOLDER(Type) \ + namespace pybind11::detail { \ + template <> \ + struct type_caster> \ + : mo2::python::detail::shared_cpp_owner_caster> { \ + }; \ + template <> \ + struct type_caster> \ + : mo2::python::detail::shared_cpp_owner_caster< \ + Type, std::shared_ptr> { \ + }; \ + } + +#include +#include + +MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::IPluginRequirement) +MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::ISaveGame) + +#endif diff --git a/src/runner-pybind11/pythonrunner.cpp b/src/runner-pybind11/pythonrunner.cpp index cedbf31..f402d1c 100644 --- a/src/runner-pybind11/pythonrunner.cpp +++ b/src/runner-pybind11/pythonrunner.cpp @@ -13,12 +13,8 @@ #include #include +#include "pybind11_all.h" #include -#include -#include -#include - -#include "pybind11_qt/pybind11_qt.h" #include #include @@ -330,7 +326,10 @@ PythonRunner::~PythonRunner() // we need to clear this here otherwise there is a crash in // finalize_interpreter() - m_PythonObjects.clear(); + // { + // py::gil_scoped_acquire s; + // m_PythonObjects.clear(); + // } // py::finalize_interpreter(); } @@ -425,7 +424,7 @@ QList PythonRunner::load(const QString& identifier) // previous call. try { - // Dictionary that will contain createPlugin() or createPlugins(). + // dictionary that will contain createPlugin() or createPlugins(). py::dict moduleDict; if (identifier.endsWith(".py")) { @@ -441,6 +440,8 @@ QList PythonRunner::load(const QString& identifier) QStringList parts = identifier.split("/"); std::string moduleName = ToString(parts.takeLast()); ensureFolderInPath(parts.join("/")); + + // check if the module is already loaded moduleDict = py::module_::import(moduleName.c_str()).attr("__dict__"); } @@ -456,24 +457,24 @@ QList PythonRunner::load(const QString& identifier) plugins.push_back(moduleDict["createPlugin"]()); // Clear for future call - PyDict_DelItemString(moduleDict.ptr(), "createPlugin"); + // PyDict_DelItemString(moduleDict.ptr(), "createPlugin"); } else if (moduleDict.contains("createPlugins")) { py::object pyPlugins = moduleDict["createPlugins"](); - if (!PySequence_Check(pyPlugins.ptr())) { - MOBase::log::error("Plugin {}: createPlugins must return a list.", + if (!py::isinstance(pyPlugins)) { + MOBase::log::error("Plugin {}: createPlugins must return a sequence.", identifier); } else { - py::list pyList(pyPlugins); - int nPlugins = py::len(pyList); - for (int i = 0; i < nPlugins; ++i) { + py::sequence pyList(pyPlugins); + size_t nPlugins = pyList.size(); + for (size_t i = 0; i < nPlugins; ++i) { plugins.push_back(pyList[i]); } } // Clear for future call - PyDict_DelItemString(moduleDict.ptr(), "createPlugins"); + // PyDict_DelItemString(moduleDict.ptr(), "createPlugins"); } else { MOBase::log::error("Plugin {}: missing a createPlugin(s) function.", diff --git a/src/runner-pybind11/wrappers/game_features.cpp b/src/runner-pybind11/wrappers/game_features.cpp index be54150..c08bdf8 100644 --- a/src/runner-pybind11/wrappers/game_features.cpp +++ b/src/runner-pybind11/wrappers/game_features.cpp @@ -146,7 +146,6 @@ namespace mo2::python { } ISaveGameInfoWidget* getSaveGameWidget(QWidget* parent = 0) const override { - // TODO: transfer ownership PYBIND11_OVERRIDE_PURE(ISaveGameInfoWidget*, SaveGameInfo, getSaveGameWidget, parent); } diff --git a/src/runner-pybind11/wrappers/pyfiletree.cpp b/src/runner-pybind11/wrappers/pyfiletree.cpp index 35850e7..5712bda 100644 --- a/src/runner-pybind11/wrappers/pyfiletree.cpp +++ b/src/runner-pybind11/wrappers/pyfiletree.cpp @@ -16,7 +16,7 @@ namespace py = pybind11; using namespace MOBase; -namespace mo2::details { +namespace mo2::detail { // filetree implementation for testing purpose // @@ -69,7 +69,7 @@ namespace mo2::details { callback_t m_Callback; }; -} // namespace mo2::details +} // namespace mo2::detail #pragma optimize("", off) @@ -329,12 +329,11 @@ namespace mo2::python { { m.def( "makeTree", - [](mo2::details::PyFileTree::callback_t callback) + [](mo2::detail::PyFileTree::callback_t callback) -> std::shared_ptr { - return std::make_shared(nullptr, "", - callback); + return std::make_shared(nullptr, "", callback); }, - py::arg("callback") = mo2::details::PyFileTree::callback_t{}); + py::arg("callback") = mo2::detail::PyFileTree::callback_t{}); } } // namespace mo2::python diff --git a/src/runner-pybind11/wrappers/pyplugins.h b/src/runner-pybind11/wrappers/pyplugins.h index d30e4d8..03a1b53 100644 --- a/src/runner-pybind11/wrappers/pyplugins.h +++ b/src/runner-pybind11/wrappers/pyplugins.h @@ -177,7 +177,6 @@ namespace mo2::python { QWidget* genFilePreview(const QString& fileName, const QSize& maxSize) const override { - // TODO: transfer ownership to C++ PYBIND11_OVERRIDE_PURE(QWidget*, IPluginPreview, genFilePreview, fileName, maxSize); } diff --git a/src/runner-pybind11/wrappers/wrappers.cpp b/src/runner-pybind11/wrappers/wrappers.cpp index d92b5cf..adbe275 100644 --- a/src/runner-pybind11/wrappers/wrappers.cpp +++ b/src/runner-pybind11/wrappers/wrappers.cpp @@ -85,7 +85,7 @@ namespace mo2::python { // ISaveGameInfoWidget py::class_> + py::qt::qholder> iSaveGameInfoWidget(m, "ISaveGameInfoWidget"); iSaveGameInfoWidget.def(py::init<>()) .def(py::init(), py::arg("parent"))