Podshot / MCEdit-Unified

Combined MCEdit & Pymclevel repository.
http://podshot.github.io/MCEdit-Unified/
ISC License
483 stars 109 forks source link

Running from source under Windows and Linux giving different results #820

Open neonerz opened 7 years ago

neonerz commented 7 years ago

This is an odd one we can't figure out. The following is a world file (PE) and a schematic taken from a Java world.

brewing-stand.zip TERRACOTTA-ISSUE--_2017-07-01--15-01-18.schematic.txt (remove .txt extension)

Using the following blocks.json and terrain-pocket.png https://github.com/PathwayStudios/blocks.json https://github.com/PathwayStudios/terrain-pocket.png

If I open the world file in Windows and import the schematic into the attached world, it works as expected (the block IDs get converted from Java to PE IDs as defined in blocks.json, though does exhibit the data value bug we reported in issue #800 ). Visual Result: python_2017-07-01_13-45-53

If I open the world file in Linux and import the schematic into the attached world, the block IDs don't get converted and I end up with a bunch of "Future Blocks" Visual Result: vmplayer_2017-07-01_15-18-00

Both Windows and Linux are running from source using commit: b735277. I confirmed the same thing as of the latest commit (c847e91) as well, but since there seems to be some issues with that commit, I thought it was best using the older commit as the example.

ghost commented 7 years ago

Can i have exact instructions for running source

neonerz commented 7 years ago

@NFGamerMC if you check the readme, it has detailed instructions on how to run from source using Windows, Mac, and Linux. Keep in mind, windows support doesn't really work since when you save the world, the seed, name, world type, and game mode gets reset.

LaChal commented 7 years ago

@neonerz lokks like you don't have same definition files and/or same texture files on both systems.

neonerz commented 7 years ago

OK, doing more testing, it's not a windows/linux thing, it just looked like it due to how you need to generate the bug.

Here's the two worlds I've been testing with

Java World: Bug_World_2.zip PE World: concrete.zip Schematic: Bug World_2017-07-03--17-37-23.schematic.txt Updated blocks.json: https://github.com/PathwayStudios/blocks.json Updated terrain-pocket.png: https://github.com/PathwayStudios/terrain-pocket.png

Steps to recreate the bug:

  1. Open Java world (any java world will do, but you could use the one I provided to do the exact same steps as I am).
  2. Without closing MCEdit, open PE world (any PE world will do, but like above, you could use mine to recreate my exact test)
  3. Import the provided schematic (the schematic includes 6 concrete blocks DVs 0-5)

The schematic gets pasted in correctly (asside from the issue I mentioned in #800 turning DV 1 into 0)

Now do the same thing, but don't open the Java world first.

  1. Open PE world (any PE world will do, but like above, you could use mine to recreate my exact test)
  2. Import the provided schematic (the schematic includes 6 concrete blocks DVs 0-5)

The blocks don't get translated as they are defined in blocks.json. The are keeping the Java item ID of 251 (instead of PE Item ID236 like it does if you open a Java map first).

Basically, if you open a Java map up first (after initially opening MCEdit), then open a PE map (without closing MCEdit), it works. But if you open a PE map first (after initially opening MCEdit), it doesn't work.

neonerz commented 7 years ago

I have more info about the bug. The issue can be seen in the logs. When loading up a PE world first, and trying to import the schematic, MCEdit doesn't seem to load all the blocks.json definitions.

See the following two mcedit.logs (renamed for your convenience) mcedit_PE_load_first.log.txt mcedit_Java_load_first_then_PE.log.txt

Starting on line 7180 in mcedit_Java_load_first_then_PE.log and line 3220 in mcedit_PE_load_first.log are where they are loaded.

In both instances, the loading seems to stop about most of the way through the definitions, but the version that was first opened with Java starts again and completes the whole file, while the one that was only ever opened in PE stops there.

Going by the missing definitions in the mcedit_PE_load_first.log, it seems to be: Observer, Terracotta, Concrete, and Concrete powder that is effected.

Here's a diff displaying the differences: https://www.diffchecker.com/UxzxHlBu Start from line 3370 in the left window. You could see both end at Frosted Ice, but then the one where a Java world was loaded up first (right window) restarts and loads all of the definitions.

LaChal commented 7 years ago

I don't know how and when you're calling convertBlocks or convertFunc methods, nor which data you send them. So it's a bit confusing for me to know what happens exactly here...

The diff you provided shows almost 100% differences. Lines 19 (in both views) are different (quite casual, when instanced, objects get unique IDs). But the next lines, from 20 to 56 included, are the exact same, but are reported as different by the viewer. The lines 57 to 155 included are also the same, except the object IDs. I looked to the line 3370 you pointed, but considering above information, it's not really relevant, badly. I think the tool you used used to create this diff view is a low level one. Try something like WinDiff. It can point differences within lines, and can find 'moved' lines.

Schematic format does not store the game version it is created from. So, it is not possible to guess block ID 1234 shall be converted to 4321 when importing.

The versionned data handling is basically implemented for now. That means a lot of MCEdit internal stuff uses the MCEDIT_DEFS and MCEDIT_IDS objects. These objects are re-created when a world is loaded. Each 'level' object stores the needed MCEDIT_DEFS and MCEDIT_IDS they need in the defsIds member. It is a MCEditDefsIds instance (look to id_definitions.py in pymclevel). According to schematics do not store any version information and materials and blocks/entities/tileentities data which are attached at runtime, automatic conversion is not possible, unless a version tag is implemented in this format. Also, each 'level' handles its own materials. It's the material member. Changes about switching from generic MCEDIT_DEFS and MCEDIT_IDS to 'level' specific ones are on the way.

neonerz commented 7 years ago

I don't think you are following what I'm saying. This isn't through my filter or anything like that, this is just plain mcedit from source. You should be able to easily reproduce it yourself.

It seems mcedit acts differently if you load a Java map first, or if you load a PE map first.

When you load a Java map, it seems to import blocks.json twice (as you've mentioned in the past, this is expected behavior) and loads all the definitions.

If you load a PE map first it only seems to import blocks.json once, and all the definitions don't get loaded.

Referring back to the diff I posted (https://www.diffchecker.com/UxzxHlBu)

The one on the left is when I load a PE world first, and the one on the right is if I load a Java world first , then without closing MCEdit load up a PE map (that's why they don't match up line for line).

If you scroll down to line 3220 on the left window, you'll see where the definitions get loaded. Lines 3211 to 3370 are the initial loading of the definitions, but as you could see it stops at "Frosted Ice". Now keeping at those lines, on the right window, you'll see the definitions also stopped at "Frosted Ice" but then the definitions get loaded again (the green section starting from line 7332 in the right window) and then at line 7478 you could see it gets to "Frosted Ice" a second time, but doesn't stop there and loads all the definitions.

I know this is a bit confusing to explain, but it's easy to reproduce if you follow the steps I wrote in this comment

If you'd like to chat live, I could demonstrate the issue to you. But like I said, it's very easy to reproduce.

LaChal commented 7 years ago

I can see what happens here (since I finally found the way to port the diff stuff to another tool, it's more clear; anyway, thanks for the research :smile:).

The fact is shcematic format do not store the game version it comes from. So, when importing a schematic, there is no way to know which game version data to use. Regardless MCEdit internal conversion functions.

It is technically possible to add any NBT tag in a schematic, but MCEdit does not "own" this format. It is also used by game mods, and potentially by other programs. Modifying this format on one side may lead to breakage on other ones...

Need to do some exploring on this subject...

Edit: I followed you steps on a not yet published version of the code (need sanitization an more tests), and saw same things, AFAIK. These changes will rely on a finer definitions files loading and handling.

neonerz commented 7 years ago

I'm happy you were able to understand. It was a bit confusing what I was trying to explain, and I wasn't sure if I was clear.

Here's a wild idea, what if when exporting a schematic, instead of just having two options (.nbt and .schematic) you had three? Basically the same as .schematic but something along the lines of .schematicpe. That way when that schematic is exported mcedit could know if it was saved from a java world or a PE world. It probably isn't the best route, and could cause PEBKAC errors, but it's an idea.

Also a long shot, but could the #800 issue be related to this?

LaChal commented 7 years ago

Your idea is not bad, but I think it's quite the same as storing game version in the schematic itself. But it can be a good start.

Is relating this to #800 a rhetoric question?

BrandonKaim commented 7 years ago

@LaChal @neonerz Are you sure it is related to how it stores the blocks? I was able to convert completely different blocks to other completely different blocks, A.K.A Borders to Invisible Bedrock / Glass or Corse Dirt to other blocks such as stained clay or just regular dirt. And @neonerz, didn't you say converting from PE to PC worked just fine though? Unless I completely misunderstood what you guys said.

ghost commented 7 years ago

I keep crashing when saving PE worlds

Sent from my iPad

On Jul 17, 2017, at 5:42 PM, LaChal notifications@github.com wrote:

I can see what happens here (since I finally found the way to port the diff stuff to another tool, it's more clear; anyway, thanks for the research 😄).

The fact is shcematic format do not store the game version it comes from. So, when importing a schematic, there is no way to know which game version data to use. Regardless MCEdit internal conversion functions.

It is technically possible to add any NBT tag in a schematic, but MCEdit does not "own" this format. It is also used by game mods, and potentially by other programs. Modifying this format on one side may lead to breakage on other ones...

Need to do some exploring on this subject...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

BrandonKaim commented 7 years ago

@NFGamerMC Are you using the latest source version?

LaChal commented 7 years ago

AFAIK, converting blocks from a game version to another may work depending on which blocks are converted... Some PE blocks aren't yet defined in our sources, and other ones may not have the right data vlue (or other internal data) defined correctly. Also, as @neonerz pointed, depending on how you load worlds in MCEdit, the conversion can work or not.

BrandonKaim commented 7 years ago

I know some blocks aren't defined and some aren't defined properly, but when they are (made sure they matched up with the PE I'd dump list) it does this issue along with #800. It's weird, MCEdit should theoretically recognize all the blocks and convert them fine since your giving a definition of what blocks should convert to what, including the data variables.

LaChal commented 7 years ago

Schematics do not include game version. So, the definition of the blocks/entities for the game version of the schematic can not be loaded, nor retrieved in MCEdit definitions handler...

BrandonKaim commented 7 years ago

I know schematics don't include the game version, but the definitions I was talking about were on MCEdit's side, since those definitions translate what blocks are which to the desired game version

LaChal commented 7 years ago

Once again, schematics do not contain game version, so MCEdit can't load the right data for schematics. It's allright if you import a PC schematic in a PC world, or a PE schematic in a PE world, but, as you remarked, there are issues when importing PE into PC and vice versa.

Transferring stuff form PE to PC and vice versa is not a feature of MCEdit.

BrandonKaim commented 7 years ago

Ok, thanks for clearing it up 😃