Import Irrlicht

[numberZero]

• Goal of the PR
  Simplifying maintenance. See Add IrrlichtMT as a submodule #13215 (comment)
• How does the PR work?
  Includes Irrlicht as of 1.9.0mt11 (+the following commit) ea1b583, with line ends fixed. Following commits can be applied on top of this, as in https://github.com/numberZero/minetest/pull/2.
• Does it resolve any reported issue?
• Does this relate to a goal in the roadmap?
  Yes, Roadmap irrlicht#107, “IrrlichtMt is meant to be copied into Minetest some day”
• If not a bug fix, why is this PR needed? What usecases does it solve?
  Everything that requires changes to both Mesebox and Irrlicht.

To do

This PR is Ready for Review.

How to test

Play Minetest on various platforms.

[lhofhansl]

Big fan of this in general. Will try out in a bit.

[numberZero]

@nerzhul Is this what you meant in #13215 (comment)?

[Desour]

• Skimming 100'000 lines of code is obviously not feasible. Therefore I'd like to have exact instructions on how to produce the same outcome, so that one can check that there's not some malicious code or other unwanted change included.
• It would be nice to have a final revision in the irrlicht repo and then the code reformatting done in the irrlicht repo, and only then have the irrlicht code copied to the minetest repo (all at once, but in this order). This way it should be easier to find the history of a line of code afterwards, because there's then a 1to1 mapping of first-commit-in-minetest line to archieved-irrlicht-repo line.
• Imo it makes more sense to put irrlicht in the src directory (i.e. src/irrlicht/). But it's fine by me if that's done in a later PR that then moves all the files, and changes the build process.

[erlehmann]

Huge commits can easily break Git tooling.

At least one of the commits in which lots of stuff got deleted from IrrlichtMT is able to hang some GUI git clients.

So a drawback of making one giant commit definitely exists. Just mentioning it in case no one has thought of that.

[SmallJoker]

The general consensus of today's meeting (https://irc.minetest.net/minetest-dev/2023-09-17#i_6114769) is that Irrilicht will eventually have to be imported.

I agree with #13642 (comment) that a reproduction is needed as sanity-check prior to merge. The last version of the IrrlichtMt repo should be archived to git blame and bisect (if needed).

[nerzhul]

Huge commits can easily break Git tooling.

At least one of the commits in which lots of stuff got deleted from IrrlichtMT is able to hang some GUI git clients.

So a drawback of making one giant commit definitely exists. Just mentioning it in case no one has thought of that.

to be honest it's tinier than downloading android source code. There is no issue here :)

[nerzhul]

@numberZero can you rebase ? and we can integrate it smoothly fast to close this topic :)

[numberZero]

Updated to latest masters (of both Minetest and IrrlichtMt).

Therefore I'd like to have exact instructions on how to produce the same outcome

Described in the commit message. (forgot to mention the commands need to be executed in the irrlicht directory, not in the repository root...)

• Imo it makes more sense to put irrlicht in the src directory (i.e. src/irrlicht/). But it's fine by me if that's done in a later PR that then moves all the files, and changes the build process.

I agree with your opinion but it’s not a straightforward thing to do, the build system isn’t all that simple. Besides, that would make porting existing PRs (of which there are 5 alive apparently) much harder. So I’d leave that to a subsequent PR, or rather PRs.

[numberZero]

Updated to minetest/irrlicht@ea1b583.

So core devs, what is your choice?

1. Merge Irrlicht now
2. Reformat it first, in the Irrlicht repo

In either case, there are two ways to reformat:

1. Wholesale, with e.g. sed and clang-format
2. Gradual, manually, with full review

The latter would take months for sure.

As for me, 2-1 and 1-2 are both viable options, with 1-1 okay and 2-2 the black hole.

[sfan5]

I vote for 2-1.

[nerzhul]

i'm with @sfan5 , thanks for working on this

[garymm]

Any blockers to this? What still needs to be done?

[numberZero]

Have no idea.

[Desour]

I don't know either.
And if nobody does, I guess it's safe to assume there is nothing blocking this.
@sfan5 might have an answer.

(The reformat PR (minetest/irrlicht#248) needs to be reapplied, I'll review then.)

[sfan5]

There's nothing directly blocking this.
I was just hoping we could delete the non-SDL drivers from Irrlicht before importing it.

[Desour]

I was just hoping we could delete the non-SDL drivers from Irrlicht before importing it.

I'd rather not do this. If we merge them into minetest git, we can revert the removal if necessary. And it's not like they're super duper big.

[appgurueu]

This might also help the gltf loader slightly since it shares the common dependency of Jsoncpp with Minetest.

[sfan5]

thought: We should preserve the source separation between Irrlicht and Minetest as long as reasonable because it's convenient to be able to build it independently during development. Irrlicht compiles in like half a minute while Minetest takes four.

I'd rather not do this. If we merge them into minetest git, we can revert the removal if necessary. And it's not like they're super duper big.

Alright.

[appgurueu]

This might also help the gltf loader slightly since it shares the common dependency of Jsoncpp with Minetest.

Current state of affairs: The gltf loader just fetches jsoncpp via FetchContent. Hence whether or not Irrlicht is integrated with Minetest doesn't make much of a difference to us anymore.

[sfan5]

Friendly advice: Distro packagers will curse you for doing source downloads at build-time