Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

Update helpers #55

Merged
merged 3 commits into from
Jul 29, 2020
Merged

Update helpers #55

merged 3 commits into from
Jul 29, 2020

Conversation

heiner
Copy link
Contributor

@heiner heiner commented Jul 29, 2020

This is to have a slightly better handle on #21.

I couldn't get rid of the const for class_sym, but I moved it to
a "from_mlet" static method now.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 29, 2020
@heiner heiner force-pushed the update-helpers branch 4 times, most recently from 7b272b2 to a612b9f Compare July 29, 2020 13:55
@heiner heiner requested review from edran and rockt July 29, 2020 13:55
Copy link
Contributor

@rockt rockt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@heiner heiner merged commit fece820 into master Jul 29, 2020
@heiner heiner deleted the update-helpers branch July 29, 2020 14:37
Comment on lines +160 to +163
.def_static(
"from_mlet",
[](char let) -> const class_sym * { return &def_monsyms[let]; },
py::return_value_policy::reference)
Copy link
Contributor

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants