search

storm345dev / ucars

The repo of ucars(bukkit)
10 stars 9 forks source link

Up to v24 #14

Closed Jakllp closed 3 years ago

Jakllp commented 3 years ago

New:

Fixes:

Tweaks:

Jakllp commented 3 years ago

After exploring the idea you had and some more thinking I think I came up with a pretty good solution.

We now climb by looking at the height of the block ahead (the +0.2 makes sure we actually get on top of it as you would sometimes get stuck mid air) This get's rid of checking for slabs or carpets or basically anything. We also avoid "floating" as we won't try to climb a block that is the same height as the current block. This results in normal friction being applied on such blocks.

So what about climbing and going down slopes. If we climb something calculated is set to true -> easy check If we fall down somewhere there is fallDistance added to the car. So we check for that.

I also had to change from 0.65 to 0.55 after some more testing of the actual speed difference.

storm345 commented 3 years ago

After exploring the idea you had and some more thinking I think I came up with a pretty good solution.

We now climb by looking at the height of the block ahead (the +0.2 makes sure we actually get on top of it as you would sometimes get stuck mid air) This get's rid of checking for slabs or carpets or basically anything. We also avoid "floating" as we won't try to climb a block that is the same height as the current block. This results in normal friction being applied on such blocks.

So what about climbing and going down slopes. If we climb something calculated is set to true -> easy check If we fall down somewhere there is fallDistance added to the car. So we check for that.

I also had to change from 0.65 to 0.55 after some more testing of the actual speed difference.

Nice! Using the bounding box is a good use of newer API - I like that change a lot. Looking on the apidocs for that method it says it is approximate and gives stairs as an edge case - can you check that the vehicle is still able to correctly handle stairs? (Hopefully the maxY() from the bounding box will be fine but let's check to be sure).

It is a little odd and annoying that we need to be adjusting for Minecraft velocity tweaks, but if that is what is required I guess it is alright. If we're committing to doing this can we just move the 0.04 and 0.55 magic numbers into labelled constants? Eg. GRAVITY_Y_VELOCITY_MAGNITUDE, and SIMULATED_FRICTION_SPEED_MULTIPLIER or similar? (And any others you have) It will make the code a little cleaner and easier to change with future Minecraft changes instead of magic repeated unlabelled numbers 😛

storm345 commented 3 years ago

(closing was a misclick haha)

Jakllp commented 3 years ago

Checked stairs, speed is right, climbing works well

Jakllp commented 3 years ago

Just a sidenote: Minecraft server versions handle Minecarts differently than single player and in general Minecarts are strange. This is why I wasn't that surprised that this was a Minecraft-Thing This for example is a "bug" I found: https://bugs.mojang.com/browse/MC-205117

storm345 commented 3 years ago

Hmmm if that is the case, could you please add a check if the entity being used as the car is a Minecart before applying the speed adjustments? It's possible to use uCars with different entities (minecarts are just the default). For example with uCarsTrade you can instead use armor stands (It uses a modified version of the entity so that it implements Vehicle) - which I assume don't have the same velocity weirdness?

Jakllp commented 3 years ago

I tested it with armor-stands and the strange slow down did not apply so I'm guessing it's really just Minecarts

storm345 commented 3 years ago

It's looking good! Can you check the y velocity tweak (0.04 instead of 0) aswell? I suspect that needs to be 0 for armor stands. After that I think it's good to merge!

Jakllp commented 3 years ago

Yeah forgot about that...