[Vanilla Bugfix] Fix Buggy Spotlight Behavior w/Event 35 - Enemy In Spotlight

[krnyoshi]

Summary

This PR fixes several spotlight trigger issues affecting Enemy In Spotlight (Event 35). This is much more noticeable when multiple light towers share the same tag.

It changes spotlight Follow handling so only the tower that actually detected the enemy begins tracking it, prevents a Follow-to-Sweep transition from immediately re-detecting in the same frame, and stops a latched single-event Event 35 trigger from firing again on unrelated event polls.

Reviewers should expect more stable spotlight behavior with shared tags. There shouldn't be unrelated tower spotlight position resetting or spazzing (when under repeated fire), and there shouldn't be anymore repeated trigger spam after a prior detection. Event 35’s existing persistent/multi-event latching behavior remains intact.

Video of the Issue:

issue.mp4

Behavior and compatibility

This is a Vanilla bug fix.
Fixes incorrect Enemy In Spotlight (Event 35) behavior affecting shared spotlight tags and persistent triggers.

Validation

Environment: Windows 10, Visual Studio 2022, OpenTS x86 Release build.
Build: cmake --build build --config Release
Test configuration: Two spotlight towers sharing the same tag, Event 35 (Enemy In Spotlight), persistence set to Repeat, with Change Spotlight Behavior -> Follow.
Result: Verified that only the detecting spotlight enters Follow, the second spotlight continues sweeping without resetting, Follow returns to Sweep without looping/re-triggering, and subsequent unrelated attacks do not cause repeated Event 35 activations.

Video of the fix:

fix.mp4

Documentation

Checklist

• The change is focused; unrelated mechanical cleanup is separate
• Compatibility effects and any migration are explicit
• Validation distinguishes what passed, failed, and was not run
• No prohibited assets, binaries, SDKs, credentials, or generated output are included

[github-actions]

Development builds of 647b5f1:

• opents-Release-2bb25ec
• opents-Debug-2bb25ec

The links work without a GitHub account. Artifacts expire after 90 days, and this comment follows the latest successful build.

[ZivDero]

Changelog note, please

[ZivDero]

Right so, I am not well-versed in triggers, unfortunately, but something an AI review found.

The Follow restriction keys on object, which is NULL on most springing paths

taction.cpp:2067 returns false when object is NULL or not a building, and that return(false) also skips the fallback loop, so there's no degraded path — the action just does nothing.

That's fine for the detection path, where blight.cpp passes owner_building. But it isn't the only way a spotlight trigger springs:

• A multi-event trigger, e.g. [Enemy In Spotlight, Elapsed Time 10s]. The spotlight event latches on detection and the trigger actually springs on the TEVENT_TIME poll from logic.cpp:271, which calls Spring(TEVENT_TIME) with the default object = NULL.
• TAction_FORCE_TRIGGER, which calls Triggers[index]->Spring(NULL, CELL_NONE). "Force trigger to spring" on a spotlight trigger with a Follow action silently becomes a no-op.
• The house-tag TEVENT_ATTACKED poll at house.cpp:1779, also object = NULL.

In all three, Follow used to reach every tagged spotlight and now reaches none. Since IsDetectionSource is only ever true inside the two Spring calls in BuildingLightClass::AI, I think the fix wants to be "restrict to the source when there is one, otherwise behave as before" rather than an early return — probably as an extra predicate inside the existing loop (!restrict_to_source || ptr == object) instead of a parallel branch, which would also drop the duplicated six-condition validity check.

Plus needs docs - 52.md, 35.md, trigger-springing.md, and the chanelog note. Remember to rebase, I bumped the version.

There is also this AI found:

Consideration: could the Should_Spring fix be general?

Not blocking, but worth a decision either way. On a trigger with exactly one event the tripped bit can never do the job it exists for — there's no second event waiting to catch up — so all it can do is make all_sprung true on unrelated polls. That's the bug, and it applies to all 28 events satisfying Is_Time_Based() && Is_To_Flag_As_Tripped(), not just Event 35: Spied upon, Discovered by player, Selected, Pickup crate, Near waypoint, the damage-threshold events, the build events, and so on.

Gating the write rather than patching the read would cover all of them:

if (persistent && Class->FirstEvent->Next != NULL) {
    if (tevent->Is_Time_Based() && tevent->Is_To_Flag_As_Tripped()) {
        Flag_Event_Tripped(index);
    }
}

IsTripped has exactly one consumer (Is_Event_Tripped, called only from this loop; the other references are Compute_CRC and Serialize), and outcomes for the spotlight case come out identical to what you have — including suppressing the ENEMY_IN_SPOTLIGHT_REPEATING double-fire.

The counter-argument is real though: that changes behavior for 28 events at once, and some existing map may accidentally depend on the current latch. Staying narrow is defensible — I'd just want it to be an explicit decision, with a comment saying the latch is meaningless on a single-event trigger and that the clause is deliberately scoped to Event 35 for now. In that case tevent->Next == NULL && index == 0 reads off the loop variable already in hand; Class->FirstEvent->Next == NULL makes the reader first prove that tevent == Class->FirstEvent.

But I frankly again, don't understand triggers well enough to understand what in the world this implies.