OreCruncher / DynamicSurroundings

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

1.10.2 crashing upon joining server #78

Closed SteveCraftwood closed 7 years ago

SteveCraftwood commented 7 years ago

1.10.2 - 3.3.6.1

1.10.2 - version 2185

https://pastebin.com/GnC7Tfxx

Crashing upon attempting to join my modded server, seems to be DynamicSurroundings, send help

OreCruncher commented 7 years ago

Saw this on OpenEye. Looks to be a timing issue. Fix will be in the next BETA.

SteveCraftwood commented 7 years ago

Thanks!

birdspider commented 7 years ago

would that be a followup bug ?

DynamicSurroundings-1.10.2-3.3.7.0.jar (from github/releases) + mc 1.10.2 + forge 1.10.2-12.18.3.2281

[22:33:31] [Client thread/ERROR] [FML]: Exception caught during firing event net.minecraftforge.event.entity.EntityJoinWorldEvent@64ac4882:
java.lang.StackOverflowError
    at org.blockartistry.mod.DynSurround.registry.BiomeInfo.getFloatTemperature(BiomeInfo.java:154) ~[BiomeInfo.class:?]
    at org.blockartistry.mod.DynSurround.registry.FakeBiome.func_180626_a(FakeBiome.java:61) ~[FakeBiome.class:?]
    at org.blockartistry.mod.DynSurround.registry.BiomeInfo.getFloatTemperature(BiomeInfo.java:154) ~[BiomeInfo.class:?]
    at org.blockartistry.mod.DynSurround.registry.FakeBiome.func_180626_a(FakeBiome.java:61) ~[FakeBiome.class:?]
    at org.blockartistry.mod.DynSurround.registry.BiomeInfo.getFloatTemperature(BiomeInfo.java:154) ~[BiomeInfo.class:?]
    at org.blockartistry.mod.DynSurround.registry.FakeBiome.func_180626_a(FakeBiome.java:61) ~[FakeBiome.class:?]
    at org.blockartistry.mod.DynSurround.registry.BiomeInfo.getFloatTemperature(BiomeInfo.java:154) ~[BiomeInfo.class:?]
    at org.blockartistry.mod.DynSurround.registry.FakeBiome.func_180626_a(FakeBiome.java:61) ~[FakeBiome.class:?]
<snip>

that being said I'm using otg + biomebundle and it worked 2 days ago - but since then my modpack updated

https://minecraft.curseforge.com/projects/open-terrain-generator

https://www.reddit.com/r/biomebundle/comments/66z7kl/biome_bundle_v5_released/

OreCruncher commented 7 years ago

Actually this is something different. What is the datestamp of the JAR you are using? (The JAR on GitHub is for testing purposes only and may be unstable.)

After looking at the problem it may be related. My guess is that a mod registered a biome late during the init process sometime after the biome registry was assembled. Can you post a link to the full log so I can check out the init order and see when the DS biome registry was initialized?

OreCruncher commented 7 years ago

@SteveCraftwood @birdspider I pushed an official v3.3.7.0 to CurseForge that addresses these issues. Let me know if they fix. @birdspider I would be interested to see your client log using this official release.

birdspider commented 7 years ago

Hi,

some baseline infos:

  1. I use ruby curse downloader and MultiMC5 on linux
  2. I basicly use the curse Restart 1.1 Modpack but I
  3. removed the following
    • TerrainControl ( the old variant of openterraingenerator )
  4. added the following
    • Optifine + a shaderpack (yeah I understand - if I shall test without it you want) - forum/other download
    • OpenTerrainGenerator 1.10.2-v14 /1.10.2-v16 - curseforge download
    • BiomeBundle v5 - curseforge download
    • Animania 1.0.4.7 - curseforge download
  5. changed the following (due to crash)
    • Dynasurroundings
    • from-pack (DynamicSurroundings-1.10.2-3.3.4.1.jar)
    • github download 1.10.2-3.3.7.0.jar (as mentioned in the github comment a view days earlier)
    • curseforge download 3.3.7.0 (right now)
  6. btw. Restart 1.0 (although without any changes) worked !

Nevertheless your 3.3.7.0 seems to continue to crash - that beeing said, I'm not entirely sure if it is the cause, so feel free to point me into another direction.

Attached are 2 Logfiles truncated so they start at Worldgen-Click. Please reply if you need the full log of the mc process.

with OpenTerrainGenerator-1.10.2-v14.jar crash_DynamicSurroundings.3.3.7.0+OpenTerrainGenerator-1.10.2-v14.jar.txt

with OpenTerrainGenerator-1.10.2-v16.jar (because I noticed there was an update) crash_DynamicSurroundings.3.3.7.0+OpenTerrainGenerator-1.10.2-v16.jar.txt

Regards,

OreCruncher commented 7 years ago

Right now I am thinking this is a DS bug since it involves only DS stuff. In a nutshell the DS logic is reporting a fake biome (an artificial construct in DS) as a real biome and that should not happen. The question is why it's happening. Ultimately it could be because of another mod, but even so I need to figure out a way to handle more gracefully than stack overflow. :)

OreCruncher commented 7 years ago

Egad. Something rewrote biome registry entries after preinit().

birdspider commented 7 years ago

@OreCruncher before I go to sleep, do you need a full log ?

OreCruncher commented 7 years ago

Nope. I think I have what I need. Thanks!

OreCruncher commented 7 years ago

@SteveCraftwood @birdspider I have another set of fixes that are going in. OTC does something weird with the Biome registry entries after mod init which is something that is suspect. Patching in code to force a reload of my data if something like this is detected. Will be in the next BETA.

I have another tweak I am making for the stack overflow condition. Best I can determine is that a null biome reference is being returned when querying the World object for a biome at a set of coordinates. If something like this were to occur the recursion would be short circuited and the situation handled. (There may be a race condition between when the client thread starts ticking and the availability of biome information from the server.)

birdspider commented 7 years ago

@OreCruncher just added DynSurroundings to OTGs Incompat Checklist - crosslinking here

(before I found their github -.- https://github.com/PG85/OpenTerrainGenerator/issues)

RikAzerion commented 7 years ago

Hey guys, OTG dev here, thanks for reporting this. @OreCruncher you're right OTG does plenty of weird stuff to the biome registry. OTG has different biomes for each world and each dimension so each time a world is loaded/unloaded biomes are registered and unregistered. OTG cannot register all it's biomes at app start because they keep changing depending on which OTG world pack(s) is/are used.

OreCruncher commented 7 years ago

@PeeGee85Spil Thanks for filling in the blank! DS normally reinitializes it's data during client connect by posting a future to the client thread. Based on what I am seeing even that is a bit too early for scanning the biome registry. Is there a better time to do the biome registry scan when OTG is installed?

EDIT: And to hijack the thread to ask a different question - it seems OTG world sea level is 51/52, but the world object reports it as 63. Any way to get that fixed up? :)

RikAzerion commented 7 years ago

Okay, are you scanning the biome registry client side or server side or both?

OreCruncher commented 7 years ago

Both.

EDIT: Further clarification - client side scan occurs in the client thread, server side occurs in server thread.

EDIT2: Link to the RegistryManager in DS.

RikAzerion commented 7 years ago

OTG registers its biomes on world creation, it sends a packet to the player on connect containing the biomes, the client then registers the biomes. This reminds me though. OTG also allows for dimensions to be created on the fly, this can be done after players have already connected so I'm not sure they will receive a packet with new biomes when they switch dimensions (note to self: test this!).

Once the player has joined the world all the biome info should be present on the client though. I usually use this (unfortunately outdated and probably incomplete) list of forge events, http://www.minecraftforum.net/forums/mapping-and-modding/mapping-and-modding-tutorials/2129838-forge-a-complete-list-of-all-forge-events. Which event are you using atm? Could you try world load or the first world tick?

OreCruncher commented 7 years ago

I listen for the client connect event here. It calls a method on the proxy instance which in turn schedules a future on the client thread to reload the registries. Recently I lowered the priority of the event handler in hopes of letting OTG do its thing but no dice. Based on your comments above it appears to be more asynchronous in behavior.

Server side does a load on demand - the first piece of logic that requests the server side RegistryManager will trigger a load. I could take this approach client side - it would be like any other "cache miss" that triggers a reload.

I think right now a reload whenever DS thinks there is an inconsistency in the registry may be the best way to go. Since OTG and create/remove biomes under various circumstances it could get messy.

Oh, any thought on the sea level? I been trying to hack a way into DS to automatically set the sea level to 51 but trying to figure out if an OTG world needs to be 51 vs. 63 is posing a problem.

RikAzerion commented 7 years ago

Sea level is configurable per world and biome for OTG so It'll probably override your sea level settings. Even if you do override sea level I don't think it should be a problem. Will do some more research for the biome registry later today when I get home from work.

OreCruncher commented 7 years ago

I think the problem is the override. The OTG world is reporting a sea level of 63 when in game it appears to be 51. This is with Biome Bundle. I noticed that they use a BiomeHeight of -1.5 (sample from Alpine.bc) which I would think would translate to a % offset from 64 which would put it to 52 or so. Granted it can change biome to biome but I would think that an overall sea level could be set to the lowest of these values from all possible biomes for the pack.

EDIT: The reason that I am hung up on the sea level is that DS uses the world sea level value to generate an artificial Underground biome for the player. This allows DS to attach specific sounds and effects to that biome when the player Y is beneath sea level. In the case of Biome Bundle standing on the surface at Y=52 is treated as underground and it strange to hear rock falls while looking at sunny skies and trees. :)

RikAzerion commented 7 years ago

In the biomeconfigs the following settings control the water levels:

RiverWaterLevel: 52

Set this to false to use the "Water / Lava & Frozen States" settings of this biome.

UseWorldWaterLevel: false

Set water level. Every empty between this levels will be fill water or another block from WaterBlock.

WaterLevelMax: 52

WaterLevelMin: 0

OreCruncher commented 7 years ago

How do I get this information via the World/WorldClient object?

RikAzerion commented 7 years ago

Hmmm you don't I think, the world and biome configs are managed by OTG seperately and aren't accessible to other mods (unless you actually read the world/biomeconfigs). I could hook something up via forge inter-mod communications for a future release but that won't help you for now. I haven't looked at the waterlevel code for a while, I'm not sure if OTG actually changes a water level setting for MC/Forge biome objects that you can actually access.

*I should think so though, I think I've seen some other stuff in MC that is dependent on water level (mob spawning?) so if OTG doesn't change the water level properly that wouldn't work. I can check the code when I get home from work.

OreCruncher commented 7 years ago

Cool. Right now I have players that run into this issue create external DS config files that explicitly state the sea level for the dimension. I am thinking of adding a more friendly override they could tweak in addition. Whenever you get to the point of reviewing the water level code I will be more than happy to test out and what not.

OreCruncher commented 7 years ago

@SteveCraftwood @birdspider @PeeGee85Spil OK - pushed a v3.3.8.0 BETA that has fixes for the various crashes as well as a sea level override in the main config to better support some OTG biome configurations. Let me know how it works for you guys.

birdspider commented 7 years ago

@OreCruncher looks good, could load/create an otg world while dynasurround was enabled - fixed for me