search

copygirl / BetterStorage

A Minecraft mod aimed at offering more storage options.
http://copy.mcft.net/mc/BetterStorage/
MIT License
57 stars 33 forks source link

log spam when placing a better storage crate #294

Closed Vectron closed 9 years ago

Vectron commented 9 years ago

when i place a crate in the world when i have mapwriter installed the log gets spammed with the next error:

[20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: java.lang.IllegalStateException: Can't be called client-side. [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at net.mcft.copy.betterstorage.tile.crate.TileEntityCrate.getPileData(TileEntityCrate.java:46) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at net.mcft.copy.betterstorage.tile.crate.TileEntityCrate.func_145841_b(TileEntityCrate.java:397) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at mapwriter.region.MwChunk.getBlockAndMetadata(MwChunk.java:205) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at mapwriter.region.ChunkRender.getColumnColour(ChunkRender.java:79) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at mapwriter.region.ChunkRender.renderSurface(ChunkRender.java:168) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at mapwriter.region.SurfacePixels.updateChunk(SurfacePixels.java:86) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at mapwriter.region.Region.updateChunk(Region.java:166) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at mapwriter.region.RegionManager.updateChunk(RegionManager.java:123) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at mapwriter.tasks.UpdateSurfaceChunksTask.run(UpdateSurfaceChunksTask.java:24) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at java.util.concurrent.FutureTask.run(Unknown Source) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source) [20:46:20] [Client thread/INFO] [STDERR]: [java.lang.Throwable$WrappedPrintStream:println:-1]: at java.lang.Thread.run(Unknown Source)

mapwriter uses the function writeToNBT() on tile entity's to get the nbttag. my guess is your todo marker has something to do with this.

i could catch and dispose this exception but i dont feel that that is the right thing to do.

copygirl commented 9 years ago

In my opinion (and suppose from my experience..?), writeToNBT and other things like inventory-accessing functions should not be called on the client, because it will often have invalid or missing data, which I imagine can cause crashes for some mods. I throw this exception because I think modders should expect these not to work, and it has in some cases highlighted problems with other mods (mostly due to server-side code accidentally running on the client).

I wouldn't mind hearing your opinion and points for calling writeToNBT on the client.

Vectron commented 9 years ago

the reason why i call writeToNBT is so i can check in a more generic way for bits of data i need (getting the blocks forgemultipart and carpenters block use to render on the block) this saves the need to cast it to the specific class to get the data i need. and lets me skip the need to make them a dependicy.

as for the server side code running client side. in my opinion the method writeToNBT should only gather the data and present them in a compoundtag. and i always asumed that all the data for the tileentity is availible client side when the given block is loaded.

copygirl commented 9 years ago

I agree that would probably be a good standard to follow. Even if I did actually still work on BetterStorage (I currently don't even have a dev environment set up), I'd probably leave things as-is because of the reason mentioned above.