Fix/hardcoded miner ladder block 1.21

[muellerjp]

see pr on version/main:
#11571

Closes #
Closes #

Changes proposed in this pull request

Review please

[github-actions]

In order for this pull request to be merged, make sure you test whether your changes work.

If the changes are working as intended, remove the https://github.com/ldtteam/minecolonies/labels/undefined label from the pull request.
As long as this label is on the pull request, it will not be merged.
If your pull request made no changes to the source code, the label will not be automatically added to the pull request.

Contributors, please review this pull request!

[muellerjp]

tested with "gradlew runClient"

[muellerjp]

Added a fix for signs. The mainbranch code only allowed for very specific signs to be used in the mainshaft schematics. Now it instead checks whether the block is part of the sign_all tag, allowing all types of signs to show the shaft level and amount of nodes, etc.

[Shadizar]

Added a fix for signs. The mainbranch code only allowed for very specific signs to be used in the mainshaft schematics. Now it instead checks whether the block is part of the sign_all tag, allowing all types of signs to show the shaft level and amount of nodes, etc.

I think allowing non vanilla signs is risky...
On some minecraft version updates non vanilla signs lost text (the update that allowed 2 sided signs for example). So, schematics made in the prior version would be blank when actually built in survival in the newer version.
Minecolonies had data preservation for vanilla signs but other mods did not handle the case (I had a big scramble to replace alll non vanilla signs in my style because of it)

So for avoiding such potential problems in the future, it may be unwise to enable non vanilla signs in the mines, no?

[muellerjp]

do you have an example (mod) i could test that with?
maybe there is a tag or other thing that can differentiate 2 sided signs from 1 sided.

My second point to that would be that we can keep that commit in the 1.21 PR and that i leave it out of the PR for the main version. 1.21 will propably not produce any larger minecraft version changes.

How does that sound for you?

[Shadizar]

we can't predict how vanilla may change signs and thus need data migration at any point. Maybe they'll change the text encoding (I dunno what it is). Maybe allow another char per line. Maybe allow controls or ways to make borders or whatever, who knows.

This would create a problem where people become comfortable using modded signs and then things cause it to break in the future, and ppl having to rescan stuff with vanilla signs. It seems too risky for a low benefit (how much do ppl look at the mine signs anyway? Just keeping them vanilla seems the safe call here with no big downside)

I think it was the 1.19 to 1.20 vanilla update, that broke all non vanilla signs because folks had 1.19 schematics and build them in 1.20. But the history is illustrative, and the fact that it doesn't happen 1.20.1 to 1.21.1 doesn't mean it won't happen with 26.2 to 27.x or whatever

[Shadizar]

in any case we know vanilla signs are kept working, and we have concrete example of non vanilla being fragile, so it seems unwise to open that can of worms

[muellerjp]

i assume you are referring to the change that was fixed with this PR in version 1.20:
#9290

The reason why only vanilla signs kept working afterwards is that this solution code hardcoded in vanilla sign classes.
Would have the check there been using the tag instead of hardcoding two specific classes,
then your impression of "fragile" modded signs would have never grown to the current extend.

On functional basis Minecraft changed how to access the text string on signs back then and the change affected all modded and vanilla signs equally. The fix then also fixed it equally for modded and vanilla signs, except the old PR it gatekept modded sign in a separate check.

[Shadizar]

I had been informed by the dev team that their code kept vanilla signs working during the bug, and that non vanilla signs would not be covered by them (and that non vanilla mods didn't bother persisting the data).
The devs recommended not to use non vanilla signs anywhere if you want them to keep working consistently across versions, and builders still usually recommend not to use non vanilla.
I still think it's not a great idea to do it but maybe the dev team can comment. It just seems like a very small gain for the potential large cost of having to replace all signs in a style and rescan it all (a place I've been and do not relish revisiting :) )

[muellerjp]

hey raycoms, could you give me some feedback here? that would be awesome!