fabric icon indicating copy to clipboard operation
fabric copied to clipboard

[Suggestion] Add `c:farmlands`

Open dayofpi opened this issue 1 year ago • 11 comments

It's common to want to add a custom farmland block to a mod, this tag would make having modded plants compatible with modded farmland simpler, especially if Fabric automatically changes vanilla crops to accept the tag.

dayofpi avatar Jun 07 '24 16:06 dayofpi

Having c:farmlands would be nice. I was just missing it yesterday.

especially if Fabric automatically changes vanilla crops to accept the tag

This bit seems unlikely to me, particularly since there are a surprisingly large number of mixins required. However, you can use Terraform API for it (or to see examples of relatively compatible versions of the mixins).

gniftygnome avatar Jun 07 '24 17:06 gniftygnome

Didn't know Terraform included something for farmland, thank you

dayofpi avatar Jun 07 '24 18:06 dayofpi

especially if Fabric automatically changes vanilla crops to accept the tag

Just FYI, Neo has a PR in works that changes the vanilla behavior to point to neoforge:villager_farmlands block tag because it is a significant behavior change. I would think if Fabric does the same, they would do it under the Fabric namespace. It seems like it would be 3 mixins to support Farmlands tags in Fabric

https://github.com/neoforged/NeoForge/pull/971/files

From there, mods with custom plants that do want to support modded farmlands could check the modloader tag

TelepathicGrunt avatar Jun 17 '24 10:06 TelepathicGrunt

So in Neoforge, we realized that using tags for farmland checks within vanilla code could cause desyncs if a vanilla client connects to the server with a modified tag. So we opted for changing the .is(Items.Farmland) checks in vanilla to be instanceof FarmBlock instead so people can make their farmlands extend FarmBlock and work out of the box. This may be more ideal for Fabric to do as well since the tag desync can be an issue

TelepathicGrunt avatar Jul 04 '24 11:07 TelepathicGrunt

could cause desyncs if a vanilla client connects to the server with a modified tag

I think it doesn't matter, since there are a great many of server side mods and many functions of them can cause desyncs. We only need to make sure that server works like vanilla if there is no custom datapack.

Phoupraw avatar Sep 06 '24 18:09 Phoupraw

@Phoupraw say you have a Fabric server with a datapack that tags sand as Farmland. Fabric runs the logic on server based on that tag. Vanilla client connects to the server. The tag syncs to vanilla but vanilla has no logic change using that tag. Vanilla client would see desync with server when doing an action that would’ve had a different behavior on client if client had fabric on instead.

I don’t think it is a good idea for modloaders to introduce this kind of desync with datapacks only. It would trick users into thinking the modloader tag is safe to modify on fabric server with vanilla client when it actually isn’t. Users won’t know which tag is truly single side and which one needs logic on both sides

TelepathicGrunt avatar Sep 06 '24 18:09 TelepathicGrunt

Only modders will use this tag, so we can add the description to the javadoc in ConventionalBlockTags#FARMLANDS.

Phoupraw avatar Sep 06 '24 19:09 Phoupraw

How do you know the tag won’t be utilized by players or vanilla compat modpack makers? That’s an assumption I wouldn’t risk. I can see someone seeing the farmland tag and then try to add more to it thinking it allows them to plant on more blocks. Which this tag may allow (depending on which checks gets replaced with the tag)

TelepathicGrunt avatar Sep 06 '24 19:09 TelepathicGrunt

Well, tag is a kind of config essentially. I think there are four kinds of configs in general:

  1. Purely cliend side. Server won't and can't use it. E.g. key bindings, video settings.
  2. Purely server side. Client won't and can't use it. E.g. loot tables, world gen.
  3. Common sides. Both server and client use it. It can't be synced from server to client. E.g. items registry, blocks registry.
  4. Server sync to client. Both server and client use it but it can be synced from server to client without causing any bug. E.g. recipes, vanilla tags.

Farmlands tag is the 4th one. Do we have to make sure all tags are the 2th one?

Phoupraw avatar Sep 06 '24 19:09 Phoupraw

if dual side functionality is changing (such as allowing block or item usage), that modifies vanilla behaviour. recipes are managed serverside, behaviour is not changed clientside. vanilla tags are the same way, there is no behavioral change over vanilla regardless of the server driven config. if custom behaviour is being made on each side, then thats not a serverside mod, its required on both sides. if the goal is to be compatible with vanilla clients without any other mods, then yes 1 and 2 are required.

You are turning the farmlands tag into a 5th type: server synced config that controls modded behaviour that is not synced (which is more like type 3 than 4)

Linguardium avatar Sep 06 '24 19:09 Linguardium

OK, but I think instanceof FarmBlock is not flexible enough. BlockApiLookup is more flexible, while only mod can modify it.

Phoupraw avatar Sep 06 '24 19:09 Phoupraw