Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multipart's data should be written into TileEntity #145

Open
Licshee opened this issue Jan 14, 2014 · 23 comments
Open

Multipart's data should be written into TileEntity #145

Licshee opened this issue Jan 14, 2014 · 23 comments

Comments

@Licshee
Copy link

Licshee commented Jan 14, 2014

Instead of writing directly into chunks' NBT data, writing into TileEntity makes it possible to do copy/paste in WorldEdit, like how it does to containers.

A conversion process may be necessary to upgrade the data from current design.

@Chicken-Bones
Copy link
Owner

They are written into the TileEntity?

@Licshee
Copy link
Author

Licshee commented Jan 15, 2014

According to the MineCraft wiki, additional data like signs' text, containers' content, spawners' mob type(NEI supports this!) are stored in TileEntity so WorldEdit can copy them with little to no problem.

I learned this from iChun's Sync mod, shell constructor and shell storage can be duplicated by WE perfectly!

@Chicken-Bones
Copy link
Owner

My apologies, I was not asking whether data was written to TileEntities in general, I was saying that FMP's data is stored in TileEntities in the chunk the same way every other mod's data is

@Licshee
Copy link
Author

Licshee commented Jan 15, 2014

That's interesting, I tried WE and it does not work for ForgeMultipart.
Then I checked your code and I didn't find the part about TileEntity, but instead I found a "container" NBT in chunks' data, so I thought you are doing it in a different way.

@Chicken-Bones
Copy link
Owner

yes, I use a container NBT, but it's still written like the others, it's just converted to something else on load

@Licshee
Copy link
Author

Licshee commented Jan 15, 2014

If I understand correctly, that means the "container" itself is not associated to specifical block, and I guess that's why WE can't duplicate FMP -- I mean, when trying to use WE to copy it, WE doesn't know where to find the FMP data it have to duplicate.

By the way, the Redstone Paste mod does this wrong too, I didn't find its source code, perhaps it's closed, when I tried to use WE to duplicate blocks from said mod, it appears the duplicated blocks are linked to the originals, I guess that mod used a container like NBT too.

Anyway, iChun's Sync mod works perfectly for me, so I think check how it works is a good idea.

@Chicken-Bones
Copy link
Owner

Nope, the container tile is definately associated with the block, WE should have no issues.

@Licshee
Copy link
Author

Licshee commented Jan 15, 2014

Just tried the newest version, didn't work.

As usual, when pasted, the block appears to be "empty", but the block id shows 1281.
Another notable phenomenon: If I use WE to replace the FMP block to something else (like glass), then undo this replace, the block becomes "empty" too, block id 1281.

@Licshee
Copy link
Author

Licshee commented Jan 15, 2014

Ahh, I find your TileEntity code, now just have to figure what makes the difference.

@Licshee
Copy link
Author

Licshee commented Jan 15, 2014

Checked the code and the docs, found three notable thing:

  1. both the Sync mod and the example from http://www.minecraftforge.net/wiki/How_to_use_NBT_Tag_Compound extend from BlockContainer, dunno why it matters tho. but the most interesting fact is that even the BlockSign class extends from BlockContainer
  2. both the Sync mod and example stated above override createTileEntity
  3. I didn't find any hack/hookLoader thing in Sync like you did in FMP, neither the example have. I don't understand why you need that.

@Chicken-Bones
Copy link
Owner

  1. Forge removed that requirement a while ago in favour of Block.hasTileEntity(int meta)
  2. createTileEntity only functions when a call is made to the block and the tile doesn't exist. If you're copying it in worldedit, then the tile should exist, if it doesn't, creating a new one won't help (won't have the multipart data in it). Most blocks use this because tile creation and loading isn't very well defined
  3. Of course, it's unique to FMP. FMP requires it because the class of the tile entity is dynamic and depends on the contents of the NBT tag

FMP's tile loading functions as follows:

  1. Every FMP tile saves with the same id "savedMultipart"
  2. "savedMultipart" is mapped to an NBT container tile, which does nothing except hold the NBTTag it was created with (when the chunk loads)
  3. when a chunk load event is received, any tiles in the chunk that are an instance of TileNBTContainer are replaced with their appropriate multipart tile class and properly loaded.

Copying a tile and id in world edit 'should' work perfectly fine.

@Licshee
Copy link
Author

Licshee commented Jan 15, 2014

Thank you for your info, I got your idea.

I'm going to do more research, but I now think it might be something to do with the NBT reading/writing thing.
Not familiar with scala, understanding your code is hard to me ._.

@ghost
Copy link

ghost commented Jan 15, 2014

Lol. When i first looked at FMP i was stupid enough to think, that i need learn Scala for it. Now i love Scala, its beautiful.

@TheMadmanofChaos
Copy link

I think this issue has a nastier side effect: If you use a mod to "Move" any multi-part(Like WayofTime's Blood magic), it will cause it to vanish into limbo(BM joke). But this has caused crashes and weirdness when a small structure or pipes are teleposed and disappear.

@Chicken-Bones
Copy link
Owner

There are several mods that support moving multipart already. See the
MultipartHelper class for all your solutions.

@TheMadmanofChaos
Copy link

Thank you, I'll let Way know. I'm tired of multi-parts and conduits falling into my nice home/void...

@Licshee
Copy link
Author

Licshee commented Feb 14, 2014

I've found what caused the problem.
Now I just have to try it out

@octylFractal
Copy link

@NanaLich any updates? I've ported Forge WorldEdit to 1.7.10, but this is the last section I have to add on to ForgeWorldEdit, and it's not working.

@octylFractal
Copy link

So I've been working on this compatibility, and the NBT data is fine. The trouble occurs when createTileEntity is called.
This is the method called by ForgeWorldEdit:

    static void setTileEntity(World world, Vector position, @Nullable NBTTagCompound tag) {
        if (tag != null) {
            updateForSet(tag, position);
            TileEntity tileEntity = TileEntity.createAndLoadEntity(tag);
            if (tileEntity != null) {
                world.setTileEntity(position.getBlockX(), position.getBlockY(), position.getBlockZ(), tileEntity);
            }
        }
    }

I'm not sure what the problem is yet. But it might be WE, not FM.

@octylFractal
Copy link

Well, loadChunk never gets called in MultipartSaveLoad, so that's the problem. If you want a TileEntityloadEvent it's pretty much readFromNBT.

@octylFractal
Copy link

Patch complete, #272.

@Licshee
Copy link
Author

Licshee commented Apr 19, 2015

... so... is this fixed or not?

@Chicken-Bones
Copy link
Owner

I don't believe it is. I'm wary of compatibility issues with the suggested method. In 1.8 I'll just be adding the hook that was rejected from forge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants