Dynamic Surroundings

Dynamic Surroundings

51M Downloads

[Enhancement] Support for Cubic Chunks mod

dzkm opened this issue ยท 15 comments

commented

*Mod Version: * 1.12.2-3.4.9.2

*Forge Version: *14.23.2.2612 (Latest)

Link to crash log (if applicable): No Crash

Description:
There is no footstep sound, simply as that. I'm using the medium preset. Already tried changing some values for the footstep, but nothing works.

List of Mods:
AppleSkin
Baubles
BetterFPS
CubicChunks
GlobalXP
Hwyla
Immersive Engineering
Just Enough Items
Malisiscore
Mantle
Optifine
RedstoneFlux
ServerPropertiesLAN
Tinker's Construct
Tinker's I/O
Tinker's Defense
Tinker Tool Leveling
TTFR (That font mod)
Wawla
Xaero's Minimap

commented

Try it without Cubic Chunks. It does some really strange stuff with world geometry and throws things off.

commented

Removing Cubic Chunks fixed the problem. Could you add support for it?

commented

Not right now. CC is still in early development and they change quite a number of parameters as it relates to world geometry. It could get messy.

commented

Usually the compatibility issues boil down to cubic chunks and mods interpreting the same vanilla method differently (or one may say, cubic chunks making some of the methods a bit more strict about what they accept or what they do). For example in vanilla world.isBlockLoaded(chunkX*16, 0, chunkZ*16) will work fine, but it won't on cubic chunks because the block at y=0 may not be loaded, while blocks above and below are. Or using some chunk related events - those are fired by cubic chunks in the most sane but also completely useless place, that makes most of what mods do there effectively no-op.

I haven't looked too much at the code but it's very likely you can make it work without ever including cubic chunks API, just changing the way you interact with vanilla code to be more friendly to the idea of cubic chunks. After quick look at the code, I already found 2 issues, one of them with no clear nice solution, unfortunately:
this line:

if (this.anyEmpty || this.world != world || from.x < this.minCX || from.z < this.minCZ || to.x > this.maxCX
would also need Y checks (not sure what the isAnyEmpty is for but making it work wouldn't be easy)
and the whole DynamicChunkCache becomes a bit pointless, as it will go through ChunkProvider to get Cubes internally anyway if you use normal methods on Chunk (and if it's used from other threads, it will break internals of CC). And using ExtendedBlockStorage array won't work (but due to the way you do bounds checks, also won't crash). And the height checks also mean it won't work beyond vanilla height range. Looking at the code, this appears to be an optimization, so option to disable it could help.

Aside of the 2 issues, I really wasn't able to find anything wrong.

commented

The DynamicChunkCache class is not used. The client uses the PassThroughChunkCache which uses Minecraft's ChunkCache. And as you point out the majority of the issue with support is related to Y handling.

Question - does CubicChunks report the proper sea level and build height via the World object? Also what would be the definition of cloud rendering height?

EDIT: isAnyEmpty is a flag that is set true IIF any of the chunks within the chunk cache region are not generated/loaded. The most common scenario of this to occur is upon initial world load or if the player changes location in a significant way - such as teleport. If a chunk is not available within one of the DS region scan operations DS will need to redo the scan until the cache settles.

commented

@Barteks2x Is there an official download for CC?

commented

Not on CurseForge yet, so currently you can either compile it yourself, find one of the still working jenkins server, or download from here https://oss.sonatype.org/content/repositories/public/io/github/opencubicchunks/cubicchunks/1.12.2-0.0.835.0-SNAPSHOT/cubicchunks-1.12.2-0.0.835.0-20180503.171642-2-all.jar if you want any kind of recent version.

commented

The latest one will work. Thanks for the link!

commented

Question - does CubicChunks report the proper sea level and build height via the World object? Also what would be the definition of cloud rendering height?

Currently sea level isn't handled in any nice way, but it shouldn't be hard to change (doesn't help that there is no clear definition of what sea level means, returning 64 is just good enough most of the time, but changing it to be configurable by user shouldn't be too hard. But just consider what would the sea level be for a world where the ocean never actually generates? Should it be interpreted to be the height of a biome witn average height 0? The height below which empty space is filled with water or some liquid in general? The height below which grass stops generating on the dirt? What if there is no ocean and it's actually skylands world?). Build height of the world - depends on what method you use.

  • world.getActualHeight - this one is a compatibility nightmare and I settled to an implementation that returns:

  • world.getHeight returns Integer.MAX_VALUE / 2 by default for cubic chunks world, which is the actual max supported height. This can currently be changed in world NBT. For vanilla worlds, it still returns 256.

  • world.isOutsideBuildHeight is modified for cubic chunks, and is the only method that handles negative heights correctly. But unfortunately things are going to change in 1.13 which makes this method static. Unfortunately that gives me no way to give mods any information about world height range using vanilla methods, so mods will have to assume the range is what this method returns and count on things just working out fine when a dimension has lowered height range. As CubicChunks won't throw exceptions when you access heights outside of the height range even directly from Chunk, it will probably work.

Cloud rendering height isn't changed at all yet as I don't have a clear idea for how to handle it.

isAnyEmpty flag is going to become a bit meaningless with cubic chunks, as vanilla Chunk becomes a virtual infinitely tall Column, and they are generated almost instantly. Depending on what you do with the flag, it may or may not be ok.

commented
  • Sea level shouldn't be too much of a concern assuming CC doesn't allow the changing of that value.
  • DS uses world.getHeight(), and calculates the cloud height as (world height) / 2.
  • As for isAnyEmpty I may make a different IChunkCache/IBlockAccessEx in support of CC. This will need some testing/measuring on my end to see how it impacts scanning.
commented

Sea level can be changed in world customization (different water level and different biome heights, those are separate things). Currently it doesn't affect values returned by any vanilla methods, but I can change it if there is need for that.
Calculating cloud height the way you do... this means you assume clouds are going to be WAY up in the sky. I would recommend changing it to use getActualHeight

commented

Pushed v3.4.9.14 that has compat changes that support CC. Specifically, if there is a problem with client side caching disable DS caching.

commented

I know there is a way to configure it so that it works, but I don't remember exactly what it was.

commented

So did it actually get fixed? I am experiencing issues with it, using Forge 2768 and the latest builds of DS, CC and Orelib.

commented

Currently, from my testing, DS + CC works fine in the overworld when "Enable Client Chunk Caching" is set to false (if not, the game appeared to run fine for seemingly random periods of time when first loading in but would crash shortly afterwards), but the game becomes instantly laggy (essentially frozen) the moment you go to the Nether and even a single chunk loads.