Add FishHook#(get/set)BobbingFluids API

[NamiUni]

Is your feature request related to a problem?

I am developing a fishing plugin and would like to implement fishing in lava.

However, the TagKey<Fluid> referenced by the NMS FishingHook class is hardcoded. To allow fishing in lava, I would have to completely reimplement the vanilla water fishing mechanics from scratch. This would require significant effort and could negatively impact server performance.

Describe the solution you'd like.

• Add a TagKey field to the FishingHook class and set its default value to FluidTags.WATER.
• Modify references to FluidTags.WATER within the FishingHook class to use the newly added field instead.
• Add two methods to the Bukkit FishHook class:
  • TagKey<Fluid> getBobbingFluids()
  • void setBobbingFluids(TagKey<Fluid> fluids)
• Implement these methods in CraftFishHook. (Would parsing with PaperRegistries.toNms be the appropriate implementation?)

I considered adding this as a World Configuration option, but since the demand for this feature is limited and the runtime cost for plugins is low, I believe it is unnecessary.

If this request is accepted, I am willing to submit a PR myself :)

Describe alternatives you've considered.

• Reimplement vanilla water fishing mechanics for lava fishing within the plugin.
  • This could lead to inconsistencies between versions.
  • It may negatively impact performance.
• Attempted to modify FluidState using ServerTickStartEvent, but it didn't work.
  • Even if it worked, it would negatively impact performance.
• Use a data pack to modify fluid/water.json in fluid tags by adding flowing_lava and lava. (This is the closest to an ideal implementation.)
  • However, it would affect other game mechanics that rely on fluid tags. Reference
• Implement a CustomFishingHook class.
  • This would involve copying the NMS FishingHook class and modifying only a few lines in the tick method. However, publishing this code on GitHub or including it in a JAR could violate Mojang Mappings licensing terms.
  • Additionally, using NMS directly introduces compatibility issues across versions.
• Fork Paper and modify the FishingHook class directly.
  • This is stupid solution.

Other

No response

[NamiUni]

I forgot that we also need the FluidRegistry modification API. However, since this is off-topic for this issue, we should create a separate issue.

[Doc94]

not sounds bad, how about that part in NMS where check for Block based in BlockState and not for Fluid for particles? this also need to be adapted based in the fluild or is a side case?

[Machine-Maker]

I forgot that we also need the FluidRegistry modification API. However, since this is off-topic for this issue, we should create a separate issue.

Why do you need registry modification for fluids? I don't think that will happen for a while. Shouldn't the getter/setter for the tag be enough?

[NamiUni]

not sounds bad, how about that part in NMS where check for Block based in BlockState and not for Fluid for particles? this also need to be adapted based in the fluild or is a side case?

Wow, you're right! I totally didn't notice that.
Why is it even handled based on BlockState in the first place?
It looks like switching it to FluidState shouldn't cause any issues!

While we're at it, it would be nice to have a way to change the particles being sent.
But I guess that would be better handled by extending PlayerFishEvent, so it might be outside the scope of this topic.

I forgot that we also need the FluidRegistry modification API. However, since this is off-topic for this issue, we should create a separate issue.

Why do you need registry modification for fluids? I don't think that will happen for a while. Shouldn't the getter/setter for the tag be enough?

That's true, too. Sorry 😉

[Machine-Maker]

Yeah, go ahead and open a PR. In your tests, make sure the desync between client/server on the tag in FishingHook doesn't cause odd issues, or if it does, keep them in mind to note them in the PR. I see the tag being used in code ran on the client side, and obviously we can't change that.

[NamiUni]

Thanks! I'll get started on it right away!

[NamiUni]

@Machine-Maker
Lava fishing works, and FishHookState transitions correctly. However, the fishing hook does not float on top of lava.

While testing various things, I realized that, just as you mentioned, there is indeed a strange desync issue with the client. It took some time to figure out, but thanks to your heads-up, I was able to notice it. I really appreciate it!

For now, I'll open the PR and include a video showing the strange behavior as proof.

[NamiUni]

Implementation is currently impossible due to desync issues between the client and server until Vanilla supports modifying the fluid registry.

• The client determines whether a fishing hook can float by referencing the water tag.
• The server does not send any packets to the client between shooting the fishing hook and a fish biting.
• The client ignores velocity packets sent for the fishing hook.

If Vanilla ever adds support for modifying the fluid registry, feel free to ping me in this issue—I'd be happy to work on the PR again! :)