HomeWildfire Games

Check for UnitMotion component [UnitAI]

Description

Check for UnitMotion component [UnitAI]

Event Timeline

elexis added a subscriber: elexis.Mar 25 2020, 6:57 PM

((Side node Documenting the use case / intention / objectives of the commit allows readers (including future self) to not have to guess that and to become able to search for use cases and find the commits implementing them.
Thats typically done in a summary of a revision proposal - if that is skipped it would be good to mention that briefly in the commit message to yield those benefits.)

In this case it was a compatibility patch for Hyrule Conquest fairies that use BuildingAI with unit aI move orders(?)
Refs discussion at http://irclogs.wildfiregames.com/2020-03/2020-03-20-QuakeNet-%230ad-dev.log
(Perhaps there was a forum post in the thread for this bug?)

(One could also speculate whether Hyrule used the 0ad components badly or whether the 0ad simulation components are badly written, but that's a timesink again, adding those bool checks is logically correct, the worst side effect may be a very slight performance issue if the check hypothetically was deemed to be unneeded and the error in the user of the function))

Angen added a comment.Mar 25 2020, 8:52 PM

Following functions already checked if unitmotion exists.

GetWalkingSpeed
GetRunMultiplier
ResetSpeedMultiplier
SetSpeedMultiplier
MoveRandomly
SetFacePointAfterMove
FaceTowardsTarget

Missing checks were in the move to function family.

StopMoving
MoveToPoint
MoveToPointRange
MoveToTargetRange
MoveToTargetRangeExplicit
MoveToGarrisonRange
MoveToTargetAttackRange

I found it when I indeed tried to put unitai to entity without unitmotion, but it was not directly requested or reported before doing this (at least I am not aware of that).
Also Always check for undefined properties and/or invalid object references, if it's possible they could occur. In this case it is possible since it is not guaranteed another code outside the functions would check that. This is not system component and it is not required by this component to have unitmotion component, it expects that it can be missing but just in the half of the code and if it is actually missing it errors on the other half. So main reason was to make it consistent behaviour and as it is not ===, !== comparison. Also I think there are another places where one should worry about performance.