+- Cuberite Forum (https://forum.cuberite.org)
+-- Forum: Cuberite (https://forum.cuberite.org/forum-4.html)
+--- Forum: Development (https://forum.cuberite.org/forum-13.html)
+--- Thread: Discussion incident removal blockpalettes (/thread-3312.html)
Discussion incident removal blockpalettes - NiLSPACE - 09-10-2020
Continuation discussion https://github.com/cuberite/cuberite/commit/3dae696b414d9141b3faa6872f3723332b7b6395
RE: Discussion incident removal blockpalettes - NiLSPACE - 09-10-2020
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.
RE: Discussion incident removal blockpalettes - tigerw - 09-11-2020
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.
RE: Discussion incident removal blockpalettes - bearbin - 09-11-2020
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
RE: Discussion incident removal blockpalettes - peterbell10 - 09-12-2020
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
56Some 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.
RE: Discussion incident removal blockpalettes - tigerw - 09-12-2020
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.
RE: Discussion incident removal blockpalettes - xoft - 09-13-2020
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).
RE: Discussion incident removal blockpalettes - tigerw - 09-14-2020
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
RE: Discussion incident removal blockpalettes - tigerw - 09-14-2020
Rest assured code is only going to be added from now :)