Getting involved

[e14]

As I find myself with some free time I would like to offer helping out in this project, but I don't know where to best start or where my help would be most appreciated.

What you can expect from me; I've written a ton of Lua, I know C, mostly from programming µCs (and courses), and a few other languages. I don't consider myself proficient in C++ yet, but I do know OOP from Java (so, how hard can it be™ - right...?)

Right now I think I need to get to know the codebase first, so if you have any direct ideas of issues for me where to start - just throw them at me before I step on any existing planning...

I saw that issue (https://github.com/cuberite/cuberite/issues/4363) which looks like a good opportunity to start with and get to know the C codebase as it looks fairly simple, not sure on the Lua side, I looked over the issues, but many of them seemed to depend on some design decision

I can also certainly help look through old issues that may be not applicable anymore for one reason or another, but I'd rather code

Cheers

[xoft]

That's great news. We could always use a helping hand.

I think that particular issue is a bit controversial and may need a bit more discussion among the team. https://github.com/cuberite/cuberite/issues/4375 should be a much better candidate - it's really self-contained, it means writing entirely new code, has almost zero chance of breaking stuff and it's crystal clear we'll be needing it. On the other hand, it will be a long time before it is possible to actually test it out in real world, at first it will only need to be tested using internal tests, since it will be used by the 1.13+ protocols, which will take a lot of time to develop.

Do note that we're currently trying to transition from supporting pre-1.13 Minecraft to supporting 1.13+, which breaks basically everything. That means that there will be a point where most of the code will be broken and will need fixing to get it to work again; huge amount of changes, and we've got almost zero developer workforce. A few people nibbling here and there currently, since most of us are taken by family matters or school or others.

[e14]

https://github.com/cuberite/cuberite/issues/4375 should be a much better candidate - it's really self-contained, it means writing entirely new code, has almost zero chance of breaking stuff and it's crystal clear we'll be needing it. On the other hand, it will be a long time before it is possible to actually test it out in real world, at first it will only need to be tested using internal tests, since it will be used by the 1.13+ protocols, which will take a lot of time to develop.

Thanks, yes I noticed the 1.13 issue. Hard to miss

I'll look into #4375, but your OTOH brings me to a question... I found only integration tests, are Unit tests a goal/non-goal/did I miss them? Especially since you mention that it may be a long while until the class could actually be integrated, that kinda calls for some unit tests

[xoft]

We have just Tests, we don't distinguish between unit, integration or whatever else. And they are very rare, most of Cuberite was written without any testing. We're adding them as we go along, and I think the 1.13 rewrite is a good opportunity to add many more, so that's what I've been doing so far. The problem is that everything depends on everything else, so testing is difficult.

I think the BlockTypeRegistry tests ( https://github.com/cuberite/cuberite/tre...peRegistry ) are a good example of what I want the tests to look like.

[e14]

We have just Tests, we don't distinguish between unit, integration or whatever else. And they are very rare, most of Cuberite was written without any testing. We're adding them as we go along, and I think the 1.13 rewrite is a good opportunity to add many more, so that's what I've been doing so far. The problem is that everything depends on everything else, so testing is difficult.

I think the BlockTypeRegistry tests ( https://github.com/cuberite/cuberite/tre...peRegistry ) are a good example of what I want the tests to look like.

Hey, just FYI/Status, it took me longer than expected (ok, and I binged some show) to get used to cmake and get into the structure of the tests.

Turns out my answer to the question about tests was "missed something", especially the SELF_TEST environment variable. Sure there could be more tests, but I think that's normal. I'm assuming the number of tests is limited due to dev time and there is no issue in me writing more of them.



Some more questions:

I'm uncertain about how strongly you want that intermediate format. Is it fine to directly use the JSON output from the generator? Parsing JSON should have no significant impact on startup performance, it will mostly be I/O-bound anyway, while using it directly should reduce chances of bugs and improve future compatibility.

I don't have an issue writing a converter (actually I basically finished it before realising this), but from my (currently still limited) perspective, I think it should be skipped.
libjsonpp already is in the project, and I'm using that.

Where should I put the build/ci instructions for generating the generator output? And where should the mappings be put, whether JSON or TSV?

Could you give me some hints on where to find a reference to what subset of C11 VC++ supports? I'm on Linux, so I can't just test it, or is CMake configuring gcc to build against it anyway?

[xoft]

The problem with the original JSON output is that it's too large to put into the git repo, and we want the user experience to be as smooth as possible, downloading / generating files shouldn't be needed for running the server. Therefore, the server has to contain the map somehow in the default distribution. Another reason: I'm not sure about what lawyers would make of it, if we simply committed the raw JSON generated by a copyrighted program into our repository. Refined data seems more likely to pass

The mentioned future compatibility is questionable as well - if Mojang changes the registry output format, we'd need a new parser in Cuberite itself, while if using our own data we could simply change the parser we use to generate the data file.

The converter could be in the Tools folder, among the other more or less useful stuff there (BUILD_TOOLS=1 to build them). The actual input file for the map should be in the Server folder, likely in a subfolder, numbered by the protocol version that generated it (there will be multiple such files, one for each protocol version). I don't really care about the actual format used, though TSV seems a bit easier to grasp.

About C++11, VC supports most of it, we generally haven't run into anything that would give us trouble. At the very worst, when you open a pull request, mark it as draft, CI will still build it and we'll know it still needs work on your end. That way you'll get it checked, we use Appveyor for VC builds.

[e14]

BUILD_TOOLS=1 to build them

OMG thanks! It's the small things, I tell you...

The lawyer thing, I actually wanted to ask about it here and decided to skip it for now and not bother you .

If it's about having a converter in between, I could flatten the format, eg.:

Code:

[
  {
    "props": {
      "axis": "x"
    },
    "id": 72,
    "name": "minecraft:oak_log"
  },
  {
    "props": {
      "axis": "y"
    },
    "id": 73,
    "name": "minecraft:oak_log"
  },
]

Then the size difference should be minimal once zipped.

I guess we can argue about the future compatibility thing, but I'd say if they stick to JSON, parsing it using JSON will increase the chance that changes are implemented more easily than if you're converting to a completely different representation.

At the very worst, when you open a pull request, mark it as draft, CI will still build it and we'll know it still needs work on your end. That way you'll get it checked, we use Appveyor for VC builds.

I'll do that, thanks!

[xoft]

BUILD_TOOLS=1 to build them

OMG thanks! It's the small things, I tell you...

It's actually at the top of our top-level CMakeLists.txt file, so I'd say the discoverability should be high.

[e14]

It's actually at the top of our top-level CMakeLists.txt file, so I'd say the discoverability should be high.

Yes, that's due to me being unused to CMake, sorry

After you mentioned it, I immediately found it in the CMakeLists configs.

[xoft]

Is there anything that would have made you aware of those settings before? Something we could improve? Perhaps listing those values at the end of the configure process, as a summary?