Skip to content
This repository has been archived by the owner on Oct 4, 2024. It is now read-only.

SurfaceTracker cleanup (WIP) #100

Open
wants to merge 2 commits into
base: MC_1.17
Choose a base branch
from

Conversation

Cyclonit
Copy link
Collaborator

This PR is supposed to tackle the following issues:

  1. Complex coupling between HeightmapNode, SurfaceTrackerLeaf and SurfaceTrackerBranch
    HeightmapNodes (LevelCube and ProtoCube) contain references to SurfaceTrackerLeaves and vice-versa. The HightmapNodes create SurfaceTrackerLeaves, which then invoke a method "sectionLoaded" that adds the SurfaceTrackerLeaf to a reference list within the HeightmapNode. Instead HeightmapNodes should simply create the leaves and add them to their internal lists themselves.

  2. Faulty handling of heightmaps during ProtoCube promotion to LevelCube
    When a LevelCube is created based on a ProtoCube, it should inherit all of the ProtoCube's heightmaps. As of right now, this behaviour is faulty because of the overly complex interactions between cubes and heightmaps.

…d new cubes. If a new cube is added to a tree, a new leaf will be created instead.

Moved invocations of HeightmapNode.sectionLoaded into ProtoCube and LevelCube respectively to loosen coupling between them and SurfaceTrackerLeaf.
Changed SurfaceTrackerLeaf.node to be final.
Added copy-constructor to SurfaceTrackerLeaf.
Changed SurfaceTrackerBranch.loadCube to support creating new leaves based on existing leaves. Uses the new copy-constructor in SurfaceTrackerLeaf.
Removed heightmap handling from LevelCube.postLoad and moved it into the proto promotion constructor.
// On creation of a new node for a cube, both the node and its parents must be marked dirty
leaf.setAllDirty();
leaf.markAncestorsDirty();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

leaf is an unattached SurfaceTrackerLeaf. There is no point in calling "markAncestorsDirty" as it does not have any.

Copy link
Member

Choose a reason for hiding this comment

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

iirc this is used elsewhere too. It's really a "mark any positions dirty if they are below the new height in myself and ancestors"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

setAllDirty() already sets everything within the leaf dirty without any checks. markAncestorsDirty() doesn't have any additional effect.

// TODO probably don't want to do this if the cube was already loaded as a CubePrimer
((LightHeightmapGetter) chunk).getServerLightHeightmap().loadCube(storage, this);
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This logic was partly duplicated between the promotion constructor and postLoad. I moved all of it into the promotion constructor for now.

@@ -43,7 +43,7 @@ protected int updateHeight(int x, int z, int idx) {
}
}

@Override public void loadCube(int localSectionX, int localSectionZ, HeightmapStorage storage, HeightmapNode newNode) {
public SurfaceTrackerLeaf loadCube(int localSectionX, int localSectionZ, HeightmapStorage storage, HeightmapNode newNode, @Nullable SurfaceTrackerLeaf protoLeaf) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

loadCube now accepts a protoLeaf to copy its data and it returns the newly created leaf. This enable promotion of "proto leaves" from ProtoCube into LevelCube.

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.

2 participants