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.

Sign up for GitHub

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

Jump to bottom

Fix Fluids #55

Open
wants to merge 4 commits into
base: 1.20
Choose a base branch
from
Open

Fix Fluids #55

wants to merge 4 commits into from

Conversation

Rover656
Copy link
Contributor

This PR addresses a handful of concerns with the original implementation.

  1. Fixed FluidEntry<>#getBlock -> This would always would query for modname:flowing_fluid_name instead of modname:fluid_name. Fixed this by making the FluidBuilder create both the source and flowing (and swapped the order) without any extra builder calls.
  2. Fixes fluid render types - applied the same fix as for blocks etc., it just needed to have another supplier around it to fix server-side support.
  3. More sensible fluid properties - we unset the bucket and block suppliers now if noBucket/noBlock is called

This could be considered breaking as the API has shifted slightly, so might want to bump major version :)

@Rover656
Rework fluid builder (Read desc)
4656568 
- Added source and flowing as type parameters, gets rid of the need for the source() methods on the builder.
- Render type is now a supplier<supplier<?>> to fix breakage on servers.
- Changed the builder from defaulting to the flowing fluid to the source fluid, this fixes the getBlock call in the fluid entry.

Will reformat tomorrow before PR.
@Rover656
Tidying
668201b
@Rover656
Better handle noBucket and noBlock
403a77e
@Rover656
Remove todo
07b333d
This was referenced Aug 27, 2024
Closed
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@Rover656