Conversation
all are now abstract
leMaik
left a comment
leMaik
left a comment
There was a problem hiding this comment.
I like the overall direction and the abstractions. 👍
Just some small suggestions, maybe we can simplify some code patterns while we're at it.
| @@ -34,7 +35,7 @@ public void clear() { | |||
| } | |||
|
|
|||
| public static void loadBiomeDataByteArray(Tag chunkData, BiomeData2d biomeData, BiomePalette biomePalette) { | |||
There was a problem hiding this comment.
Should this be moved to the world? Seems to be world format / chunk data dependant.
| @@ -46,7 +47,7 @@ public static void loadBiomeDataByteArray(Tag chunkData, BiomeData2d biomeData, | |||
|
|
|||
| public static void loadBiomeDataIntArray(Tag chunkData, BiomeData2d biomeData, BiomePalette biomePalette) { | |||
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
Would getMinY() and getMaxY() be a simpler interface?
chunky/src/java/se/llbit/chunky/ui/controller/WorldChooserController.java
Outdated
Show resolved
Hide resolved
| ChunkView map = mapView.getMapView(); | ||
| if (map.isRegionVisible(position)) { | ||
| Dimension dimension = mapLoader.getWorld().currentDimension(); | ||
| JavaDimension dimension = (JavaDimension) mapLoader.getWorld().currentDimension(); // FIXME: don't cast |
There was a problem hiding this comment.
what's still only in JavaDimension that is needed here? That might show what the Dimension class is still missing
There was a problem hiding this comment.
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) {} |
There was a problem hiding this comment.
That would require two octrees and a blockspec map, wouldn't it? As minimum data for rendering?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
This could use the existing Registerable interface.
NotStirred
force-pushed
the
feature/world_format
branch
from
5341fed to
200298a
Compare
NotStirred
force-pushed
the
feature/world_format
branch
from
200298a to
bcf8d12
Compare
NotStirred
changed the title
|
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. |
2 participants
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,Dimensionare now abstract classes, the previous implementation is nowJavaChunketc.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
Chunkbe 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