OreCruncher / DynamicSurroundings

Dynamic Surroundings mod for Minecraft 1.10.x+
MIT License
120 stars 35 forks source link

[Enhancement] Support for Cubic Chunks mod #229

Closed dzkm closed 6 years ago

dzkm commented 6 years ago

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

OreCruncher commented 6 years ago

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

dzkm commented 6 years ago

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

OreCruncher commented 6 years ago

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.

Barteks2x commented 6 years ago

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: https://github.com/OreCruncher/DynamicSurroundings/blob/e2dcc560900fec4c1ec1cec8b078c72a2ab797de/src/main/java/org/blockartistry/lib/chunk/PassThroughChunkCache.java#L70 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.

OreCruncher commented 6 years ago

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.

Barteks2x commented 6 years ago

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.

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.

OreCruncher commented 6 years ago
Barteks2x commented 6 years ago

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

OreCruncher commented 6 years ago

@Barteks2x Is there an official download for CC?

Barteks2x commented 6 years ago

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.

OreCruncher commented 6 years ago

The latest one will work. Thanks for the link!

OreCruncher commented 6 years ago

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.

Xorbah commented 5 years ago

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

Barteks2x commented 5 years ago

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

pressRtowin commented 5 years ago

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.