Chisels & Bits - For Fabric

Chisels & Bits - For Fabric

2M Downloads

Make all Tile Entities cross-world compatible, or add an option to

Ivorforce opened this issue ยท 17 comments

commented

For chiseled blocks to be available in save formats such as ReC structures or schematics, the crossWorld option passed on writeChisleData must be set to true.

I'd strongly prefer all tile entities to write their data cross-world compatible, since they aren't supposed to tap into the block map stored in the chunk (it might change per-chunk in the future, just saying), but if you prefer to keep it that way I'd at least like an option to force-enable it.

Would fix Ivorforce/RecurrentComplex#134.

commented

Doing this would cause cause a large increase in the size of the tileenties save data, as well as the size of the packets sent to the clients, and the amount of memory used to store C&B data.

Probably in the 3x + range depending on composition. So if you current C&B blocks use 30mb now they would probobly use over 100mb

I don't think that this is wise, lots of players already have memory issues.

The API is there specifically to allow people to convert to the alternate version if its needed.

If a mod wants to use it that is, I wish theres was a generic way to request a cross world tileenitty, but there isn't, Vanilla assumes that tiles can only hold items, and thus are crossworld be default, however C&B holds states, as chunks do, which in turn is where the issue comes from. Chunks are stored based on the world state map, items are not.

commented

I can't say how your version compares (haven't read through the code), but I happen to have made a similar engine to yours in 1.7 and the saved tile entities' sizes were never a problem. Perhaps a more compact save algorithm would help more than an arbitrary decision like this?

Either way, I'd still love a general option for overall cross-world compatibility. If you prefer we can also work out some kind of 'save parameter' indicating that the currently intended save is expected to be cross-world compatible, but it doesn't really help me if it ends up being a solution that works only specifically for your mod (i.e. your API).

commented

The format is already intended to be quite small, but having 4096 distinct values per block is still a lot of data. And that doesn't include any strings which are required for the cross world format. And its already compressed.

if you prefer to not use my API then perhaps you have an API I can register something with to do the conversion?

I'd rather not sit here arguing about how much memory constitutes a reasonable investment, a 3x increase in size per block to me is not proper, especially considering the number of players with memory issues already, 1.8+ causes problems for a number of players without C&B having to use 3x as much memory per block so that mods can serialize tile-entities without the API.

--

I'm not sure that having a 'cross world force' option is a suitable solution. anything created with that option on and transplanted from one world to another would automatically be the inflated size, and would only shrink when modified.

commented

To possibly give abit of context as to why this is a requested feature in ReC (Recurrent Complex).
Me and a team of builders, lore writers and modders are working on what's going to be at the end-stage a Server-bound RPG Modpack.

We're using Chisel and Bits for a lot of decorating and touch-ups on both buildings and things like rocks and bushes, etc. However since CnB doesn't come with a option to store CnB "structures" or other blocks in general (aka "Schematic Style") bigger than 1 block (at least not that I'm aware of) we're turning to ReC for our "Schematic" needs, the current thing that happens is that the CnB block simply get "removed" (aka replaced with air) which we realized after turning a short piece of "trial" railway into a schematic then loading the schematic :P

Either way this turns out I'm happy to see that there is smth being discussed about it.
Worst case I'll inform the builders to simply store the schematics without CnB and do "touch-ups" and such in the game world.

Either way, hope you both have a good holiday season o/
//Best Regards
Lanse505

commented

You may want to take a look at https://github.com/Ivorforce/IvToolkit/blob/master/src/main/java/ivorius/ivtoolkit/blocks/IvBlockCollection.java / https://github.com/Ivorforce/IvToolkit/blob/master/src/main/java/ivorius/ivtoolkit/math/IvBytePacker.java - for me, a 16x16x16 chiseled block typically had a top of 16 different blocks, which makes for a max of 2kb + 16 strings.

Again, not sure if you are doing something along these lines already, but I thought it might help you out.

API-wise, unless you got a better idea, would be some kind of thread-wide boolean somewhere easily accessible giving the 'save context' (i.e. world vs cross-world). That's shitty OOP but hey, save contexts don't really get stacked so there's little risk. What do you think?

commented

2kb is enormous for a single block. A Floor 50x50 at that size would be 5mb, As I said, C&B already has reasonable compression in place, some blocks only requiring 20-30 bytes, other getting up into the 100 or 200 range, complexity is always costly.

We clearly have very different opinions about what a "suitable" size per block would be.

I don't think that using a thread local makes much sense, if something adjusts it improperly or forgets to reset it, or some complicated exception situation arises it could cause the wrong mode to be set improperly.

From where i'm sitting, the option that makes the most sense is some way you can either register, or I can register a handler for C&B tiles by name, or something. So that they can be converted on load / on save.

commented

The method is,
public NBTTagCompound writeTileEntityToTag( NBTTagCompound tag, boolean crossWorld );

I named it a little differently then the standard method to avoid confusion, and I also include it as part of my api just for reference's sake.

Test Build should be available here, https://dvs1.progwml6.com/jenkins/job/Chisels-and-Bits/66/ after a few, Let me know if there are any issues, if everything looks good and works I'll include it in the next release.

commented

I don't really like the idea of handlers, since they can be annoying to keep updated, but I'm open to it if that's the only route you're willing to go.

Do you want to register it on some kind of API endpoint on my mod, or should we settle on some method (like writeToNBT(nbt, worldIndependent) that you keep so I can call it via reflection? I don't really mind either way.

commented

If you prefer the less formal route, I could just create the method and you could call it via reflection. I don't mind that route.

commented

Sounds good. Let me know when you have it :)

commented

Sure thing, I'm releasing the public build now, so it should be in the wild soon.

commented

Works perfectly. Thank you for the cooperation!

commented

@AlgorithmX2
Stupid Question:
But is there any chance to backport this Method addition to 1.10.2?
Same goes for the stuff on your side @Ivorforce

Reason being at least in my case the pack we're developing on is 1.10.2 and there are still a large chunks of mods either not stable enough or even ported to 1.11 yet.

It's completely fine with a no :P
Just wanted to ask the question atleast ^^

commented

If needed I could back port the method, but it really is only relevant if @Ivorforce is going to back-port as well.

Its a pretty minimal change on my end, not sure about on @Ivorforce 's side.

commented

Also @Ivorforce @AlgorithmX2
Using: C'n'B 13.7, ReC 1.2.6 & IvToolkit 1.3.1
https://www.youtube.com/watch?v=etP1D2WOUAc

Schematics doesn't seem to be working.
So no idea whose side it's at but for reference I'm posting it here

commented
commented

This method is backported to 1.10.2 officially now.