For some reason, whenever I use terraforge, strongholds will no longer appear in the overworld, is it possible to disable strongholds or to be able to fix this? Strongholds DO spawn in the nether.

damn that actually sucks, I'm so sorry you have to deal with all that crap. Also, that's a big yikes in their attitude.

Thank you for explaining to me though and taking the time to reply, if you need any help (I code in python and java), just lmk :)

Note: Be aware you're only hearing one side of the story there (the conversation is on our discord if you want to make your mind up for yourself). We were frank, yes, but didn't offer anything but advice to someone who didn't want to accept it (on the grounds that they'd already told a bunch of people to do it some other, incompatible, way)

Regardless, for the record, the correct solution is to register the structure separation settings at any point before the chunk generator is created. Requires zero hacks. Can use FMLoadComplete maybe + other forge events for updating them:

// Note: Do thread-safely!
private static void registerStructure(RegistryKey<DimensionSettings> dimension, Structure<?> structure, StructureSeparationSettings settings) {
    WorldGenRegistries.NOISE_SETTINGS.getOptionalValue(dimension).ifPresent(dim -> dim.getStructures().func_236195_a_().put(structure, settings));
}

Tele, maybe you could add a check in Blame to scan biomes to make sure all structures that were added to biomes have default separation settings correctly registered for them 👍

Won-ton, other people saw the discord discussion and they too, felt your tone was quite rude. Second, I already stated that this is only forcing other mods to conform to Terraforged due to a bad assumption that final remains final. And to make Blame report that is kinda ridiculous when the only mod that has an issue is Terraforged. The hack currently used handles all use cases as far as I know and to even add it to the StructureTutorialMod would seem redundant to modders and the only comment I can really add is "this is needed for Terraforged" which will make your mod sound bad so I won't do that.

Tbh, your chunkgenerator code doesn't really make sense either. Reducing code duplication is often praised so it very odd that you made a duplicate field, overrode getStructureConfig, and use that instead of the inherited class's method. Especially since your class didn't change that field at all so it makes no sense to duplicate it with extra code.

And even on a user-focused side, making other mods have to incorporate the proposed code above to be compatible with Terraforged is only going to draw out the whole situation for months as there will constantly be mods that need it whereas, Terraforged can just call a single method and end this whole incompatibility entirely.

There's nothing more to be said here. Either use the better modding practice or go make that Structure API PR and contact every structure mod to go use it.

There's a world of difference between professional styled coding practices and what is best modding practices. Some cases, they overlap. Some cases like here, they do not. Private, Final, Immutable rarely means that at all when modding Minecraft. It's just the nature of modding.

I also just realized, the proposed fix of looping over the noise setting registry would break my dimension blacklisting as well. The hack has the noise setting tied to dimensions which works great. But if I use the noise settings registry loop, I cannot do dimension blacklisting anymore which is quite crucial to my mod. If I do both the proposed fix and the hack, and someone blacklists the overworld dimension, /locate will be broken again as terraforged is using the noise setting from the registry while the hack made the new map for that dimension.

It's just not a proper solution for my use case.

Good modding practices:

1. Register your stuff. It makes your content visible to other mods - case in point, if your structures were registered users would be able to configure their geneartion settings in TF's world creation screen without us needing to know a thing about what you add. It's really nice.
2. If you need hacks, design robust ones that don't rely on specific vanilla implementation.

Sorry to hear that that's your stance.
We tried :(

The maps are plain ol' hashmaps btw. You can add/remove from them as you like as long as it's thread-safe.

It's an immutable map when done through json worldgen

Yeah, so you add/remove before everything is serialized (that happens each time you create a new world etc using the WorldGenRegistries as the starting point)

Fellas, this is extremely entertaining. I have no clue who's in the right here but I'm rooting for Grunt. He seems nice!

WorldGenRegistries is not the same as the dynamic registry. Json worldgen only exists in the dynamic registry. Any changes in WorldGenRegistries will be overriden by the json worldgen files and set in stone for the dynamic registry.

meaning, mods and datapacks with json noise settings files will make the immutable map immediately for their chunkgenerator creation upon the creation of the dynamic registry

Yeah you can still use your injection thing for datapack compat if you really want, I explained on discord that datapacks are intended to be static configurations so it's a bit of a messy workaround. There are better (albeit more involved) solutions for that

I just recently added this to the curseforge page so yeah, not everyone has seen this yet.

The structures do spawn in the world. Just /locate, treasure maps, and eyes of enders won't work due to an issue on Terraforged's end that they won't fix. Until they fix it and make /locate work for my mod and like a hundred other structure mods, the locate command won't work for their worlds.

For the stronghold, you can turn off RS's stronghold in its config so that the vanilla stronghold spawns again. Then you can use Eyes of Ender to find that stronghold.

oh okay, I contacted terraforge via github and they said this:

"Hi. Repurposed structures replaces the ender eye behaviour. The settings for their structures aren't registered properly for terraforged to use them so they either don't generate or can't be located (which is probably why the item isn't consumed).

We gave them a ~5 line fix for it already but they didn't seem receptive to using it. You could alternatively do the datapack thing I suggested in #38 . It's a bit laborious unfortunately (+ not 100% sure if it'll work for it either)"

so that's why I opened the issue but if it's not from your end, then I'm sorry for possibly wasting your time. Thank you for fast response though :)

The issue is actually a bit more in depth than that. They did, what I considered to be, bad modding practice of assuming a final field would remain final when people use ATs and Accessor Mixins all the time. That was their mistake. Modding is not a corporate codebase held up to standards. It's a realm of hacks and workarounds.

They basically made their own duplicate field and use that field instead of the inherited class's field instead. Good modding practice would be to use the inherited class's stuff as much as possible to maintain mod compat with other mod's hacks and coremods.

All Terraforged has to do is change a single line of code to make their mod compatible with most other structure mods. However, they took a "No U! Hacks bad!" approach and now wants everyone else to add code to make their mod compatible with Terraforged. Basically putting the work on everyone else instead of owning up to their flawed assumptions that would often be proved wrong in a modding environment.

Their solution isn't even a solution either as it would make my structures no longer spawn in worldgen datapack or mods that uses json noise settings files so I won't be removing my hack that lots of other structure mods also use. No other mod has a conflict with this hack. Only Terraforged. So even if I try their solution as a conjunction with my hack, that's only one less mod that is incompatible with Terraforged when there's still a massive amount of other mods structure locate broken due to Terraforged's assumption.

Hence why this is on Terraforged's end. I already spoke to them in the discord and honestly, I definitely got an attitude of disrespect tbh. Especially when I was told to go make a full-fledged structure API, bicker with Forge to get it merged, and then convince everyone to use that API to make our mods compatible with Terraforged, all so Terraforged doesn't have to change a single line of code. Yeah I'm still miffed about that as you can see lol. It's clear how little they value my time and refuse to work within the confines of modded Minecraft instead of taking best practices of never assuming something final or immutable would remain final or immutable. I made that assumption myself a few times and it always backfired fast with mod incompatibility lol. I guess they haven't learned that lesson yet.

Sorry, didn't mean to bring up drama like that but the point is, Terraforged just needs to change this.settings to super.getStructureConfig() to work with most other structure mods. Their mod is the odd one out so it's really on them to accommodate everyone else instead of forcing everyone to conform to Terraforged's wishes. Or Terraforged could go make that Structure API PR themselves to Forge. I don't have the time or energy to do that much work just to be compat with one mod and I suspect I wouldn't even make the API compat anyway as I do not know of any other way to be compatible with worldgen datapack without using the hack I use.

Tough situation all around yeah.

@Won-Ton it seems like this.structureGenerator is not using the supplier DimensionStructuresSettings and is still a different object as well. Once that one uses the same DimensionStructuresSettings object of everywhere else, the hack will work for terraforged and any other weird side-effect of the different DimensionStructuresSettings objects should be gone. Hopefully.

In StructureGenerator, change
this.structuresSettings = settings.structures.apply(((DimensionSettings)generator.getSettings().get()).getStructuresConfig());
to
this.structuresSettings = generator.getStructuresConfig()
or something along those lines.

That's intentional. We give the user ui controls to configure the structure separation settings for everything that's registered and apply their preferences on top of the settings provided from the Supplier<DimensionSettings>. We only retain that supplier for proper serialization/deserialization behaviour in our ChunkGenerator codec (in the same way that NoiseChunkGenerator does, except that also uses it for Lifecycle validation it would seem).

So you're saying, if someone uses getStructuresConfig() on the world's chunkgenerator to check the structure spacings for their own calculations, it would be the wrong DimensionStructuresSettings with different values than the one that Terraforged actually uses for structure generation and UI config?

@Won-Ton I would like to make an apology. I was wrong about the initial cause about the bug. I was doing a deeper look into it as one part bothered me as it felt like my hack shouldn't had conflicted in the first place. Instead, the issue is not caused by having multiple final fields referencing the same object. The issue is Terraforged has a bug that make the final fields not the same object anymore when re-entering a Terraforged world. My hack only changes the map in the structures field (field_236193_d_) in DimensionStructuresSettings. The fields Terraforged stores and references around is DimensionStructuresSettings so if that object were to be the same as the one passed in the super call in TFChunkGenerator's constructor, my hack would work perfectly fine. DimensionStructuresSettings itself is not replaced by my hack.

Better view in what is going on.

To confirm my suspicions, I commented out my hack, placed a breakpoint in the constructor of TFChunkGenerator, and looked at what DimensionStructuresSettings the super ChunkGenerator is getting vs what is being stored in TFChunkGenerator. And yep, they are not the same object.

In addition, without my hack in place either, the two are still different because the biome source has modded structures in it that the DimensionStructuresSettings parameter in the TFChunkGenerator constructor doesn't have. There's a mismatch between the state of the DimensionStructuresSettings in the super and inherited class as a result which shouldn't be.

Instead, I think what is intended, and what vanilla does, is pass the DimensionStructuresSettings parameter directly into the super call so that the DimensionStructuresSettings is not mismatched. By making a new DimensionStructuresSettings from scratch, this breaks the assumption that DimensionStructuresSettings is final for everyone and it seems, this is going to cause unintended behavior for your mod too even without my hack in place. Especially as the base chunkgenerator's DimensionStructuresSettings is being used for setStructureStarts instead of TFChunkGenerator's. I don't know all the side effects of this are but it is so well hidden that it took this long to realize the objects were never the same from the start and had nothing to do with my hacks.

That isn't the case on our latest release builds. It got corrected naturally whilst fixing a separate issue we had supporting datapack DimSettings which we now can do in conjuction with WorldGenRegistered settings

Guys calm down, anyway this is for a game, do you want to start a war for stupidity? They are adult people I imagine, I hope the best of you, set an example and be partners, help each other, there is no need to insult each other, it is good to argue, but without disrespect, I love the work of both and it is sad to see this please apologize and forget what happened, it was really a nonsense right ?. Regards!

We protect access to the settings because we don't want mods modifying them. It's our prerogative and responsibility as the chunk generator to ensure that the user's settings are respected. It causes unexpected behaviour for them (that we have to deal with) when third parties start silently tampering with them (I'm not suggesting your mod specifically is guilty of "tampering", but others certainly could be doing bad things using the same technique).

We can't protect ourselves entirely (as you pointed out, mods changing the settings in ChunkGenerator will break stronghold-locating and possibly other stuff), but we can make sure regular structures actually generate according to what our user specified.

As I've covered already, the proper way for mods to get their structure separation settings into the chunk generator is by registering or by datapack. The datapack method is inherently un-friendly to other mods (it's the root cause of you having to use the method you are afterall) so we encourage registering instead.

We're ok with TF not working properly with mods/datapack that don't want to do things in a friendly way. It's commendable that you're happy to spend time/effort supporting them with these sorts of workarounds. You could use the friendly approach of registering your stuff in addition to your workarounds for datapacks but I totally undserstand if you choose not to if you feel like we're being uncooperative or something.

We're just discussing technicals @FrannDzs - it might read a little dry but there's no malace or ill-will as far as I'm aware 🙂

@TelepathicGrunt @Won-Ton great guys, hope you come to a solution soon, cheers!

Doing both the hack and registering ahead of time will cause confusion for users as now they won't know which mod's configs will take effect over the other's. I rather have my configs be in charge so people can see there are also biome configs, placement configs, etc alongside the spacing configs.

Furthermore, this will break RS's dimension blacklisting as the registry is not tied to any world. If one blacklists the overworld, my suspicion is that /locate will now give false positives in the world with Terraforged on. And once again, even if we somehow solve those two issues, this only resolves the incompat between my mod and yours but not all the other structure mods. I get what you mean by trying to lock it down and prevent bad actors but this is causing more issues than it solves imo. There's most likely a better solution to this than the current lockdown that only partially works and breaks other mods.

Honestly, I think it would be better to not lock it down and just see if there even are any bad actors in the first place as I don't think it's as widespread as you imply and most are just people making their own structures spawn.

Edit: the hack I use, like I said, is powerful as it allows compat with datapacks and dimension based checks. If I add the registry loop as well to the StructureTutorialMod, then I would have to tell people as to why it's there as it would seem redundant. So far, the only mod that needs it in the registry is Terraforged. And the worldLoad.event hack would need a way to know when to skip Terraforged's chunkgenerator without a hard dep on terraforged. That would solve the dimension blacklist issue with just terraforged's world

Edit2: I'm having trouble telling when a chunk generator is TFChunkGenerator without using a dep and without getting class names itself as you might change the name later on.

As the chunk-generator TF is able to store the user settings for structure separation per world because we get to dictate that in our codec. It'd be nice if vanilla did the same so that your settings are persisted but it doesn't store a full copy of the DimSettings, just a registry key. The only thing similar that vanilla supports is per world datapacks, dunno if that's an avenue to look at.

Re: the hard dependency thing - we register our chunk generator codec so you could inspect the registry name with something like:

// func_230347_a_() has protected access so you might need Reflection/MethodHandle/Mixin to access
ResourceLocation name = Registry.CHUNK_GENERATOR_CODEC.getKey(generator.func_230347_a_());

edit: I'll politely bow out now, I don't have much more input to give without repeating stuff which wouldn't be productive. You'll need to find workarounds for your workarounds if you don't want to use the conventional way. Sorry I couldn't be of more help

The settings passed into our parent ChunkGenerator have the same preferences applied as the ones we use for generation.

Yes you're right. Tested that. But then that raises even more questions as to why it isn't using the same object if they are suppose to be the same. You mentioned it being different objects for UI controls but looking at this code, it seems like both the chunkgenerator's and StructureGenerator's DimensionStructuresSettings are being used for worldgen in different spots so that doesn't seem right. And since StructureGenerator is created inside TFChunkGenerator's constructor, it doesn't make much sense to make the new DimensionStructuresSettings object when there is a valid one right above it.

I went ahead and forked Terraforged, did the .getStructuresConfig() change I suggested in #52 (comment)
and everything is working. The structure configs works perfectly fine. I was able to set the config of buried treasure and desert pyramids to 1000 and they took effect in the world. Re-entering the worlds kept the same spacing. Making new worlds as well worked fine. There's no issue that I can find. I can PR this change to the Terraforged github if you wish.

v2.4.0 is now released with workaround

Genius!

v2.4.0 is now released with workaround

@Won-Ton I may still not agree with the idea of wrecking mod compat over purely optional configs but it turns out, Terraforged does have access to the majority of structure mod’s spacing. Even those that uses the hack.

Part of my structure tutorial mod has people add their structure and spacing to DimensionStructuresSettings.field_236191_b_. In yarn, it is called something like DEFAULT_STRUCTURES so FabricAPI adds all structures to it. I recommended to do the same in the tutorial and people followed that part especially.

All Terraforged has to do is take DimensionStructuresSettings.field_236191_b_’s entries after FMLCommonSetupEvent’s enqueueWork is ran for all mods and shove the entries into whichever noise setting entry Terraforged reads from in the WorldGenRegistries. This will maximize Terraforged’s compatibility with structure mods and leave out only the far few who doesn’t add to WorldGenRegistries or that field either. I do not see any realistic side-effects of this method either and should be very safe.

Now I’m off to sleep

There is a clear and clean way to register your stuff without AT'ing, without mixin'ing, without breaking immutabiity guarantees, and without breaking datapack behaviours for end users. There is no good reason not to do it - registering your stuff correctly is a fundamental for intermod compatibility without the need for hard dependency.

There's perhaps a scope for compat/helper mod that uses that map you mention to register other mods' structures for them, but we won't be adding that debt to TF. It's other mods that are changing the access/mutability, not us.

As suggested previously, a forge API might help unify things. I've submitted one myself to help with the issue of datapacks 'ejecting' content which should aleviate you of some of the 'hax'.

The bottom line is; TF has a different implementation to vanilla. Of course it does. We fulfill every obligation of the ChunkGenerator in terms of the methods that we override, the values we return etc. but we cannot have our current internals or future feature internals dictated to us by mods doing things they shouldn't be doing.

Modding is a wild-west of abilities, good and bad information, and people just doing weird & unexpected things. Despite this, TF remains highly compatibile with the vast majority of mods. We 'play by the book' as much as we possibly can to ensure this, and we will always seek to work with those that try to do the same. I don't think that is unreasonable.