Use CMake's FetchContent to install third_party dependencies

[jbcoe]

Remove git submodules.

Managing software versions within a build allows for more easily reproducible builds.

[StephenOman]

This looks interesting!

For my education, I'd like to understand the "why" behind the change. Why is this way better than the existing way? What problem does it fix or what does it enable us to do that we couldn't do with the GitHub submodules?

[jbcoe]

Using FetchContent means that it's impossible to get into an inconsistent state where main source and submodules are not in sync. This happens fairly easily when switching between historical branches. This caught me out enough times to make the change to use FetchContent for local work.

FetchContent will make it (slightly) clearer which version of a package you're using and let you document why.

FetchContent will need an active internet connection when building which you might get away without if all your submodules are up-to date.

FetchContent prevents you from having locally modified versions of third-party code. This can be good, can be bad.

I prefer FetchContent to submodules mainly for the above reasons, but also in part due to taste. Feel free to close the PR if you're happy with the existing submodules setup.

https://blog.timhutt.co.uk/against-submodules/

[jbcoe]

Closing stale PR.

[jbcoe]

@heiner @StephenOman I'd like to get this merged before working on other modernization PRs as the current submodule setup is blocking me (see issues with draft PR #71 ).

[StephenOman]

Just to be complete, the README.md (and in a few other documents scattered about) say that to develop on NLE, you need to do:

$ git clone https://github.com/NetHack-LE/nle --recursive

I assume this change means the --recursive flag isn't needed anymore? If so, can you update the docs please?

[jbcoe]

@StephenOman thanks for spotting that. All updated now.

[jbcoe]

I've found some possible issues related to use of EXCLUDE_FROM_ALL. I'll convert this PR to a DRAFT until these are resolved.

[StephenOman]

The EXCLUDE_FROM_ALL parameter in FetchContent_Declare was added in CMake v3.28, so we need to update README.md and doc/nle/source/getting_started.rst too.

[jbcoe]

Given the CMake requirements have changed, do we want to increase the version number?

We were previously using EXCLUDE_FROM_ALL on L83 of CMakeLists.txt so I'd not anticipate any new breakages being introduced.

[StephenOman]

Releases are still manual at the moment (another thing I was thinking about maybe changing) so it doesn't really matter for now.

[jbcoe]

I'm happy to have this squashed and merged. I don't see a button to do so myself.