Skip to content

Commit efaea1e

Browse files
committed
Better implementation for FileWrapper / DirectoryWrapper.
1 parent e204c00 commit efaea1e

File tree

12 files changed

+399
-249
lines changed

12 files changed

+399
-249
lines changed

README.md

+22
Original file line numberDiff line numberDiff line change
@@ -44,3 +44,25 @@ sub-directories:
4444
- [`tests/runner`](tests/runner/) contains C++ tests, using GTest
4545
Tests in this project instantiate a Python runner and then use it to check that
4646
plugins implemented in Python can be used properly on the C++ side.
47+
48+
## Building & Running tests
49+
50+
Tests are not built by default with `mob`, so you will need to run `cmake` manually
51+
with the proper arguments.
52+
53+
You need to define `PLUGIN_PYTHON_TESTS` with `-DPLUGIN_PYTHON_TESTS` when running
54+
the configure step of cmake.
55+
56+
You can then build the tests
57+
58+
```bash
59+
# replace vsbuild with your build folder
60+
cmake --build vsbuild --config RelWithDebInfo --target "python-tests" "runner-tests"
61+
```
62+
63+
To run the tests, use `ctest`
64+
65+
```bash
66+
# replace vsbuild with your build folder
67+
ctest.exe --test-dir vsbuild -C RelWithDebInfo
68+
```

src/mobase/mobase.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ PYBIND11_MODULE(mobase, m)
6868
// typing stuff to be consistent with stubs and allow plugin developers to properly
6969
// type their code if they want
7070
{
71-
m.add_object("Path", py::module_::import("pathlib").attr("Path"));
7271
m.add_object("TypeVar", py::module_::import("typing").attr("TypeVar"));
7372

7473
auto s = m.attr("__dict__");
@@ -83,11 +82,6 @@ PYBIND11_MODULE(mobase, m)
8382
"MoVariant",
8483
py::eval("None | bool | int | str | list[object] | dict[str, object]"));
8584

86-
// same things for FileWrapper and DirectoryWrapper
87-
//
88-
m.add_object("FileWrapper", py::eval("str | PyQt6.QtCore.QFileInfo | Path", s));
89-
m.add_object("DirectoryWrapper", py::eval("str | PyQt6.QtCore.QDir | Path", s));
90-
9185
// same thing for GameFeatureType
9286
//
9387
m.add_object("GameFeatureType", py::eval("TypeVar('GameFeatureType')", s));

src/mobase/pybind11_all.h

+92-40
Original file line numberDiff line numberDiff line change
@@ -12,78 +12,130 @@
1212

1313
#include "pybind11_qt/pybind11_qt.h"
1414

15-
#include "pybind11_utils/arg_wrapper.h"
1615
#include "pybind11_utils/functional.h"
1716
#include "pybind11_utils/shared_cpp_owner.h"
17+
#include "pybind11_utils/smart_variant_wrapper.h"
1818

1919
#include <isavegame.h>
2020
#include <pluginrequirements.h>
2121

2222
namespace mo2::python {
2323

24-
// struct to wrap "path" object between C++ and Python, allowing to mix
25-
// pathlib.Path, QString, QFileInfo and QDir when possible
26-
//
27-
class BasePathWrapper {
28-
protected:
29-
// we store a std::filesystem::path because it can be converted to most thing,
30-
// even though we lose basic functionality on QDir (name filter, etc.)
31-
std::filesystem::path path_;
24+
namespace detail {
25+
26+
template <>
27+
struct smart_variant_converter<QString> {
3228

33-
public:
34-
BasePathWrapper() = default;
35-
BasePathWrapper(BasePathWrapper const&) = default;
36-
BasePathWrapper& operator=(BasePathWrapper const&) = default;
37-
BasePathWrapper(BasePathWrapper&&) = default;
38-
BasePathWrapper& operator=(BasePathWrapper&&) = default;
29+
static QString from(std::filesystem::path const& path)
30+
{
31+
return QString::fromStdWString(path.native());
32+
}
3933

40-
BasePathWrapper(std::filesystem::path const& path) : path_{path} {}
41-
BasePathWrapper(QString const& path) : path_{path.toStdWString()} {}
34+
static QString from(QFileInfo const& fileInfo)
35+
{
36+
return fileInfo.filePath();
37+
}
4238

43-
operator QString() const { return QString::fromStdWString(path_.native()); }
44-
operator std::filesystem::path() const { return path_; }
45-
};
39+
static QString from(QDir const& dir) { return dir.path(); }
40+
};
4641

47-
class FileWrapper : public BasePathWrapper {
48-
public:
49-
using BasePathWrapper::BasePathWrapper;
42+
template <>
43+
struct smart_variant_converter<std::filesystem::path> {
5044

51-
FileWrapper(QFileInfo const& fileInfo)
52-
: BasePathWrapper(fileInfo.filesystemFilePath())
53-
{
54-
}
45+
static std::filesystem::path from(QString const& value)
46+
{
47+
return {value.toStdWString()};
48+
}
5549

56-
operator QFileInfo() const { return QFileInfo(path_); }
57-
};
50+
static std::filesystem::path from(QFileInfo const& fileInfo)
51+
{
52+
return fileInfo.filesystemFilePath();
53+
}
5854

59-
class DirectoryWrapper : public BasePathWrapper {
60-
public:
61-
using BasePathWrapper::BasePathWrapper;
55+
static std::filesystem::path from(QDir const& dir)
56+
{
57+
return dir.filesystemPath();
58+
}
59+
};
6260

63-
DirectoryWrapper(QDir const& dir) : BasePathWrapper(dir.filesystemPath()) {}
61+
// we do not need specialization for QFileInfo and QDir because both of them can
62+
// be constructed from std::filesystem::path and QString already
6463

65-
operator QDir() const { return QDir(path_); }
66-
};
64+
} // namespace detail
6765

66+
using FileWrapper = smart_variant<QString, std::filesystem::path, QFileInfo>;
67+
using DirectoryWrapper = smart_variant<QString, std::filesystem::path, QDir>;
68+
69+
// wrap the given function to accept FileWrapper (str | PathLike | QFileInfo) at the
70+
// given argument positions (or any valid positions if Is... is empty)
71+
//
6872
template <std::size_t... Is, class Fn>
6973
auto wrap_for_filepath(Fn&& fn)
7074
{
7175
return mo2::python::wrap_arguments<FileWrapper, Is...>(std::forward<Fn>(fn));
7276
}
7377

78+
// wrap the given function to accept DirectoryWrapper (str | PathLike | QDir)
79+
// at the given argument positions (or any valid positions if Is... is empty)
80+
//
7481
template <std::size_t... Is, class Fn>
7582
auto wrap_for_directory(Fn&& fn)
7683
{
7784
return mo2::python::wrap_arguments<DirectoryWrapper, Is...>(
7885
std::forward<Fn>(fn));
7986
}
8087

81-
} // namespace mo2::python
88+
// wrap a function-like object to return a FileWrapper instead of its return type,
89+
// useful to generate proper typing in Python
90+
//
91+
// note that QFileInfo has a __fspath__ in Python, so it is quite easy to convert
92+
// from "FileWrapper", a.k.a., str | os.PathLike | QFileInfo to Path by simply
93+
// calling Path() on the return type if necessary
94+
//
95+
// this should be combined with custom return-value in PYBIND11_OVERRIDE(_PURE), see
96+
// ISaveGame binding for an example
97+
//
98+
template <class Fn>
99+
auto wrap_return_for_filepath(Fn&& fn)
100+
{
101+
return mo2::python::wrap_return<FileWrapper>(std::forward<Fn>(fn));
102+
}
82103

83-
MO2_PYBIND11_WRAP_ARGUMENT_CASTER(mo2::python::FileWrapper, "FileWrapper", QFileInfo,
84-
std::filesystem::path, QString);
85-
MO2_PYBIND11_WRAP_ARGUMENT_CASTER(mo2::python::DirectoryWrapper, "DirectoryWrapper",
86-
QDir, std::filesystem::path, QString);
104+
// similar to wrap_return_for_filepath, except it returns a DirectoryWrapper instead
105+
// of its return type
106+
//
107+
// this is much less practical than wrap_return_for_filepath since QDir does not
108+
// expose __fspath__, so more complex things need to be done in Python, which is why
109+
// this should be used carefully (e.g., should not be used if the return type is
110+
// already QDir)
111+
//
112+
template <class Fn>
113+
auto wrap_return_for_directory(Fn&& fn)
114+
{
115+
return mo2::python::wrap_return<DirectoryWrapper>(std::forward<Fn>(fn));
116+
}
117+
118+
// convert a QList to QStringList - QString must be constructible from QString
119+
//
120+
template <class T>
121+
QStringList toQStringList(QList<T> const& list)
122+
{
123+
static_assert(std::is_constructible_v<QString, T>,
124+
"QString must be constructible from T.");
125+
return {list.begin(), list.end()};
126+
}
127+
128+
// convert a QStringList to a QList - T must be constructible from QString
129+
//
130+
template <class T>
131+
QList<T> toQList(QStringList const& list)
132+
{
133+
static_assert(std::is_constructible_v<T, QString>,
134+
"T must be constructible from QString.");
135+
return {list.begin(), list.end()};
136+
}
137+
138+
} // namespace mo2::python
87139

88140
MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::IPluginRequirement)
89141
MO2_PYBIND11_SHARED_CPP_HOLDER(MOBase::ISaveGame)

src/mobase/wrappers/game_features.cpp

+16-10
Original file line numberDiff line numberDiff line change
@@ -155,12 +155,12 @@ namespace mo2::python {
155155
public:
156156
QString BinaryName() const override
157157
{
158-
PYBIND11_OVERRIDE_PURE(FileWrapper, ScriptExtender, BinaryName, );
158+
PYBIND11_OVERRIDE_PURE(QString, ScriptExtender, binaryName, );
159159
}
160160

161161
QString PluginPath() const override
162162
{
163-
PYBIND11_OVERRIDE_PURE(DirectoryWrapper, ScriptExtender, PluginPath, );
163+
PYBIND11_OVERRIDE_PURE(DirectoryWrapper, ScriptExtender, pluginPath, );
164164
}
165165

166166
QString loaderName() const override
@@ -210,11 +210,10 @@ namespace mo2::python {
210210
}
211211
QStringList secondaryFiles(const QString& modName) const override
212212
{
213-
auto result = [&] {
213+
return toQStringList([&] {
214214
PYBIND11_OVERRIDE_PURE(QList<FileWrapper>, UnmanagedMods,
215215
secondaryFiles, modName);
216-
}();
217-
return QList<QString>(result.begin(), result.end());
216+
}());
218217
}
219218
};
220219

@@ -297,10 +296,10 @@ namespace mo2::python {
297296

298297
py::class_<ScriptExtender, PyScriptExtender>(m, "ScriptExtender")
299298
.def(py::init<>())
300-
.def("BinaryName", &ScriptExtender::BinaryName)
301-
.def("PluginPath", &ScriptExtender::PluginPath)
299+
.def("binaryName", &ScriptExtender::BinaryName)
300+
.def("pluginPath", wrap_return_for_directory(&ScriptExtender::PluginPath))
302301
.def("loaderName", &ScriptExtender::loaderName)
303-
.def("loaderPath", &ScriptExtender::loaderPath)
302+
.def("loaderPath", wrap_return_for_filepath(&ScriptExtender::loaderPath))
304303
.def("savegameExtension", &ScriptExtender::savegameExtension)
305304
.def("isInstalled", &ScriptExtender::isInstalled)
306305
.def("getExtenderVersion", &ScriptExtender::getExtenderVersion)
@@ -312,8 +311,15 @@ namespace mo2::python {
312311
.def(py::init<>())
313312
.def("mods", &UnmanagedMods::mods, "official_only"_a)
314313
.def("displayName", &UnmanagedMods::displayName, "mod_name"_a)
315-
.def("referenceFile", &UnmanagedMods::referenceFile, "mod_name"_a)
316-
.def("secondaryFiles", &UnmanagedMods::secondaryFiles, "mod_name"_a);
314+
.def("referenceFile",
315+
wrap_return_for_filepath(&UnmanagedMods::referenceFile), "mod_name"_a)
316+
.def(
317+
"secondaryFiles",
318+
[](UnmanagedMods* m, const QString& modName) -> QList<FileWrapper> {
319+
auto result = m->secondaryFiles(modName);
320+
return {result.begin(), result.end()};
321+
},
322+
"mod_name"_a);
317323
}
318324

319325
} // namespace mo2::python

src/mobase/wrappers/pyplugins.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -375,23 +375,23 @@ namespace mo2::python {
375375
}
376376
QDir gameDirectory() const override
377377
{
378-
PYBIND11_OVERRIDE_PURE(DirectoryWrapper, IPluginGame, gameDirectory, );
378+
PYBIND11_OVERRIDE_PURE(QDir, IPluginGame, gameDirectory, );
379379
}
380380
QDir dataDirectory() const override
381381
{
382-
PYBIND11_OVERRIDE_PURE(DirectoryWrapper, IPluginGame, dataDirectory, );
382+
PYBIND11_OVERRIDE_PURE(QDir, IPluginGame, dataDirectory, );
383383
}
384384
void setGamePath(const QString& path) override
385385
{
386386
PYBIND11_OVERRIDE_PURE(void, IPluginGame, setGamePath, path);
387387
}
388388
QDir documentsDirectory() const override
389389
{
390-
PYBIND11_OVERRIDE_PURE(DirectoryWrapper, IPluginGame, documentsDirectory, );
390+
PYBIND11_OVERRIDE_PURE(QDir, IPluginGame, documentsDirectory, );
391391
}
392392
QDir savesDirectory() const override
393393
{
394-
PYBIND11_OVERRIDE_PURE(DirectoryWrapper, IPluginGame, savesDirectory, );
394+
PYBIND11_OVERRIDE_PURE(QDir, IPluginGame, savesDirectory, );
395395
}
396396
QList<ExecutableInfo> executables() const override
397397
{

src/mobase/wrappers/wrappers.cpp

+8-3
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,9 @@ namespace mo2::python {
5454

5555
QStringList allFiles() const override
5656
{
57-
PYBIND11_OVERRIDE_PURE(QStringList, ISaveGame, allFiles, );
57+
return toQStringList([&] {
58+
PYBIND11_OVERRIDE_PURE(QList<FileWrapper>, ISaveGame, allFiles, );
59+
}());
5860
}
5961

6062
~PySaveGame() { std::cout << "~PySaveGame()" << std::endl; }
@@ -80,11 +82,14 @@ namespace mo2::python {
8082

8183
py::class_<ISaveGame, PySaveGame, std::shared_ptr<ISaveGame>>(m, "ISaveGame")
8284
.def(py::init<>())
83-
.def("getFilepath", &ISaveGame::getFilepath)
85+
.def("getFilepath", wrap_return_for_filepath(&ISaveGame::getFilepath))
8486
.def("getCreationTime", &ISaveGame::getCreationTime)
8587
.def("getName", &ISaveGame::getName)
8688
.def("getSaveGroupIdentifier", &ISaveGame::getSaveGroupIdentifier)
87-
.def("allFiles", &ISaveGame::allFiles);
89+
.def("allFiles", [](ISaveGame* s) -> QList<FileWrapper> {
90+
const auto result = s->allFiles();
91+
return {result.begin(), result.end()};
92+
});
8893

8994
// ISaveGameInfoWidget - custom holder to keep the Python object alive alongside
9095
// the widget

src/pybind11-utils/README.md

+5-5
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22

33
This library contains some utility stuff for `pybind11`
44

5-
## arg_wrapper.h
5+
## smart_variant_wrapper.h
66

7-
Expose a function `mo2::python::wrap_arguments` and a macro
8-
`MO2_PYBIND11_WRAP_ARGUMENT_CASTER`.
9-
These can be used to convert C++ function when exposing them to Python to accept more
10-
type than the C++ one.
7+
Expose a function `mo2::python::wrap_arguments` and a template
8+
`mo2::python::smart_variant` that can be used to expose more interesting types Python
9+
than the C++ one, e.g., accept `os.PathLike` and `QFileInfo` when a simple `QString` is
10+
expected.
1111

1212
A toy example can be found in the test folder at
1313
[`tests/python/test_argument_wrapper.cpp`](../../tests/python/test_argument_wrapper.cpp).

0 commit comments

Comments
 (0)