MineColonies

MineColonies

53M Downloads

[BUG] Decoration controller doesn't always store the pack and path

MotionlessTrain opened this issue ยท 0 comments

commented

Is there an existing issue for this?

  • I have searched the existing issues

Are you using the latest MineColonies Verison?

  • I am running the latest alpha version of MineColonies for my Minecraft version.

Did you check on the Wiki? or ask on Discord?

  • I checked the MineColonies Wiki and made sure my issue is not covered there. Or I was sent from discord to open an issue here.

Minecraft Version

1.19

MineColonies Version

commit 2de0aab (corresponds to 1.0.1057)

Structurize Version

1.0.449

Related Mods and their Versions

No response

Current Behavior

Whenever a builder finishes building a decoration with a decoration controller, they don't properly update the decoration controller's pack and path. In some cases, this can lead to decorations not being upgradable, or decorations being upgradable to the wrong decoration (something that might happen with competition buildings, depending on whether they change upgradable decorations, and how they are doing that).

The reason is that upon completion, the builder makes a custom CompoundTag, containing the blueprintDataProvider tag with a pack and path:

tagData.putString(TAG_NAME, location);
tagData.putString(NbtTagConstants.TAG_PACK, blueprint.getPackName());
((IBlueprintDataProviderBE) te).readSchematicDataFromNBT(compoundNBT);

The decoration controller calls its parent to read in the schematic name, corners and tag list from this CompoundTag (ignoring the pack and path), and then proceeds to use the "name" and "pack" tags from the decoration controller itself (not the ones in the blueprintDataProvider tag).
IBlueprintDataProviderBE.super.readSchematicDataFromNBT(compound);
if (compound.contains(TAG_NAME))
{
this.schematicPath = compound.getString(TAG_NAME);
final String[] split = Utils.splitPath(this.schematicPath);
this.schematicName = split[split.length - 1].replace(".blueprint", "");
}
if (compound.contains(TAG_PACK))
{
this.packName = compound.getString(TAG_PACK);
}
else

Those don't get updated anywhere, but rather have some random names. When a decoration controller got placed in the world and scanned in, those will be empty. When they have been pasted in, those contain the pack and the name from that decoration, which means that if the decoration is changed and saved under a different name (possibly in a different pack), instead of the path of that decoration, the path of the original decoration will be used in the gui.

Note that constructed paste works without issues. Instead of calling TileEntityDecorationController#readSchematicDataFromNBT, the setBlueprintPath and setPackName methods get called, which update those fields correctly

Expected Behavior

I expect those fields to get set correctly, such that those decorations are properly upgradable.
Either the builder uses the same logic as pasting does, and uses the setPackName and setBlueprintPath calls there (either directly or in IBlueprintDataProviderBE#readSchematicDataFromNBT), and handles the logic there (which is partly done already), or it keeps the same logic, but doesn't use the "name" and "pack" entries of the NBT tag there, but the ones in BlueprintDataProvider, since those are the ones that have been updated.

Reproduction Steps

  1. Place a few blocks (e.g. iron blocks in my own tests) and stick a decoration controller to it somewhere.

  2. Build next to it the same structure but with different blocks (diamond blocks in my test) and stick a decoration controller on the same place on the structure.

  3. Scan the first one with any name (as long as it ends with a "1"), e.g. myschematic1

  4. Scan the second one with the same name, but level 2, e.g. myschematic2

  5. Confirm that both are showing up in the build tool together as upgradable decoration, and assign the level 1 version to the builder

  6. After the builder is done building, look at the decoration controller. The window is completely empty (no name, no buttons). /data get block <x> <y> <z> shows that most if not all fields of the nbt of the decoration controller are empty

  7. Paste both schematics in (either constructed or schematic could work). The decoration controller should now work correctly, and the data get block command shows that the nbt is filled without issue.

  8. Now modify both a bit (enough to make them visibly different from the original ones. I replaced one of the iron blocks at level 1 with diamond, and the diamond blocks with emerald blocks at level 2).

  9. Scan both decorations again with different names (e.g. mytestschematic1 and mytestschematic2)

  10. Those should show up as upgradable decoration in the build tool again. Select this one to be placed by the builder.

  11. The builder should have built mytestschematic1 correctly. The decoration controller now shows the name "myschematic1.blueprint", and the resource list on the "repair" and "upgrade" buttons refer to the needed items of the original schematics from steps 1 and 2, rather than the ones from steps 8 and 9. /data get block shows it still has the pack and name information of the pasted in schematics, not from mystestschematic1.

Logs

https://gist.github.com/d5f0ed4d8b72b1606ef6ca9eb209e5f7

Anything else?

I think the second way of how I think to solve it would be better. Changing the logic of the builder to remove the call to readSchematicDataFromNBT would probably give problems with buidings, and even changing IBlueprintDataProviderBE#readSchematicDataFromNBT to give pack and path information could give problems with older decorations, which don't have a pack property yet (it is not detectable anymore whether the name referred to an old style path). Then again, I'm not sure if that is necessary, since the path is being set at that point anyway

  • Add a thumbs-up to the bug report if you are also affected. This helps the bug report become more visible to the team and doesn't clutter the comments.
  • Add a comment if you have any insights or background information that isn't already part of the conversation.