Unit's image is missing when attacking

[grz0]

Game and System Information

The Battle for Wesnoth version 1.17.6+dev (565b318c9f0166d09f2dbff8f7982a52cd382dbb-Modified) x86_64
Running on Fedora Linux 36 (Workstation Edition) x86_64
Distribution channel: Default

SDL video drivers: wayland [x11] KMSDRM dummy
Window size: 1920x1021
Game canvas size: 1920x1021
Final render target size: 1920x1021
Screen refresh rate: 60
Boost: 1.76
Lua: 5.4.4
OpenSSL/libcrypto: 3.0.0e-dev (runtime 3.0.0e-dev)
Cairo: 1.17.6 (runtime 1.17.6)
Pango: 1.50.8 (runtime 1.50.8)
SDL: 2.0.22 (runtime 2.0.22)
SDL_image: 2.0.5 (runtime 2.0.5)
SDL_mixer: 2.0.4 (runtime 2.0.4)

Description of the bug

Unit's image is missing at the destination hex, sometimes the unit's image is irregularly blinking at hex when the unit was previously standing:

Steps to reproduce the behavior

1. Select unit then move&attack another unit

Expected behavior

No response

Additional context

No response

[Wedge009]

It's not entirely clear to me what the issue is - is the heavy infantryman blinking on attack? I have not observed anything like this in 1.17.7+dev (314f6f7). knyghtmare, how did you confirm this?

Also, the build version indicates a modified source - what was changed?

[grz0]

It's not entirely clear to me what the issue is - is the heavy infantryman blinking on attack?

There are two issue, one is missing unit's sprite (Heavy infantry) at village location. The second issue is that unit's sprite (Heavy infantry) stays at the old position, and it is blinking in an irregular glitchy way (game display switches between those two screenshots, some times Heavy infantry is also cropped at the top).

[Wedge009]

How should one replicate what you described? It has to be more than just 'select unit then move&attack another unit', unless I'm missing something obvious.

[grz0]

It has to be more than just 'select unit then move&attack another unit'

It's just that simple, I don't know how to explain this better 🙂, here is a screenshot of almost all game's window:

In this case, the second issue with old sprite is a bit different, it's not blinking but half of the sprite is drawn under the ice layer. When I close 'Attack Enemy' dialog the sprite is redrawn correctly.

[knyghtmare]

@Wedge009 the screenshot provided by @grz0 confirms it. This was not present in 1.16.x So, it's a confirmed bug.

[Wedge009]

So it's the unit on the village in the original report that's missing? And in that last image, the unit in the water?

As I said, I'm testing on 314f6f7 and I cannot replicate this.

[grz0]

As I said, I'm testing on 314f6f7 and I cannot replicate this.

I just compiled from 314f6f7 and I still see the same issue:

On which platform do you test?

[Wedge009]

Linux/X11.

Your comments suggest you are encountering this with every attack interaction, so I am not trying to replicate any specific circumstances, just randomly attacking units. Strangely enough I can also confirm this now. I recall seeing a similar issue logged previously...

[grz0]

I just compiled 1.16.5 and I cant reproduce the issue. Screenshot from 1.16.6:

[grz0]

It looks that the issue has been caused by commit 06254e5. @mesilliac

$ git bisect log
git bisect start
# status: waiting for both good and bad commits
# good: [f6cfad7b5065d0a99f1c2d9e5115f6d97d08d56b] 1.17.5.
git bisect good f6cfad7b5065d0a99f1c2d9e5115f6d97d08d56b
# status: waiting for bad commit, 1 good commit known
# bad: [ab8fe65b8381fd78d953719193c8e30888caf29a] 1.17.6
git bisect bad ab8fe65b8381fd78d953719193c8e30888caf29a
# good: [0fbc12ea01986bc97ef6426e0edc89f12224a0d5] Remove largely incorrect usage of floating_to_fixed_point()
git bisect good 0fbc12ea01986bc97ef6426e0edc89f12224a0d5
# bad: [2279ec99eb8bed477bdddda4595f2f4afe53e2e0] Fix crash when resizing window in map editor
git bisect bad 2279ec99eb8bed477bdddda4595f2f4afe53e2e0
# good: [c8e948a62b8432c9f66ec5e5ecb93b319e4ce5c4] std::cerr -> PLAIN_LOG, to stop bypassing the logging system
git bisect good c8e948a62b8432c9f66ec5e5ecb93b319e4ce5c4
# good: [b063bb5e2e7f961a23dbcc3ece4432d7ee1ee982] Split ubuntu build into separate steps
git bisect good b063bb5e2e7f961a23dbcc3ece4432d7ee1ee982
# bad: [06254e5581cbb79b5eaf72bec0dbb3fc1566b7a4] Mega draw manager implementation
git bisect bad 06254e5581cbb79b5eaf72bec0dbb3fc1566b7a4
# good: [09fe5a19fda9884b70aed413de84d4f0672d3f1a] Adding Amorphous to credits
git bisect good 09fe5a19fda9884b70aed413de84d4f0672d3f1a
# good: [a964c807ab80daad1eeb661a904fa7218c5ef9bf] Update changelog.md
git bisect good a964c807ab80daad1eeb661a904fa7218c5ef9bf
# good: [3dbf212f8cdeea132617d0d5d61c8ad280ba26c8] Add a draw manager, to manage drawing to the screen
git bisect good 3dbf212f8cdeea132617d0d5d61c8ad280ba26c8
# first bad commit: [06254e5581cbb79b5eaf72bec0dbb3fc1566b7a4] Mega draw manager implementation

[Toranks]

Stills happens on 1.17.10 Sometimes the sprite blinks, sometimes dissapear completely, and others (the most) doesn't happen.

[Toranks]

I've captured this video with a more serious version of this glitch, where the unit stays displaced and blinking permanently until I select it again. This occurs randomly, so far I haven't been able to figure out any more pattern than what seems to be related to scenarios with water (but not always), and that as you can see in the second half of the video, it is not always reproducible.

2023-01-27.22-00-16.mp4

[gfgtdf]

So from the 1.16 screenshot i take that the expected behaviour is that there is simply no unit&halthbar shown at the destination hex? Because the the first message it sounds like it is expected to be there.

A possible fix might be to add some display::invalidate(loc) calls in temporary_unit_mover.

[Toranks]

On the contrary, the unit should be seen where the HP bar is visible on the screenshot, it doesn't matter if these bars are visible or not, but the unit should be seen at the attack target.

[gfgtdf]

On the contrary, the unit should be seen where the HP bar is visible on the screenshot, it doesn't matter if these bars are visible or not, but the unit should be seen at the attack target.

But the post which says "Screenshot from 1.16.6:" also doesn't show any unit there from what i can see

[Toranks]

Oh that's right. I'm so used to the unit disappearing in 1.17 that I forgot that the original behavior didn't alter the attacking unit until you hit the attack button. So you're probably right.

[Pentarctagon]

1.16:

1.17:

[Pentarctagon]

@mesilliac is this something you'd be willing to look into, given the bisect above by grz0 points to 06254e5 as the cause?

[gfgtdf]

I tested a bit and its really odd, sometimes the unit shows in the original hex, sometimes it doesn't, playing other units next to it seems to have an influence. the bar+orb is always drawn on the new hex

[gfgtdf]

We move the unit here:

wesnoth/src/mouse_events.cpp

Line 938 in 27e5dd5

temporary_unit_mover temp_mover(pc_.get_units(), src, attack_from, itor->move_left);

but appearently this only moves the unit on the board and and the bars/orbs, the units image is somwhow left behind.

The two obvious way to fix it would then be:

1. Don't move the unit there ("there" = that line in the code not that mp location), (instead move the unit somewheere else, so that the unit is only moved during the calculation of the attack stats not all the time while the dialog is shown. )
2. Fix the drawing code to be able to handle this case.

[Pentarctagon]

I'm not sure where else it could be moved from. If it's moved any later then the unit doesn't exist to trigger the attack dialog, and if it's moved any earlier it's before the check to make sure it can actually reach the hex to attack from.

[Pentarctagon]

Something else I noticed - if a unit has a standing animation (such as a drake over water), then the unit flickers in place on its starting hex with the animation continuing as normal even if you can only see the frame for an instant. So I wonder if this is related to dialogs no longer blocking animations on the map, and this is what it always would have done except that previously the attack dialog opening prevented this from being seen.

[Pentarctagon]

Would it make sense to spawn a temporary copy of the unit at the hex for the attack dialog to use rather than use the temporary_unit_mover?

[CelticMinstrel]

Well… I think the temporary_unit_mover should do what it's supposed to…

[Pentarctagon]

Interestingly, if I do just the move logic of temporary_unit_mover:
auto [iter, success] = pc_.get_units().move(src, attack_from);
then after closing the attack dialog the unit's actual location is moved, but has no image:

Then hovering over the new location makes it draw at the old location:

And then hovering over the old location removes it again:

So something is out of sync between where the unit actually is and wherever the rendering code is checking to determine where to draw the unit.

[gfgtdf]

I'm not sure where else it could be moved from. If it's moved any later then the unit doesn't exist to trigger the attack dialog, and if it's moved any earlier it's before the check to make sure it can actually reach the hex to attack from.

Hmm i thought there wodl be once central place where the attack calculation are done but it seems like it split between unit_attack::preshow and the start of mouse_handler::show_attack_dialog. So its not as easy as it though.

Would it make sense to spawn a temporary copy of the unit at the hex for the attack dialog to use rather than use the temporary_unit_mover?

i think its not really a good solutin as it could lead to other problems, lets for example assume that the unit moves one tile, and has a leadership ability affecting adjacent unit of the same level , if the unit it still on the original tileaswell it would give itself leadship (during the attack dialog, not during the actual attack)

So something is out of sync between where the unit actually is and wherever the rendering code is checking to determine where to draw the unit.

Yes indeed, it seems like some code is called that updates unit animations to the actual unit position is called during "normal" drawing, but not during drawing while dialogs are open.

[mesilliac]

@mesilliac is this something you'd be willing to look into, given the bisect above by grz0 points to 06254e5 as the cause?

As mentioned elsewhere, I touched unit animation as little as possible, and frankly have no more idea how it works than anybody else.

There were weird problems like this before my changes, and there were slightly fewer weird problems like this after my changes. I could never replicate this particular bug, and i'm not familiar with any of the code that seems to be related.