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

Dead bushes can be placed on glazed terracotta, unlike vanilla. #306

Closed
Lolothepro opened this issue Nov 29, 2023 · 2 comments · Fixed by #416
Closed

Dead bushes can be placed on glazed terracotta, unlike vanilla. #306

Lolothepro opened this issue Nov 29, 2023 · 2 comments · Fixed by #416
Labels
1.20 Targeted at Minecraft 1.20 bug A bug or error

Comments

@Lolothepro
Copy link
Contributor

MinecraftForge/MinecraftForge#8732

@Lolothepro Lolothepro added the triage Needs triaging and confirmation label Nov 29, 2023
@Matyrobbrt Matyrobbrt added this to the 20.4 Stable Release milestone Dec 8, 2023
@Lolothepro
Copy link
Contributor Author

@sciwhiz12 sciwhiz12 added bug A bug or error 1.20 Targeted at Minecraft 1.20 and removed triage Needs triaging and confirmation labels Dec 23, 2023
@sciwhiz12
Copy link
Member

Looking through the code shows the cause to be simple: in the Block patch which implements PlantType, the DESERT plant type is allowed to be placed on glazed terracotta.

That is in error, since there is no vanilla plant which is placeable on glazed terracora, and there is no reason that plants should be placeable on glazed terracotta.

After a bit of patch hunting throughout the ages, I believe I've narrowed down the source of that particular error. In MinecraftForge/MinecraftForge@7c0d94c#diff-67749f72fc217feef6791d7e82996c56ab23ef92ce892c85560585198543f040R233, one of the patches of the 1.13 porting efforts, it seems the EnumPlantType1 patch was reimplemented with that addition of the check against the glazed terracotta class. (Previous versions of the patch did not use glazed terracota.)

While I think the best long-term course of action here is to revisit the PlantType system -- to either improve it or replace it -- the short-term solution is to simply remove glazed terracotta from the DESERT plant type check. It will be a minor regression, as desert plants will not be placeable on glazed terracotta, but I think it's minor enough that no mod will substantially notice.

Footnotes

  1. The Enum prefix is a relic of that era, which was borne out of (as I recall from past conversations) the need to specifically identify enums during the time when all Minecraft classes were placed under the net/minecraft/src package, when subpackages were not yet used in the mappings.

sciwhiz12 added a commit to sciwhiz12/NeoForge that referenced this issue Dec 23, 2023
Matyrobbrt pushed a commit that referenced this issue Dec 23, 2023
Fixes #306 by removing glazed terracotta as acceptable farmland for the `DESERT` plant type.

Also includes a related fix to expand `DESERT` plant types to be plantable on both uncolored/raw terracotta and stained terracotta, to match with the plantability of dead bushes on both. (This only really matters for modded plants, because dead bushes extend `BushBlock` and therefore have their `mayPlaceOn` method called to allow placing on stained terracotta anyway.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20 Targeted at Minecraft 1.20 bug A bug or error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants