Skip to content

Conversation

@NotStirred
Copy link
Member

@NotStirred NotStirred commented Dec 7, 2024

I mainly want comments on general structure, everything else is bikesheddable. I'll go over everything again and polish it up before review.

Lots of changes here, so far I've only looked at loading worlds from the gui, haven't looked at reloading from the previous
session

Chunk, World, Dimension are now abstract classes, the previous implementation is now JavaChunk etc.
there is also a skeleton of registerable WorldFormats. Each is asked if a directory is valid for it, if the user selects it, the format is asked to load the world.

The ultimate goal would probably be to have Chunk be an implementation detail of java world, but that's a much larger task as it would require a rewrite of parts of the map view and scene chunk loading

Copy link
Member

@leMaik leMaik left a comment

Choose a reason for hiding this comment

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

I like the overall direction and the abstractions. 👍
Just some small suggestions, maybe we can simplify some code patterns while we're at it.

Arrays.fill(biomes, 0);
}

public static void loadBiomeDataByteArray(Tag chunkData, BiomeData2d biomeData, BiomePalette biomePalette) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be moved to the world? Seems to be world format / chunk data dependant.

}
}

public static void loadBiomeDataIntArray(Tag chunkData, BiomeData2d biomeData, BiomePalette biomePalette) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here, ie. let the biome data classes only contain the data and add per world format factories to create them?


for (RegionPosition region : regions) {
dimension.getRegion(region).parse(yMin, yMax);
((JavaDimension) dimension).getRegion(region).parse(yMin, yMax);
Copy link
Member

Choose a reason for hiding this comment

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

Since regions are handy for selecting many chunks, could we add virtual regions (= squares of many chunks) to Bedrock? As a UI feature, not as in actually loading regions, if the world format doesn't actually have regions.

Copy link
Member Author

@NotStirred NotStirred Jul 29, 2025

Choose a reason for hiding this comment

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

The idea being abstract out a virtual region that also happens to be the same size as a java region?

yMax.set(256);
mapView.setYMinMax(0, 256);
}
IntIntPair heightRange = mapLoader.getWorld().currentDimension().heightRange();
Copy link
Member

Choose a reason for hiding this comment

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

Would getMinY() and getMaxY() be a simpler interface?

ChunkView map = mapView.getMapView();
if (map.isRegionVisible(position)) {
Dimension dimension = mapLoader.getWorld().currentDimension();
JavaDimension dimension = (JavaDimension) mapLoader.getWorld().currentDimension(); // FIXME: don't cast
Copy link
Member

Choose a reason for hiding this comment

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

what's still only in JavaDimension that is needed here? That might show what the Dimension class is still missing

Copy link
Member Author

Choose a reason for hiding this comment

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

Dimension doesn't have regions. the map view is very region based.
Maybe we should simulate regions on every world to cope with this as you previously suggested

public interface Renderable extends Loadable {
void populateData(RenderableData data);

record RenderableData(Octree octree) {}
Copy link
Member

Choose a reason for hiding this comment

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

That would require two octrees and a blockspec map, wouldn't it? As minimum data for rendering?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. The intention for this api is schematics where everything should be loaded immediately.

I guess if we're simulating regions they can just be smuggled in as real worlds too. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good. Schematics support would be great! Maybe that's even be a good first test of this new API?

import java.util.List;

/** For worlds that have multiple dimensions, and fully support the map view */
public interface WorldFormat extends Loadable {
Copy link
Member

Choose a reason for hiding this comment

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

This could use the existing Registerable interface.

@NotStirred NotStirred force-pushed the feature/world_format branch from 5341fed to 200298a Compare August 23, 2025 16:44
@NotStirred NotStirred force-pushed the feature/world_format branch from 200298a to bcf8d12 Compare August 23, 2025 17:28
@NotStirred NotStirred marked this pull request as ready for review August 23, 2025 17:31
@NotStirred NotStirred changed the title WIP: Registerable WorldFormat api Registerable WorldFormat api Aug 23, 2025
@NotStirred
Copy link
Member Author

Still need to figure out the region situation, as currently a dimension needs regions for it to be viewable on the map

Additionally we could provide a simplified api (for things like schematics etc.) which implements regions, regionchangewatcher, chunkdata stuff. But I think this PR is big enough already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants