Fabric API

Fabric API

106M Downloads

Structure Module

frqnny opened this issue ยท 10 comments

commented

Somewhere between 20w20a & 20w22a, structures are now likely hardcoded in many places.

Here is a field in class 5311
field_24822 = ImmutableMap.builder().put(StructureFeature.VILLAGE, new class_5314(32, 8, 10387312)).put(StructureFeature.DESERT_PYRAMID, new class_5314(32, 8, 14357617)).put(StructureFeature.IGLOO, new class_5314(32, 8, 14357618)).put(StructureFeature.JUNGLE_PYRAMID, new class_5314(32, 8, 14357619)).put(StructureFeature.field_24851, new class_5314(32, 8, 14357620)).put(StructureFeature.PILLAGER_OUTPOST, new class_5314(32, 8, 165745296)).put(StructureFeature.STRONGHOLD, new class_5314(1, 0, 0)).put(StructureFeature.MONUMENT, new class_5314(32, 5, 10387313)).put(StructureFeature.END_CITY, new class_5314(20, 11, 10387313)).put(StructureFeature.MANSION, new class_5314(80, 20, 10387319)).put(StructureFeature.BURIED_TREASURE, new class_5314(1, 0, 0)).put(StructureFeature.MINESHAFT, new class_5314(1, 0, 0)).put(StructureFeature.RUINED_PORTAL, new class_5314(40, 15, 34222645)).put(StructureFeature.SHIPWRECK, new class_5314(24, 4, 165745295)).put(StructureFeature.OCEAN_RUIN, new class_5314(20, 8, 14357621)).put(StructureFeature.BASTION_REMNANT, new class_5314(30, 4, 30084232)).put(StructureFeature.FORTRESS, new class_5314(30, 4, 30084232)).put(StructureFeature.NETHER_FOSSIL, new class_5314(2, 1, 14357921)).build();

I would love for someone to see what this mess is, map these classes, and create a module since class 5311 will throw an exception if your structure is not in that ImmutableMap.

commented

It looks like class_5311, has a default list of structure spacing, an instance list of structure spacing, and a CODEC generated at class load time, containing structures, the CODEC seems to be the problem, you need to add your structure to the registry before class_5311 loads, or else it won't be in the CODEC, this might need a mixin or access widener.

commented

Okay so I got a working implementation of a jigsaw structure feature on 1.16-pre4 and here are the things that I figured out along the way

  1. The way Minecraft's default serialization of StructureStarts is really gross and relies upon it's own hash map and an odd logic system meaning all custom structure features need to be added to a specific hashmap in order to be serialized and deserialized properly
    1. To serialize it always writes the key for the StructureStart as StructureFeature.getName() 1 which through inheritance should return STRUCTURES.inverse().get(this)2 aka the name that the structure was added to the hashmap3 in StructureFeature under
    2. To deserialize however it reads each key from the serializer and attempts to find the corresponding value for the key inside of the Hashmap in StructureFeature3 by taking the string and making it lowercase4
    3. Conclusion: under normal circumstances where the child class (extending StructureFeature) doesn't override StructureFeature.getName() (unlike how is currently recommended on the wiki5) everything works fine, but this system is extremely fragile and the instant the getName method is overridden everything breaks
    4. It appears that while minecraft doesn't do this, it is possible to add the StructureFeature onto the hashmap in StructureFeature3 using a string formatted as an identifier and everything works just fine meaning so long as mods add their structure to the hashmap using a full identifier there should be no conflicts (or, as long as the updated api automatically adds structure features to the hashmap using their full identifiers there should never be a conflict between mods)
    5. In addition, not having the StructureFeature added to the hashmap in StructureFeature3 spams the log with severe errors indicating the structure cannot be properly serialized so it should definitely be a task for the updated api to add structures features to the hashmap in StructureFeature3
    6. Another fun feature is that by adding the StructureFeature to this hashmap3 it automatically makes it included in the /locate command so any mixins modifying the locate command would be obsolete (Closing #123)
  2. Minecraft has in addition to the registry for structure features, not just one hashmap, but two hashmaps that the structure feature has to be added to in order to properly generate
    1. The first was mentioned in the point above
    2. The second hashmap is used to determine the generation step in which the StructureFeature will generate6 and is unfortunately private
      • The following access widener was used in testing to make the field public so items could be appended to it accessible field net/minecraft/world/gen/feature/StructureFeature STRUCTURE_TO_GENERATION_STEP Ljava/util/Map;
    3. If the StructureFeature is not added to this hashmap6 then it will not generate in the game whatsoever but the empty structure bounds will still be generated meaning the location the structure should've been generated at can be found using /locate
  3. Maybe we want to change the Yarn mappings now that Structures are severed from Features and share none of the same code to something like StructureFeature -> Structure just an extra thought I had

A working implementation of StructureFeatures in 1.16-pre4 can be found here

Hope all of this helps in some way, I've been beating my head against a wall for the past like 6 hours trying to figure out what all of this new code does.

I don't actually know what the stuff in net.minecraft.world.gen.chunk.StructureConfig (the class formerly known as class_5311) does, but not dealing with it hasn't created any errors and the structure was generating perfectly fine, so I don't know what it is used for, but it doesn't seem necessary for us to deal with it right now, as it doesn't seem to be causing any problems.


Footnotes

[1]: See line 442 of net.minecraft.world.ChunkSerializer under method writeStructures()
[2]: See line 229 of net.minecraft.world.gen.feature.StructureFeature under method getName()
[3]: See line 39 of net.minecraft.world.gen.feature.StructureFeature at static field STRUCTURES
[4]: See line 456 of net.minecraft.world.ChunkSerializer under method readStructureStarts()
[5]: See https://fabricmc.net/wiki/tutorial:jigsaw
[6]: See line 40 of net.minecraft.world.gen.feature.StructureFeature at static field STRUCTURE_TO_GENERATION_STEP

commented

StructuresConfig is so they don't all spawn right next to each other.

commented

Yeah I feel like we've been over that, I mean on a technical level

commented

The class contains a default list of structure spacing, an instance list of structure spacing, and a codec. A structure spacing consists of spacing, separation, and salt (I don't know what that mean). When StructuresConfig is instantiated it will contain either the default list, or a custom specified list. That is all I know about StructuresConfig.

commented

The mapping is supposed to be in yarn's domain. However the fact the map is immutable is something we can cover

commented

I cannot guarantee this is all that might break structures.

commented

class 5299 extending DataFix (?)
also has a similar field
private static final ImmutableMap<String, class_5299.class_5300> field_24647 = ImmutableMap.builder().put("minecraft:village", new class_5299.class_5300(32, 8, 10387312)).put("minecraft:desert_pyramid", new class_5299.class_5300(32, 8, 14357617)).put("minecraft:igloo", new class_5299.class_5300(32, 8, 14357618)).put("minecraft:jungle_pyramid", new class_5299.class_5300(32, 8, 14357619)).put("minecraft:swamp_hut", new class_5299.class_5300(32, 8, 14357620)).put("minecraft:pillager_outpost", new class_5299.class_5300(32, 8, 165745296)).put("minecraft:monument", new class_5299.class_5300(32, 5, 10387313)).put("minecraft:endcity", new class_5299.class_5300(20, 11, 10387313)).put("minecraft:mansion", new class_5299.class_5300(80, 20, 10387319)).build();

commented

Don't think we need to cover the datafix one, but the actual used one needs to be covered.

commented

I believe that this issue has been addressed with the merge of #917.