Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nethack 3.6.7 patch only #18

Merged
merged 13 commits into from
Jun 8, 2024
Merged

Nethack 3.6.7 patch only #18

merged 13 commits into from
Jun 8, 2024

Conversation

StephenOman
Copy link
Collaborator

This PR is focused on pulling in the changed files in NetHack 3.6.7 Released and reapplying the NLE-only changes (which are invisible in the file comparison below).

Several files have been deleted from NetHack, a few new ones have been added. One NLE test needed minor modification.

Also removed the requirement for Facebook Contributor License Agreement.

Copy link
Owner

@heiner heiner left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

So no changes to the cmake files required?

@StephenOman
Copy link
Collaborator Author

Not that I could see. 😀

There was a fair bit of preparation work in this. I was experimenting in the other upgrade branch with some ideas to make it easier to upgrade over time. I'll put them into some other PRs so you can review and see what you think.

@heiner
Copy link
Owner

heiner commented Jun 5, 2024

One thing that I personally like but that might be too annoying is actually merging upstream NetHack, i.e., having their commits in our commit history. One can do that in various ways (cherry-picking, git merge, git rebase onto?) but I understand if that's too involved.

@StephenOman
Copy link
Collaborator Author

We could look at moving the core NetHack files into a GitHub Subtree of NLE. That way we can pull upstream changes and merge them, retaining the NLE specific changes. Would need a bit of change to the CMake setup.

@heiner
Copy link
Owner

heiner commented Jun 6, 2024

I think that wouldn't work well with the changes we have to make in NetHack core files, of which there are a few. Just the commits from upstream NetHack would be good. You could probably get that by going to current main, merging upstream nethack with -X theirs and then applying whatever diff moves you to the state of the branch in this PR.

@StephenOman StephenOman merged commit 93a601b into main Jun 8, 2024
16 checks passed
@StephenOman StephenOman deleted the nethack-3.6.7-patch-only branch June 8, 2024 11:29
@StephenOman StephenOman added the enhancement New feature or request label Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants