Bug #2432
openreversed condition in unit activity change test
0%
Description
In packhand.c , when checking if a unit change represents a change in activity, the condition has `punit->activity_target == packet_unit->activity_target` instead of `!=`.
Meaning in practice this condition is nearly always true (since activity_target is usually null).
Changing it doesn't have any obvious effect I can see, but it's probably at least a small optimisation. The comparison has been this way for 13 years so I doubt it is doing much harm.
Files
Updated by Marko Lindqvist 11 days ago
Andrew McGuinness wrote:
Changing it doesn't have any obvious effect I can see, but it's probably at least a small optimisation. The comparison has been this way for 13 years so I doubt it is doing much harm.
One cannot change activity_target of their own unit without also the activity going through ACTIVITY_IDLE, so this has no effect on own units, or any human controlled units really. For other player's units the only relevant thing within that conditional code block is setting repaint_unit. On standard rulesets, Pillage is the only activity where multiple activity_targets can be legal simultaneously so that one can change between them.
So: If an AI unit (likely barbarian) pillaging one extra on a tile starts pillaging some other extra on the same tile instead, and extra specific pillage icons are used, opponent's client may fail to redraw the unit with the updated icon.
Updated by Marko Lindqvist 9 days ago
- Category set to client-common
- Status changed from New to In Review
- Assignee set to Marko Lindqvist
- Target version set to 3.2.8