Change Details

This fixes a class of bugs related to entity renaming (upgrade, promotion, construction, mirages). It is essentially the same spirit as rP22023. UnitAI properly update orders to use the new target, but the currently active FSM state was not running any check that the new target was valid (nor, in fact, handling anything specially). The simplest fix is to exit-reenter. By processing a message, this can be overwritten easily should there be cases where we want to handle that explicitly. This can be seen as a stronger variant of rP23449, fixing related issues in other cases. UnitMotion is still not made aware of entity renaming, as the leave-enter makes it irrelevant in practice. It still may be a good idea to implement that someday. ---- Fixes the concern raised at rP20393. ---- As reported by Freagarach: https://code.wildfiregames.com/rP22526#38783 As reported by bb: https://code.wildfiregames.com/rP20393#33645 Other, unreported variants of this issue likely exist (I have not found any from a rapid search on Trac). I have added a test case for bb's example, as well as 2 other cases. This is not exhaustive, but it validates that we are not doing anything too odd.

This fixes a class of bugs related to entity renaming (upgrade, promotion, construction, mirages). It is essentially the same spirit as rP22023. UnitAI properly update orders to use the new target, but the currently active FSM state was not running any check that the new target was valid (nor, in fact, handling anything specially). The simplest fix is to exit-reenter. By processing a message, this can be overwritten easily should there be cases where we want to handle that explicitly. This can be seen as a stronger variant of rP23449, fixing related issues in other cases. UnitMotion is still not made aware of entity renaming, as the leave-enter makes it irrelevant in practice. It still may be a good idea to implement that someday. ---- With regards to information leak (a concern here because units might bet renaming information that they should not have): - Mirages are correct-by-design - States for which this is relevant should already react to a unit going in FoW. Indeed, combat states already do this. - The OG bug is that UnitMotion should fail to track a target that is no longer visible, but that is currently unimplemented. Relying on incorrect behaviour sounds broken, and I this does not change UnitAI states significantly from their current behaviour (units might stop where they would have carried on moving before in rare cases). Chasing was fixed in rP23566. ---- Fixes the concern raised at rP20393. ---- As reported by Freagarach: https://code.wildfiregames.com/rP22526#38783 As reported by bb: https://code.wildfiregames.com/rP20393#33645 Other, unreported variants of this issue likely exist (I have not found any from a rapid search on Trac). I have added a test case for bb's example, as well as 2 other cases. This is not exhaustive, but it validates that we are not doing anything too odd.

This fixes a class of bugs related to entity renaming (upgrade, promotion, construction, mirages). It is essentially the same spirit as rP22023. UnitAI properly update orders to use the new target, but the currently active FSM state was not running any check that the new target was valid (nor, in fact, handling anything specially). The simplest fix is to exit-reenter. By processing a message, this can be overwritten easily should there be cases where we want to handle that explicitly. This can be seen as a stronger variant of rP23449, fixing related issues in other cases. UnitMotion is still not made aware of entity renaming, as the leave-enter makes it irrelevant in practice. It still may be a good idea to implement that someday. ---- With regards to information leak (a concern here because units might bet renaming information that they should not have): - Mirages are correct-by-design - States for which this is relevant should already react to a unit going in FoW. Indeed, combat states already do this. - The OG bug is that UnitMotion should fail to track a target that is no longer visible, but that is currently unimplemented. Relying on incorrect behaviour sounds broken, and I this does not change UnitAI states significantly from their current behaviour (units might stop where they would have carried on moving before in rare cases). Chasing was fixed in rP23566. ---- Fixes the concern raised at rP20393. ---- As reported by Freagarach: https://code.wildfiregames.com/rP22526#38783 As reported by bb: https://code.wildfiregames.com/rP20393#33645 Other, unreported variants of this issue likely exist (I have not found any from a rapid search on Trac). I have added a test case for bb's example, as well as 2 other cases. This is not exhaustive, but it validates that we are not doing anything too odd.