pmmp / Math

PHP library containing math related code used in PocketMine-MP
GNU Lesser General Public License v3.0
41 stars 20 forks source link

AxisAlignedBB: added method to shift it along a given direction #64

Closed ColinHDev closed 1 year ago

ColinHDev commented 2 years ago

Explanation

As explained in https://github.com/pmmp/Math/issues/63, there is currently no easy way to shift an AxisAlignedBB along a given direction. Instead, that must be done with AxisAlignedBB::offset() which forces developers to make their own checks to determine the axis, based on the direction, the AABB should be shifted along, as done in https://github.com/pmmp/PocketMine-MP/blob/ad56392d959e9d4eefb1cdae4bc659eac7ae2e1f/src/block/Skull.php#L128-L132.

API changes

ColinHDev commented 2 years ago

(I thought it would be a good idea to not reinvent the wheel and just rely on the already existing methods. That's why I am using AxisAlignedBB::extend() instead of writing an own if else chain.)

dktapps commented 2 years ago

This looks like a hack.

dktapps commented 2 years ago

Food for thought: I wonder if it might make more sense to accept an axis instead of a facing. e.g.

bb->shift(Axis::Y, that->y - this->y)

instead of

bb->shift(that->y < this->y ? Facing::DOWN : Facing::UP, abs(that->y - this->y))

Seems like it would simplify the logic at call site and in the function itself.