Discussion incident removal blockpalettes

[NiLSPACE]

Continuation discussion https://github.com/cuberite/cuberite/com...332b7b6395

[NiLSPACE]

I think removing a tigerw as a member from the repo is really harsh, but I agree with xoft that this shouldn't have been committed directly to master. If possible I'd like to make it impossible for anyone, even adminsitrators of the repo, to directly push to master.

EDIT:
To clarify, I don't think the code shouldn't have been committed directly because it was bad (I haven't really looked at the code yet) but because it prevents a process where the code could be reviewed. The overal architecture has been discussed in detail, and to just throw things away without informing the others is simply not-done.

[tigerw]

Thanks for forum post!

xoft please add me back to the organisation.

About the change in question, replacing BLOCKTYPE (8 bits) and NIBBLETYPE (8 bits) with dynamic strings is detrimental to performance and heap allocations would have eaten our lunch. See for example the slowdown in vanilla 1.13+.

Vanilla blocks will be represented in the server as shorts (16 bits hopefully) and type-safe accessors generated by Lua script automatically from Vanilla registries. Protocol translation is a simple switch statement. Strings will only make an appearance in Anvil serialisation and deserialization.

Plugins will eventually be able to add blocks to a global mapping, presumably encoded in the BlockState class to store arbitrary stringly-typed data.

[bearbin]

tigerw has now been reinstated as a regular member of the cuberite org. Owing to conflicting interests, I cannot contribute that much to this discussion save to say that:

- the code at issue should not have been committed directly to master, but
- complete removal from the org was not a proportionate response

[peterbell10]

Looking at the git history, Tiger seems to have made a habit of committing direct to master. His latest PR was merged on the 27th of July (#4780). Yet the git log shows 56 commits by him on master since then:

Code:

$ git log --author="Tiger Wang" --format=oneline --since="2020-07-27" | wc -l
56

Some of those commits are quite small. Others have changed several hundred lines of code.

IMO nobody should be committing straight to master. Not just because it sidesteps review, but also because it doesn't generate notifications. If there's a rush to get some fix in, then I would say open a PR and self-merge it with a note saying why it's urgent. I understand that PR review can be frustratingly slow and maybe we could all work to improve that. But I don't think this is the right way to speed up development.

[tigerw]

I understand the well founded concerns about tracking changes. I'll open pulls again. I'm sure we can find a balance between development velocity and proper review.

[xoft]

I wanted to comment more on this, but basically most has been said already.

Even with PRs, changing that particular piece of code without even talking to us or describing why use that particular decision feels like a complete betrayal. I've been spending a lot of my (scarce) free time on that, and it just goes through the gutter without even a word on why, or what's the vision.

I have also turned on the GH setting that not even admins can push into master, hopefully it doesn't break something else. With this, fine, let Tiger be part of the team, I acknowledge that he is putting a lot of work into the code lately (although I can't say I agree with it most).

[tigerw]

Hi xoft, I looked at history of the palette parsing code again and found that it was something you added. With this realisation your reaction makes sense, and I can totally understand your strong displeasure. I'm very sorry that I removed it without consulting or informing you, and I can assure you it was not my intention to antagonise or slight anyone, much less yourself! You were a figure I followed and aspired to be in my formative years, and still are; please accept my sincerest apologies.

I've written some docs about the change here, thoughts/comments appreciated: https://forum.cuberite.org/thread-3313.html

[tigerw]

Rest assured code is only going to be added from now :)