-
Notifications
You must be signed in to change notification settings - Fork 79
Registerable WorldFormat api #1795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
all are now abstract
leMaik
left a comment
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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?
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.
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
There was a problem hiding this comment.
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) {} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
5341fed to
200298a
Compare
200298a to
bcf8d12
Compare
|
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. |
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