Delay shroud updates breaks second_unit references for sighted events

[PointMeAtTheDawn]

Game and System Information

• Version: 1.18.0
• OS: Windows

Description of the bug

Messages for second_unit do not fire when delay shroud updates is on and shroud is manually updated, firing the event.

As an example of a mainline breakage, Eastern Invasion S2 "A horde of undead is pursuing us! You should come with us and escape." is missing, so Duduril replies to himself.

Steps to reproduce the behavior

1. Enable shroud updates.
2. Move to a position that WOULD trigger a sighted event that includes a second_unit reference. I reproduced this with a message tag whose speaker was second_unit, unsure if other references would be affected.
3. Manually update shroud / disable shroud updates / end turn - anything that will cause shroud to update and fire the event, uncorrelated to a single unit's movement.
4. Note that the message is not displayed, presumably due to an empty second_unit reference.

Expected behavior

I'm not sure what the right approach is here. We could:

1. Change delayed shroud updates to store a reference of which events WOULD fire when a unit moved - this would have to rollback on undo. This is the cleanest approach in my opinion because it neatly solves the problem in the engine. Unfortunately, I don't have experience with this part of Wesnoth so I'm not the best equipped to pitch in here.
2. Change WML behavior in one of several ways when second_unit references are empty. This...seems poor for a variety of reasons, but listing it here for the sake of argument. Can expand on it if anyone thinks it would have merit.
3. Expect all mainline and UMC content to be updated to handle cases where second_unit is empty (filling in a default, whatever). This seems insanely tedious and unDRY, but again listing it here for the sake of argument.
4. Tell delayed shroud players to suck it up lol get rekt. This would be my second preference if #1 is undesirable for some reason.
5. Change WML behavior of message to accept a fallback speaker. This is kind of a hybrid of #2 and #3 with much of the worst parts of each, but at least the final state isn't as horrendously unDRY as #3. This would be my third preference.

Additional context

No response

[gfgtdf]

Imo dsu should be removed in favor of a something like a simplified planning mode. which would the basicially bahave/look like dsu does currently, (no arrows, only simgle turn, commit all moves on attack) but actually uses planning mode mechanics under the hood, see also

https://github.com/wesnoth/wesnoth/issues/7507

[CelticMinstrel]

I think option 1 is on the right track for how this should be handled, but I'm not sure how feasible it is to actually store a reference to an event. Storing an event's ID is definitely possible, but not all events have an ID.

Just accepting that the second_unit might be empty sometimes is not good. The fallback speaker idea doesn't seem great either.

[PointMeAtTheDawn]

Sorry, I meant storing a reference to the second_unit of said event, so it could be filled in for speaker later.

[CelticMinstrel]

That definitely sounds viable, yes. I think you'd have to account for a possibility that the unit doesn't actually exist anymore by the time the event fires – while I think that's probably not possible with normal gameplay, it would definitely be possible if some event removes it from the map. So it may be necessary to store a copy of the second_unit rather than a reference to it.