Astral Sorcery

Astral Sorcery

63M Downloads

Compatibility issue with structure blocks / Capsule Mod

Lythom opened this issue ยท 7 comments

commented

Context

Hello,

I'm developing the https://minecraft.curseforge.com/projects/capsule mod that allow plays to capture structures and deploy them elsewhere. It's based on the structure blocks logic vanilla minecraft uses to snapshot a region (structure block save), and to spawn it (structure block load). All errors described here can be reproduced using vanilla only mechanics.

Versions :
Forge 14.23.5.2781
Astral Sorcery 1.12.2-1.10.18
Baubles 1.12-1.5.2

Issue 1: Error in console

When despawning a full altar, I get the following stacktrace :

[17:58:22] [Client thread/INFO] [minecraft/GuiNewChat]: [CHAT] Structure 'astral template' position prepared
[17:58:25] [Server thread/WARN] [Astral Sorcery]: Tried to get a network node at a TileEntity, but didn't find one! At: dim=0, pos=BlockPos{x=3048, y=73, z=1240}
[17:58:25] [Server thread/WARN] [Astral Sorcery]: Could not find transmission node for Transmission tile 'TileCollectorCrystal'
[17:58:25] [Server thread/WARN] [Astral Sorcery]: This is an implementation error. Report it along with the steps to create this, if you come across this.
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: java.lang.Throwable
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler.removeThisNextFromSources(WorldNetworkHandler.java:186)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler.removeSource(WorldNetworkHandler.java:155)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at hellfirepvp.astralsorcery.common.starlight.transmission.TransmissionNetworkHelper.informNetworkTileRemoval(TransmissionNetworkHelper.java:149)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at hellfirepvp.astralsorcery.common.tile.base.TileNetwork.onBreak(TileNetwork.java:34)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at hellfirepvp.astralsorcery.common.block.network.BlockStarlightNetwork.breakBlock(BlockStarlightNetwork.java:46)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.world.chunk.Chunk.setBlockState(Chunk.java:615)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.world.World.setBlockState(World.java:401)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.world.gen.structure.template.Template.addBlocksToWorld(Template.java:285)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.world.gen.structure.template.Template.addBlocksToWorld(Template.java:222)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.world.gen.structure.template.Template.addBlocksToWorldChunk(Template.java:210)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.tileentity.TileEntityStructure.load(TileEntityStructure.java:563)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.tileentity.TileEntityStructure.load(TileEntityStructure.java:505)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.network.NetHandlerPlayServer.processCustomPayload(NetHandlerPlayServer.java:1806)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.network.play.client.CPacketCustomPayload.processPacket(CPacketCustomPayload.java:68)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.network.play.client.CPacketCustomPayload.processPacket(CPacketCustomPayload.java:11)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.network.PacketThreadUtil$1.run(PacketThreadUtil.java:21)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:266)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at java.util.concurrent.FutureTask.run(FutureTask.java)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.util.Util.runTask(Util.java:53)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:798)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.server.MinecraftServer.tick(MinecraftServer.java:743)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:192)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:592)
[17:58:25] [Server thread/INFO] [STDERR]: [hellfirepvp.astralsorcery.common.starlight.WorldNetworkHandler:removeThisNextFromSources:186]: 	at java.lang.Thread.run(Thread.java:748)
[17:58:25] [Server thread/WARN] [Astral Sorcery]: Could not find transmission node for Transmission tile 'TileCollectorCrystal'
[17:58:25] [Server thread/WARN] [Astral Sorcery]: This is an implementation error. Report it along with the steps to create this, if you come across this.
[17:58:25] [Client thread/INFO] [minecraft/GuiNewChat]: [CHAT] Structure loaded from 'astral template'

This error don't have visible side effect, the game keeps working as expected.

Steps to reproduce :

  • setup a structure block under a naturally spawned crystal collector in any altar
  • load another template that will overwrite the current cystal collector position
  • Stacktrace appear

Issue 2: Unworking spawned crystal collector

Steps to reproduce :

  • setup a structure block under a naturally spawned crystal collector in any altar
  • save a template that will capture the crystal (ie. named "crystal")
  • setup another structure block anywhere
  • load the "crystal" template using the structure block.
    => It's not possible to craft a luminous crafting table or a Resonating Wand

Also this is the state of the crystal after being spawned. The x/y/z nbt values are actually up to date with the newest coordinates.

/give @p astralsorcery:blockcollectorcrystal 1 0 {BlockEntityTag:{ticksExisted:5422,purity:100,sizeOverride:-1,doesSeeSky:1b,multiBlockPresent:0b,collectorType:0,size:400,fract:0,x:683,y:97,constellationName:"astralsorcery.constellation.evorsio",z:87,id:"astralsorcery:tilecollectorcrystal",wasLinkedBefore:0b,collect:100,linked:[]}}

Considerations

I understand not being able to move the crystals is a design choice of Astral Sorcery, and whatever the resolution of this issue will be I think I will blacklist the crystal so that a capsule cannot capture it by default. That said players will be able to override this blacklist configuration enventually, also I'd like to offer that possibility to OP players (ie. map pakers, modpack makers).

My expectations for those issues would be :

  1. Have the code not crash when the crystal is brutally despawned.
  2. Have the crystal work even if not spawned naturally (ie. when copied from another crystal using structure block).

I case you are accepting contributions, I'd be glad to look into it myself and would take any tips or intuitions to help getting started with those issues.

Thank you for your time !

commented

Well, yes, more or less what doom said.

For performance and gameplay reasons, the starlight network operates on a cache of network nodes, created and managed by tileentities in order to operate separately from loaded chunks.
Part of this can be seen here

As such, the blocks owning tileentities that are part of the starlight network, clear the cache when the block's broken to maintain cache consistency (for the most part. mcedit and related breaks this ofc). So it relies on those blocks being broken and they are blacklisted from most tileentity-moving-without-breaking mods.

Do note that as far as vanilla is concerned, blocks that have a tileentity are not to be moved unless broken by a player. In a way that, assuming that they can be moved without being broken, is violating vanilla constraints. Either way, not my fault.

commented

That's expected, since it blows up the starlight network to do so. Moving any TE from AS isn't recommended, and is actively discouraged. If removed and placed properly into the world using the proper block placement methods, there won't be errors. Creative-mode's shift-middle-click to block-pick and then place normally also does not see this.

commented

Thanks for you answers !

I first must partly disagree with the last point, in creative mode loading a structure block breaks the original block by replacing it using setBlockState, it calls the block "breakBlock" method on the block but there is no BreakEvent triggered. So this is a vanilla mechanic that doesn't properly break a block, and no vanilla tileentity fails from it.

My point is : conceptually I would expect Astral Sorcery tile entities to work with structure blocks. I do understand the technical trade-offs that were made.

A last question to determine the issue closing : If I submit a pull request allowing Astral Sorcery tileentities to be moved, would you accept to defer design decision of allowing tile entity moving to pack makers ?

  • If you think that the gameplay should forbid tile entities to be moveable (regardless of the technical possibility) then the issue can be closed. I'm not here to contest game design decisions.
  • If you think that it should be up to the player/modpack maker to choose if they can move the tileentity, then I'd like to take a look to make it technically possible !
commented

I'll give it a try next week then ! I'm puzzled that breakBlock is not reached somehow, I'll debug this out.

commented

It does unload the network-tile through the breakBlock: https://github.com/HellFirePvP/AstralSorcery/blob/master/src/main/java/hellfirepvp/astralsorcery/common/block/network/BlockStarlightNetwork.java#L46
Whatever's done during that breaking apparently then doesn't reach that point then.

A PR that would make them movable? It's still linked to the tileentity itself and the network data cannot be stored on the tileentity because this needs to be available even when the tileentity in question is no longer loaded. Another potential breaking point would be that the network-node is still there but the tileentity is not, because the tileentity got removed and the network never updated. If you wish to make a PR, you might wanna drop a comment describing what you'd roughly do to achieve it, so we can see how stable it would be.

If you think that the gameplay should forbid tile entities to be moveable

I think this way due to past experiences and the current technical possibilities surrounding tileentities. It just really looks like a bad idea and always was looking back. The code/architecture around it didn't change so it's still as unstable as ever.

commented

Fixed by #1121.

commented

Am going to leave this open until the version containing this released on Curseforge.