Minecraftforge: EntityGuardian.canBreatheUnderwater() is false

[jredfox]

EntityGuardian.canBreatheUnderwater() is false. This is a vanilla bug please fix it. I also reported it on their issue tracker for future releases:
https://bugs.mojang.com/browse/MC-132623

how to fix: simply edit the class of EntityGuardian.canBreatheUnderwater() to return true

Without this mods will fail to detect that a guardian is a water mob. Simple patch on your part to return true and will make it mod compatible with vanilla.

[killjoy1221]

The question is do guardians breathe?

[jredfox]

it's a boolean you either need this to be true or have a new boolean called isWaterMob() for all mobs and while your at it isFireMob() isMonster(), isHostile(), isPeaceful(),isAmbient(),isFlying() etc..... or you could just set the thing to true

[mezz]

Forge accepts PRs for confirmed vanilla bugs, but there isn't much point in opening an issue here.
The Mojang tracker is a much better place for vanilla bugs.

[pau101]

This can only be considered a bug within our own understanding of what the intention of the code is. What we have currently is the mcp mappings imposing the purpose of the method being whether the entity can breathe underwater or not.

The context we have to make this judgement:

• Its default implementation returning false
• Singular invocation of it used to check if the entity should perform drown logic in EntityLivingBase#onEntityUpdate()
• Override in EntityWaterMob to return true
• Implementation of EntityWaterMob#onEntityUpdate() performing drown logic for _not_ being in water
• Sole descendent of EntityWaterMob being EntitySquid

Looking at EntityGuardian to determine why it doesn't need to do a similar override to avoid drown damage we see the implementation of EntityGuardian#onLivingUpdate() replenishing air each tick the entity is in water. Effectively preventing drown damage from possibly being performed. We also identify that Guardian entities can live outside of water.

The evidence suggests that the mcp imposed meaning canBreatheUnderwater is inaccurate and a more appropriate name could be hasGills, requiresWater, etc. (my examples aren't great)

The immediate options are for forge to side with mcp and patch EntityGuardian or to submit an issue to mcp to change the mapping. In order to make the most informed decision, waiting on a developer response to MC-132623 would be ideal.

[jredfox]

so @pau101 your saying that it should return false because they can drowned on land maybe there needs to be two booleans here.

Either way we need methods to define entity types water,fire,hostile,peaceful,ambient,creature,boss etc... Currently the best way is to check instanceof hard coded class or check it's spawn behavior rather then the actual entity returning what it should and shouldn't be

Should I make a separate pull request and leave this open for others to make a more proper definition for canBreathUnderwater() seperation from canBreathOnLand() ????

[pau101]

In the context of forge, yes, an "entity dictionary" of sorts would be convenient. But that discussion would continue on a different issue.

[jredfox]

ok opening up another issue if I code it won't be good as forge's api so I won't do a pull request

But, yeah this is still an issue there needs to be two booleans here not one and then a patch for the gaurdian one for land one for water and if can't breathe on land then you start drowning.

Also I recommend when forge does this that they store them by default as variables so other mods can change other entities and that's where the fun begins.

[jabelar]

This issue though explains why an entity dictionary of entity "types" doesn't work well. A Guardian is a water mob but doesn't get hurt out of water. In other words, most entities are pretty unique and their type is actually their Java type -- like you said you pretty much just use instanceof.

So I guess it really depends on what you want the types for. If you want the types to simply be "spawns in water" type or something I guess that could be useful. But if you ALSO wanted it to represent how they "breathe in water", "move in water", "act (i.e. like aggressive) in water" and so forth it will get very complicated very fast.

The Java type hierarchy should actually mostly already be logically organized. Of course, the Minecraft source is actually inconsistent in many areas so I suppose it can be improved. But the Java type hierarchy is mostly organized by similarities in behavior.

[jredfox]

right so there should be two booleans one for land one for water canBreathe() the design needs to be altered in order for a proper fix

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___." or "Here's a screenshot of this issue on the latest version"). Thank you for your contributions!

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.