Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Updates for 1.18.x #60

Open
wants to merge 10 commits into
base: 1.17.x
Choose a base branch
from
Open

Updates for 1.18.x #60

wants to merge 10 commits into from

Conversation

MeeniMc
Copy link
Contributor

@MeeniMc MeeniMc commented Nov 20, 2021

Update Hydrogen for 1.18.x

The game loads a world and I could play for several hours without a hitch (singleplayer survival, some scicraft machines, etc); I have used it on my own server for several weeks, all lights are green.

What has been done and what needs double checks:

  1. mixin ChunkSerializer::private static void loadEntities(ServerWorld world, NbtCompound nbt, WorldChunk chunk) does not exist anymore, need to have Jelly validate that this optimization is now indeed unecessary. The idea of the optimization is to make sure we don't keep around Lambdas with plenty of NBT data in them, a similar idea appears to have been implemented in Minecraft, see ChunkSerializer::getEntityLoadingCallback. The whole mixin has been removed in
    920301c. (safe to git revert)

  2. mixin WorldChunk::init optimization of sectionArray (formerly sections) has changed significantly in MC. I updated the code (6cfa544), but the assumption that WorldChunk will create an air block for missing sections does not hold anymore: multiple places do sectionArray[i].empty() without first testing for null. I am afraid that this is hard to repair without rewriting a lot of WorldChunk as there is no clear way to mixin into the uses of sectionArray. The whole mixin has been removed in b3c72f0. (safe to git revert)

  3. Someone with better Java skills need to validate what I did with the ImmutableMap.

  4. No bugs have been found by normal playing. Maybe requires some rigorous targeted testing.

@MeeniMc MeeniMc changed the title Updates for 1.18.x WIP: Updates for 1.18.x Nov 20, 2021
@MeeniMc MeeniMc marked this pull request as draft November 21, 2021 03:31
this.sections[i] = null;
for (int i = 0; i < sectionArray.length; i++) {
if (sectionArray[i].isEmpty()) {
sectionArray[i] = null;
Copy link
Contributor Author

@MeeniMc MeeniMc Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optimization is currently not applicable anymore because in WorldChunk we find multiple uses of sectionArray.empty() without a prior check for null. It's not immediately clear to me how we could mixin our way into intercepting the individual uses without rewritting the whole class.

@MeeniMc MeeniMc force-pushed the 1.18.x branch 2 times, most recently from f7cf8d4 to e5c1e8f Compare November 21, 2021 22:29
@MeeniMc MeeniMc changed the title WIP: Updates for 1.18.x Updates for 1.18.x Nov 21, 2021
@MeeniMc MeeniMc marked this pull request as ready for review November 21, 2021 22:41
@MeeniMc MeeniMc force-pushed the 1.18.x branch 3 times, most recently from 6f51736 to 593eeac Compare November 23, 2021 08:19
Mappings are compatible between pre4..rc1

Signed-off-by: MeeniMc <[email protected]>
…mmit 0fd8b03)

A similar idea has been implemented in Minecraft in  ChunkSerializer::getEntityLoadingCallback

Signed-off-by: MeeniMc <[email protected]>
Change the way we access protected sectionArray from the mixinWorldChunk

Signed-off-by: MeeniMc <[email protected]>
… broken

This is done by deleting mixinWorldChunk altogether

Signed-off-by: MeeniMc <[email protected]>
@MeeniMc
Copy link
Contributor Author

MeeniMc commented Nov 30, 2021

Ready for review when you have time @jellysquid3

@the-potatocouch
Copy link

the-potatocouch commented Dec 3, 2021

@MeeniMc do you have a build for 1.18?

@MeeniMc
Copy link
Contributor Author

MeeniMc commented Dec 4, 2021

The github actions have built one right here. Look into the action tab. Remember that this is not official or supported.

Signed-off-by: MeeniMc <[email protected]>
@Boobies
Copy link

Boobies commented Jan 7, 2022

Have there been any issues found with the PR or did no one have the time to review it yet? Asking so I know whether it's safe to use.

@srnyx
Copy link

srnyx commented Jan 7, 2022

Have there been any issues found with the PR or did no one have the time to review it yet? Asking so I know whether it's safe to use.

I've been using it and it's fine for me

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants