-
Notifications
You must be signed in to change notification settings - Fork 112
Conversation
I couldn't get rid of the const for class_sym, but I moved it to a "from_mlet" static method now.
7b272b2
to
a612b9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
.def_static( | ||
"from_mlet", | ||
[](char let) -> const class_sym * { return &def_monsyms[let]; }, | ||
py::return_value_policy::reference) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is... interesting. Was this to bypass const?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NVM, I saw the comment in the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit of an unusual situation for pybind: NetHack maintains an array of class_sym structs. We invent a Python class for class_syms, but we don't actually want to create (or modify) any class_sym objects, just expose the entries of the array. Returning a pointer and using py::return_value_policy::reference makes pybind not take ownership.
What doesn't work is turning this into init with the py::nodelete trick as in permonst (same situation otherwise). So we'll just use this static method.
This is to have a slightly better handle on #21.