Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error when full share transfers units to a dead player #6432

Merged
merged 5 commits into from
Sep 10, 2024

Conversation

lL1l1
Copy link
Contributor

@lL1l1 lL1l1 commented Sep 4, 2024

Issue

Reported on Discord. Replay 23190625.

WARNING: Error running lua script: ...aforever\replaydata\gamedata\lua.nx2\lua\aibrain.lua(498): bad argument #1 to `getn' (table expected, got no value)
         stack traceback:
         	[C]: in function `getn'
         	...aforever\replaydata\gamedata\lua.nx2\lua\aibrain.lua(498): in function <...aforever\replaydata\gamedata\lua.nx2\lua\aibrain.lua:479>
         	...aforever\replaydata\gamedata\lua.nx2\lua\aibrain.lua(526): in function `TransferUnitsToHighestBrain'
         	...aforever\replaydata\gamedata\lua.nx2\lua\aibrain.lua(608): in function <...aforever\replaydata\gamedata\lua.nx2\lua\aibrain.lua:440>

The game tried to transfer units to a defeated player which causes the TransferUnitsOwnership function to return nil, and TransferUnitsToBrain didn't handle this case before putting the results into table.getn.

Description of the proposed changes

Add a guard so that it puts an empty table into getn if the result is nil.

Testing done on the proposed changes

Running the replay again with the changes properly transfers the units after the player dies.
To run the replay with changes from your local repo, load the replay from the FAF client to unpack it, and then run the console command:

UI_Lua LaunchReplaySession('C:/ProgramData/FAForever/data/cache/temp.SCFAReplay')

Checklist

  • Changes are annotated, including comments where useful
    • The TransferUnitsOwnership function has a proper return value of Unit[]?
  • Changes are documented in the changelog for the next game version

@lL1l1 lL1l1 added type: bug area: sim Area that is affected by the Simulation of the Game labels Sep 4, 2024
lua/aibrain.lua Outdated
@@ -495,7 +495,7 @@ AIBrain = Class(FactoryManagerBrainComponent, StatManagerBrainComponent, JammerM
units = self:GetListOfUnits(categories.ALLUNITS - categories.WALL - categories.COMMAND, false)
end
if units and not table.empty(units) then
local givenUnitCount = table.getn(TransferUnitsOwnership(units, brain.index))
local givenUnitCount = table.getn(TransferUnitsOwnership(units, brain.index) or {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overthinking. Put function result into a variable and check against nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, also I can use next() instead of table.getn > 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also bad idea. table.empty would be more understandable.

@4z0t
Copy link
Member

4z0t commented Sep 9, 2024

I doublt table.empty accepts nil

@lL1l1
Copy link
Contributor Author

lL1l1 commented Sep 9, 2024

It does accept nil:

fa/lua/system/utils.lua

Lines 59 to 62 in 2d1e06a

function table.empty(t)
if type(t) ~= 'table' then return true end
return next(t) == nil
end

@4z0t
Copy link
Member

4z0t commented Sep 9, 2024

It does accept nil:

fa/lua/system/utils.lua

Lines 59 to 62 in 2d1e06a

function table.empty(t)
if type(t) ~= 'table' then return true end
return next(t) == nil
end

https://github.com/FAForever/fa/blob/develop/lua/simInit.lua#L25
Sim uses asm implementation.

@lL1l1
Copy link
Contributor Author

lL1l1 commented Sep 9, 2024

Console command:

SimLua 
LOG('"table.empty":', table.empty, '"table.empty(nil)"', table.empty(nil))
LOG('"table.empty2":', table.empty2, '"table.empty2(nil)"', table.empty2(nil))

Output:

info: "table.empty":	function: 22180F28	"table.empty(nil)"	true
info: "table.empty2":	cfunction: 1AF80C00	"table.empty2(nil)"	true

@lL1l1
Copy link
Contributor Author

lL1l1 commented Sep 9, 2024

Also debug.getinfo says that table.empty in Sim is sourced from /lua/system/utils.lua

@4z0t
Copy link
Member

4z0t commented Sep 9, 2024

It is seems like a bug. Currently pending.

@4z0t
Copy link
Member

4z0t commented Sep 9, 2024

#6439

@lL1l1 lL1l1 merged commit d3145bc into FAForever:develop Sep 10, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: sim Area that is affected by the Simulation of the Game type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants