factoriolib / flib

A set of high-quality, commonly-used utilities for creating Factorio mods.
https://mods.factorio.com/mod/flib
MIT License
61 stars 15 forks source link

Bounding Box Functions do not respect Orientation #62

Open Caleb-Wishart opened 5 months ago

Caleb-Wishart commented 5 months ago

Was going to use flib bounding box function to do some transformations for a mod I am working on when I noticed that they do not preserve orientation.

With flib_bounding_box.ceil(box) for example:

local area = {
      left_top = { x = 0.5, y = 0.5 },
      right_bottom = { x = 0.5, y = 0.5},
      orientation = 0.1,
    }
game.print(flib_bounding_box.ceil(area))

will result in { left_top = { x = 0, y = 0 }, right_bottom = { x = 1, y = 1}, }

Proposed solution for Ceil.

function flib_bounding_box.ceil(box)
  if box.left_top then
    return {
      left_top = { x = math.floor(box.left_top.x), y = math.floor(box.left_top.y) },
      right_bottom = { x = math.ceil(box.right_bottom.x), y = math.ceil(box.right_bottom.y) },
      orientation = box.orientation,
    }
  else
    return {
      { math.floor(box[1][1]), math.floor(box[1][2]) },
      { math.ceil(box[2][1]), math.ceil(box[2][2]) },
    }
  end
end

I haven't looked at a solution for the other functions yet as I wanted to see if this is something that would be accepted as a PR or if not preserving the orientation was an intention design choice because it seems like a lot of functions don't properly use orientation and can be confusing as to what ceil would actually mean when applied to a rotated object's coordinate system.

raiguard commented 4 months ago

I completely forgot about orientation. A PR would be welcome, but if not then I'll get to it eventually.

Caleb-Wishart commented 4 months ago

I can get a PR done.

I've already implemented some of the BB functions for the mod I'm working on so can do the rest done soon. Will just get my mod stuff done first.

After working with orientation I found it's not often used or considered (even in some base game functions). Given no one else has noticed / interacted with this PR I don't imagine it's a common issue.