Change Details

Currently, UnitAI often calls "MoveToTarget/Point" without checking if we are already in range of that. This works because UnitMotion's MoveToTarget/Point range functions return "false" if we are already in range (according to UM). This is broken because: - UnitAI should be in charge of deciding if we are in range for actions, not unit motion. - UnitMotion should not be the component with range checking functions (these have nothing to do with movement, see D981). - Relying on movement supposes the entity has a `UnitMotion` component, which it might not. By explicitly checking we are in range first, we are moving closer to merging BuildAI and unitAI. - `MoveTo...Range` should be retuning whether the move will be successful or not - a move which is in range is arguably successful, not "not successful". This confuses the cases where we //can't move//, //can't reach the target// and //don't need to move//. Follow-up diffs will make `MoveTo...Range` return `true` if the goal is reachable, and false if it is not, allowing UnitAI to make decisions based on that instead, which is much better. For that, we need explicit range checks first. Note that there are several places in UnitAI where we are already calling `Check...Range` before moving, so this is arguably a standardisation. This is a prerequisite to D981 since that currently sends us in an infinite loop because range checks in unit motion and the range functions are actually different.

Currently, UnitAI often calls "MoveToTarget/Point" without checking if we are already in range of that. This works because UnitMotion's MoveToTarget/Point range functions return "false" if we are already in range (according to UM). This is broken because: - UnitAI should be in charge of deciding if we are in range for actions, not unit motion. - UnitMotion should not be the component with range checking functions (these have nothing to do with movement, see D981). - Relying on movement supposes the entity has a `UnitMotion` component, which it might not. By explicitly checking we are in range first, we are moving closer to merging BuildAI and unitAI. Note that there are several places in UnitAI where we are already calling `Check...Range` before moving, so this is arguably a standardisation. This is a prerequisite to D981 since that currently sends us in an infinite loop because range checks in unit motion and the range functions are actually different.

Currently, UnitAI often calls "MoveToTarget/Point" without checking if we are already in range of that. This works because UnitMotion's MoveToTarget/Point range functions return "false" if we are already in range (according to UM). This is broken because: - UnitAI should be in charge of deciding if we are in range for actions, not unit motion. - UnitMotion should not be the component with range checking functions (these have nothing to do with movement, see D981). - Relying on movement supposes the entity has a `UnitMotion` component, which it might not. By explicitly checking we are in range first, we are moving closer to merging BuildAI and unitAI. - `MoveTo...Range` should be retuning whether the move will be successful or not - a move which is in range is arguably successful, not "not successful". This confuses the cases where we //can't move//, //can't reach the target// and //don't need to move//. Follow-up diffs will make `MoveTo...Range` return `true` if the goal is reachable, and false if it is not, allowing UnitAI to make decisions based on that instead, which is much better. For that, we need explicit range checks first. Note that there are several places in UnitAI where we are already calling `Check...Range` before moving, so this is arguably a standardisation. This is a prerequisite to D981 since that currently sends us in an infinite loop because range checks in unit motion and the range functions are actually different.