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

Entity#isInWater, Entity#isTouchingWater and Entity#isInsideWater are confusing names. #954

Closed
LambdAurora opened this issue Nov 12, 2019 · 8 comments
Labels
bug Fixes or discusses a bug within the mappings

Comments

@LambdAurora
Copy link
Contributor

Methods isInWater and isInsideWater are currently very confusing: the methods have very similar names but doesn't return the same result at all.

isInWater returns true only when the full hitbox is in water, and should be renamed to isSubmergedInWater. And isInWater returns the value of the field inWater which is set by isSubmergedIn(FluidTags.WATER, true).
Renaming the inWater field should also be done then.

isInsideWater returns true when the hitbox hits water, the method should be renamed to isTouchingWater. The associated field insideWater should also be renamed.

As isTouchingWater already exists, the method should also be renamed.
The name isWet came in some minds but it might conflict with WolfEntity#isWet which is client-side only.

@Pyrofab
Copy link
Contributor

Pyrofab commented Nov 12, 2019

Maybe we could rename WolfEntity#isWet to isWolfWet ?

@LambdAurora
Copy link
Contributor Author

My problem by renaming isTouchingWater to isWet is it's not entirely correct on a language point of view. You can be wet for a certain time after touching water. I thought of isMakingContactWithWater but it's way too long.

@Pyrofab
Copy link
Contributor

Pyrofab commented Nov 16, 2019

Could always be isTouchingWaterBlock and isTouchingWater. It's not ideal but it should be clear enough.

@Runemoro
Copy link
Contributor

Since the isWet method is a separate method in WolfEntity (rather than just using Entity.isWet, or overriding it), I'm guessing that the Mojang name is probably something like shouldDoXyz, where "xyz" is something that wolves do only when wet.

@Runemoro Runemoro added the bug Fixes or discusses a bug within the mappings label Nov 19, 2019
@liach
Copy link
Contributor

liach commented Nov 19, 2019

Depends. Wolfs shake their body when they are wet, and if they are wet water particles are rendered on them as well. shouldDoXyz may not fit as well for the particle render case.

@Sollace
Copy link
Contributor

Sollace commented Dec 12, 2019

@Runemoro I agree with that. Might be isFurWet or isASoggyBoi.

Situation might be that the method on wolves predates the methods on Entity.

@Pyrofab
Copy link
Contributor

Pyrofab commented Dec 14, 2019

+1 for Wolf#isFurWet

@LambdAurora
Copy link
Contributor Author

This issue is fixed with #1058 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes or discusses a bug within the mappings
Projects
None yet
Development

No branches or pull requests

5 participants