Skip to content

Commit

Permalink
GetUnitEffectiveBuildRange accepts nil unitDefID
Browse files Browse the repository at this point in the history
Some tasks (in particular, feature reclaim) do not have a relevant unitDefID.
In theory this means that the function should just accept a featureID, but I
don't want to do this before ticket #717 is done, and also because it needs
more design thought.

Some of the possible questions:
* what about terraform? How should the API look to request it?
* could featureDefID ever be a sensible thing to request?

Right now though, accepting `nil` acknowledges the existence of these
edge cases without actually addressing them in full.
  • Loading branch information
sprunk committed Aug 16, 2023
1 parent 10c4252 commit 6f0466e
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 5 deletions.
6 changes: 3 additions & 3 deletions doc/changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ Lua:
and `beingBuilt` from `GetUnitIsStunned`, but as you can see it wasn't terribly convenient/intuitive.
- rules params now support the boolean type. Skirmish AI and the rules param selection filter can
read them via existing numerical interface by using 0 and 1.
- add `Spring.GetUnitEffectiveBuildRange(builderID, buildeeDefID)`. Returns the effective build range
for building given target, counted from the center of the prospective target's center. Useful for
setting move goals manually.
- add `Spring.GetUnitEffectiveBuildRange(builderID[, buildeeDefID])` -> number. Returns the effective
build range for building given target, counted from the center of the prospective target's center.
Returns just the build range if buildee is nil. Useful for setting move goals manually.
- add `Script.DelayByFrames(frameDelay, function, args...)`. Runs `function(args...)` after a delay
of the specified number of frames (at least 1). Multiple functions can be queued onto the same frame
and run in the order they were added, just before that frame's GameFrame call-in.
Expand Down
24 changes: 22 additions & 2 deletions rts/Lua/LuaSyncedRead.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4279,8 +4279,8 @@ int LuaSyncedRead::GetUnitWorkerTask(lua_State* L)
* @function Spring.GetUnitEffectiveBuildRange
* Useful for setting move goals manually.
* @number unitID
* @number buildeeDefID
* @treturn number effectiveBuildRange counted to the center of prospective buildee
* @number buildeeDefID or nil
* @treturn number effectiveBuildRange counted to the center of prospective buildee; buildRange if buildee nil
*/
int LuaSyncedRead::GetUnitEffectiveBuildRange(lua_State* L)
{
Expand All @@ -4292,6 +4292,26 @@ int LuaSyncedRead::GetUnitEffectiveBuildRange(lua_State* L)
if (builderCAI == nullptr)
return 0;

/* FIXME: there are some cases where a unitDefID does not suffice.
* This function was mostly created as a reactive afterthought so
* does not handle them properly, but accepting `nil` acknowledges
* their existence to some extent:
*
* - features, for example reclaim. I think ideally a thingID would
* be the third argument (exclusive with the unitDefID), but this
* requires the featureID ticket (#717) to be done first.
*
* - terraform (restore ground). Fourth boolean parameter? Sounds
* like it's getting a bit bloated, though it's rare and doesn't
* actually pollute the usual use cases.
*
* - design question: would featureDefID ever be a sensible thing
* to use here? I doubt, but it's something to keep in mind. */
if (lua_isnoneornil(L, 2)) {
lua_pushnumber(L, builderCAI->GetBuildRange(0.0f));
return 1;
}

const auto buildeeDefID = luaL_checkint(L, 2);
const auto unitDef = unitDefHandler->GetUnitDefByID(buildeeDefID);
if (unitDef == nullptr)
Expand Down

0 comments on commit 6f0466e

Please sign in to comment.