Fix various horse bugs #2115
Fix various horse bugs #2115
Conversation
|
In the past month, there have been a couple instances of people reporting in the discord that they got kicked while on a horse. One of which was two days ago. I'm not saying testing isn't necessary, but if it happened once it might not necessarily be because of your changes |
...in/java/org/geysermc/connector/network/translators/bedrock/BedrockPlayerInputTranslator.java
Outdated
Show resolved
Hide resolved
...in/java/org/geysermc/connector/network/translators/bedrock/BedrockPlayerInputTranslator.java
Outdated
Show resolved
Hide resolved
|
|
||
| if (vehicle instanceof BoatEntity) { | ||
| // Remove some Y position to prevents boats flying up | ||
| vehiclePosition = vehiclePosition.down(EntityType.BOAT.getOffset()); |
There was a problem hiding this comment.
This is inconsistent with the offset we currently apply to boats (https://github.com/GeyserMC/Geyser/blob/master/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockMoveEntityAbsoluteTranslator.java#L48). I don't think the offset I chose was perfect, but the two should probably be consistent.
There was a problem hiding this comment.
Logging the two values seem to give the same answer.
...geysermc/connector/network/translators/bedrock/entity/player/BedrockRiderJumpTranslator.java
Outdated
Show resolved
Hide resolved
| } | ||
| if (sendMovement) { | ||
| long timeSinceVehicleMove = System.currentTimeMillis() - session.getLastVehicleMoveTimestamp(); | ||
| if (timeSinceVehicleMove >= 100) { |
There was a problem hiding this comment.
Should you set the lastVehicleMovementTimestamp here?
There was a problem hiding this comment.
No, as that is set in BedrockMoveEntityAbsoluteTranslator. The goal of this is to send a ClientVehicleMovePacket with every ClientSteerVehiclePacket.
| // Add dummy health attribute since LivingEntity updates the attribute for us | ||
| attributes.put(AttributeType.HEALTH, AttributeType.HEALTH.getAttribute(20, 20)); | ||
| // Add horse jump strength attribute to allow donkeys and mules to jump | ||
| attributes.put(AttributeType.HORSE_JUMP_STRENGTH, AttributeType.HORSE_JUMP_STRENGTH.getAttribute(0.5f, 2)); |
There was a problem hiding this comment.
Does this cause an issue with llamas?
There was a problem hiding this comment.
I disabled power jumping on llamas in the latest commit.
| @@ -90,7 +90,7 @@ public void updateBedrockMetadata(EntityMetadata entityMetadata, GeyserSession s | |||
| } | |||
|
|
|||
| // Needed to control horses | |||
| metadata.getFlags().setFlag(EntityFlag.CAN_POWER_JUMP, true); | |||
| metadata.getFlags().setFlag(EntityFlag.CAN_POWER_JUMP, entityType != EntityType.LLAMA); | |||
There was a problem hiding this comment.
You should also add TRADER_LLAMA as an exclusion.
There was a problem hiding this comment.
Sorry about the wait in reviewing!
Really, my only major complaint is how jank the health attribute system is right now in Geyser. It doesn't even work right without initializing the attribute, and other entities such as the ender dragon completely disregard it. In the future, we need to send the health attribute for iron golems in order for their health to properly compute and show the iron golem cracked texture for the GeyserOptionalPack. I've been hesitant to touch the system since everything "works" fine as-is, but it's due for a revamp.
This may need some additional testing. I once got randomly kicked for flying while jumping on a horse, but I can't seem to reproduce it.