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

Added SchematicEvent(WritePre, Write, WritePost). Added "Mapping" tag to the correct alias IDs in forge mods that load schematics. #325

Closed
wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 25, 2015

Added SchematicEvent(WritePre, Write, WritePost), necessary for the main feature.
Added "Mapping" tag to schematics saved using the Forge Adapter. This allows for mapping numerical block IDs to the correct alias IDs in forge mods that load schematics.

For example, if a schematic contains gold blocks, a new CompoundTag will be added to the "ForgeIdMap" ListTag containing the block id(41), and the alias id used in Forge mods(minecraft:gold_block). This has been tested with vanilla and mod blocks, and is functional. It will not add duplicate ID mappings or ID mappings that are not used in the schematic.

This was done using a new event I added called "SchematicEvent". There are 3 sub events; WritePre, Write, and WritePost. The mappings use SchematicEvent.Write to inject the mapping values into the schematic.

Sorry for basically creating a duplicate PR, but the previous one got screwed up.

schematics saved using the Forge Adapter. This allows for mapping block
IDs to the correct alias IDs in forge mods that load schematics.
@Ivorforce
Copy link

I approve - the 'Mapping' tag is a standard that both Schematica and Recurrent Complex honour in schematic files now, too.

@wizjany
Copy link
Collaborator

wizjany commented Mar 25, 2015

It's not a standard in mcedit though: mcedit/mcedit2@5477bd5
MCEdit-Unified doesn't handle it at all: https://github.com/Khroki/MCEdit-Unified/blob/master/pymclevel/schematic.py
I would like to note that there's already standard discrepancies with Add/AddBlocks, I would like to avoid having more of them.

What is the point of having the three events?
It would be nice if this could save mappings on bukkit platforms too.

* @param idMap - The id map that is being checked.
* @return Return true if the mapping exists in the id map.
*/
public boolean mappingExists(String alias, Map<String, Tag> idMap) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like a static method.

@wizjany
Copy link
Collaborator

wizjany commented Mar 25, 2015

Instead of doing events at all, wouldn't it be nice for the BlockRegistry to supply a mapping, which the SchematicWriter can serialize? It already has access to the WorldData, I see no reason to outsource making the mapping.

@Ivorforce
Copy link

It's not a standard in mcedit because the author refuses to change the format to consider dynamic block IDs, and has not shown any signs of updating the format in quite a while.

Rather than waiting for all schematic files to slowly deteriorate and become unusable, as will be the case from 1.8's dynamic IDs and forward, we'd prefer implementing our own system. It's completely backwards compatible, too.

@wizjany
Copy link
Collaborator

wizjany commented Mar 25, 2015

You make it sound like you've had a conversation with him. I'd love to see his PoV on it now.

edit: looking at his code, what do you mean by "dynamic IDs" and how does it not allow for them?
edit2: also what do you mean by "updating the format in quite a while"? He literally just added that 6 days ago, no?

@Ivorforce
Copy link

1: Int IDs are dynamically assigned - each world of MC can have a different mapping, or even entirely different blocks (mods, and later plugins - remember, even MC itself will just be a plugin some time, according to Mojang). Schematics currently count on the fact that they are static.

2: Good call, I was not aware of that. Literally 6 days ago, lol. The concept is pretty... odd (str(intID) => StringID), but let's wait it out - Maybe he'll be willing to change still.

@Lunatrius
Copy link

Going to wait out before I push the "Mapping" change (thank god I didn't schedule a build earlier today).

Waiting for feedback on that commit because it's a rather inefficient and unsafe way of saving the mapping. Flipping them around removes the need to cast/parse the numeric IDs.

@sk89q
Copy link
Member

sk89q commented May 29, 2015

The original way that I was thinking about doing was to do it in SchematicReader/Writer directly with a passed in WorldData object. Instances of those currently come from Worlds, and are already passed into ClipboardReaders.

The main thing left now is:

  1. Updating WorldData with API to get a mapping
  2. Updating the reader/writer to use this data

@me4502
Copy link
Member

me4502 commented Aug 18, 2018

This has been superseded by recent changes

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

Successfully merging this pull request may close these issues.

6 participants