Skip to content

ClosureFrames: one shared trigger again; per-pair triggers lost real clicks #514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Frotty merged 1 commit into from
Oct 21, 2026
Merged

ClosureFrames: one shared trigger again; per-pair triggers lost real clicks #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 24 additions & 77 deletions wurst/closures/ClosureFrames.wurst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,19 +36,16 @@ import Framehandle

constant FRAME_MAP = keyedMapCreate()

// Frame handle id -> event type handle id -> the trigger which holds that event of that frame, and how many
// listeners use it. Each pair has a trigger of its own, so that the last listener going away can retire it: WC3 has
// no per-event unregister. A pair which gets listeners again, or a new frame with the reused handle id of a
// destroyed one, is then registered on a fresh trigger, and never twice on a live one.
constant FRAME_EVENT_TRIGGERS = InitHashtable()
constant FRAME_EVENT_LISTENERS = InitHashtable()

// Triggers whose last listener went away. They are destroyed only while no frame event is being dispatched, since
// one of them may be the trigger whose action is running (possibly suspended in a wait): by the next listener
// change or the next frame event after the dispatches end. Until then onEvent ignores their fires, as they are no
// longer the pair's trigger.
trigger array retiredTriggers
var retiredTriggerCount = 0
// Every frame event is registered on this ONE trigger, once per (frame, event) pair. A trigger per pair, created on
// demand, lost real mouse clicks: in Castle Fight a button made inside another button's click handler (a dialog
// opening the next one) never received its clicks, while BlzFrameClick on the same button still reached it. Going
// back to the shared trigger fixed it.
constant FRAMEHANDLE_TRIGGER = CreateTrigger()..addAction(function onEvent)

// Frame handle id -> event type handle id -> true once the pair is registered on FRAMEHANDLE_TRIGGER. WC3 has no
// per-event unregister, so a pair stays registered after its last listener goes away; registering it again would
// make every click fire twice.
constant FRAME_EVENT_REGISTERED = InitHashtable()

// The listener each dispatch in progress steps to next, in a slot the dispatch owns until it ends. Slots are not
// tied to a nesting depth: a listener may wait, and dispatches paused that way resume in any order.
Expand All @@ -58,8 +55,6 @@ int array freeFrameCursors
var freeFrameCursorCount = 0
// How many slots were ever handed out; the ones not in use are on freeFrameCursors.
var frameCursorSlots = 0
// How many dispatches are in progress, paused ones included
var frameDispatches = 0


/* CLOSURES */
Expand Down Expand Up @@ -119,21 +114,15 @@ public function framehandle.onSpriteAnimUpdate(FrameHandleListener listener) ret

function addListenerForFrame(FrameHandleListener listener, framehandle frame, frameeventtype eventType) returns FrameHandleListener
listener.frame = frame
let t = linkListener(listener, frame.getHandleId(), eventType)
if t != null
t.registerFrameEvent(frame, eventType)
if linkListener(listener, frame.getHandleId(), eventType)
FRAMEHANDLE_TRIGGER.registerFrameEvent(frame, eventType)
return listener

/** Puts the listener at the head of the frame's list. Returns the new trigger to register (frame, eventType) on when
the pair had no listener yet, or null when its trigger already holds the event.

A pair is registered only ONCE per trigger. onEvent() already fans a single trigger fire out to every listener in
the list, so re-registering on each added listener makes the trigger fire once PER registration: a frame with two
CONTROL_CLICK listeners then runs its click handler twice (which silently toggled controls back, e.g. a
checkbox that never unchecks). */
function linkListener(FrameHandleListener listener, int id, frameeventtype eventType) returns trigger
if frameDispatches == 0
destroyRetiredTriggers()
/** Puts the listener at the head of the frame's list. Returns whether (frame, eventType) still has to be registered
on FRAMEHANDLE_TRIGGER: only the first listener the pair ever gets. onEvent() fans a single fire out to every
listener in the list, so a pair registered twice would run each of its handlers twice per click (which silently
toggled controls back, e.g. a checkbox that never unchecks). */
function linkListener(FrameHandleListener listener, int id, frameeventtype eventType) returns bool
listener.frameId = id
listener.eventType = eventType
let first = keyedMapGet(FRAME_MAP, id)
Expand All @@ -143,64 +132,28 @@ function linkListener(FrameHandleListener listener, int id, frameeventtype event
keyedMapPut(FRAME_MAP, id, listener)

let eventId = eventType.getHandleId()
let listeners = LoadInteger(FRAME_EVENT_LISTENERS, id, eventId)
SaveInteger(FRAME_EVENT_LISTENERS, id, eventId, listeners + 1)
if listeners > 0
return null
let t = CreateTrigger()..addAction(function onEvent)
SaveTriggerHandle(FRAME_EVENT_TRIGGERS, id, eventId, t)
return t

/** Called when a listener of the pair goes away: the last one retires the pair's trigger. */
function unlinkEvent(int id, frameeventtype eventType)
if eventType == null
return
let eventId = eventType.getHandleId()
let listeners = LoadInteger(FRAME_EVENT_LISTENERS, id, eventId) - 1
if listeners > 0
SaveInteger(FRAME_EVENT_LISTENERS, id, eventId, listeners)
return
RemoveSavedInteger(FRAME_EVENT_LISTENERS, id, eventId)
let t = LoadTriggerHandle(FRAME_EVENT_TRIGGERS, id, eventId)
RemoveSavedHandle(FRAME_EVENT_TRIGGERS, id, eventId)
if t != null
retiredTriggers[retiredTriggerCount] = t
retiredTriggerCount++
if frameDispatches == 0
destroyRetiredTriggers()

function destroyRetiredTriggers()
for i = 0 to retiredTriggerCount - 1
DestroyTrigger(retiredTriggers[i])
retiredTriggers[i] = null
retiredTriggerCount = 0
if LoadBoolean(FRAME_EVENT_REGISTERED, id, eventId)
return false
Comment on lines +135 to +136

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Re-register reused Jass frame handles

On the Jass target, frame handle IDs are reused after BlzDestroyFrame, but this entry is never removed when the old frame's last listener is destroyed. If a later frame receives the same ID and event type, this branch returns false, addListenerForFrame skips registerFrameEvent, and the new frame's exported onClick/other event listeners never receive real events. The previous fresh-trigger behavior explicitly handled this case, so the shared-trigger solution must distinguish a new frame lifetime rather than permanently treating the numeric pair as registered.

AGENTS.md reference: AGENTS.md:L12-L12

Useful? React with 👍 / 👎.

SaveBoolean(FRAME_EVENT_REGISTERED, id, eventId, true)
return true


/* ON EVENT */

function onEvent()
let id = BlzGetTriggerFrame().getHandleId()
let eventType = BlzGetTriggerFrameEvent()
// A retired trigger may still fire until it is destroyed; only the pair's current trigger dispatches.
if GetTriggeringTrigger() == LoadTriggerHandle(FRAME_EVENT_TRIGGERS, id, eventType.getHandleId())
dispatchFrameEvent(id, eventType)
dispatchFrameEvent(BlzGetTriggerFrame().getHandleId(), BlzGetTriggerFrameEvent())

/** Runs the listeners of the frame for the event, newest first. A listener may destroy itself or any other listener:
the successor is published in frameCursor before each call, and the destructor advances it when it is the one
destroyed. There is one cursor per dispatch, as a listener may raise frame events itself or wait. */
function dispatchFrameEvent(int id, frameeventtype eventType)
// Triggers retired by an earlier dispatch go now, unless a dispatch is still in progress. The trigger which
// runs this one is not among them: onEvent dispatches only for a pair's current trigger.
if frameDispatches == 0
destroyRetiredTriggers()
var slot = frameCursorSlots
if freeFrameCursorCount > 0
freeFrameCursorCount--
slot = freeFrameCursors[freeFrameCursorCount]
else
frameCursorSlots++
frameCursorUsed[slot] = true
frameDispatches++
var listener = keyedMapGet(FRAME_MAP, id)
while listener != null
frameCursor[slot] = listener.next
Expand All @@ -211,7 +164,6 @@ function dispatchFrameEvent(int id, frameeventtype eventType)
frameCursorUsed[slot] = false
freeFrameCursors[freeFrameCursorCount] = slot
freeFrameCursorCount++
frameDispatches--


/* FRAME HANDLE LISTENER */
Expand Down Expand Up @@ -244,21 +196,16 @@ public abstract class FrameHandleListener
next.prev = prev
next = null
prev = null
unlinkEvent(id, eventType)

/** Links a listener to a frame id and event without a frame, for tests: the frame event itself is not
registered. Returns whether the pair needed a new trigger. */
registered. Returns whether the pair still had to be registered. */
protected static function linkForTest(thistype listener, int frameId, frameeventtype eventType) returns bool
return linkListener(listener, frameId, eventType) != null
return linkListener(listener, frameId, eventType)

/** Runs the listeners of a frame id for an event, as a fire of its trigger does. For tests. */
protected static function dispatchForTest(int frameId, frameeventtype eventType)
dispatchFrameEvent(frameId, eventType)

/** How many retired triggers wait for no frame event to be running. For tests. */
protected static function retiredTriggersForTest() returns int
return retiredTriggerCount

class ReleaseKeyboardFocusFrameListener extends FrameHandleListener

override function onEvent()
Expand Down
61 changes: 31 additions & 30 deletions wurst/closures/ClosureFramesTests.wurst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,13 @@ class TestFrameListener extends FrameHandleListener
static function dispatch(int frameId, frameeventtype eventType)
dispatchForTest(frameId, eventType)

static function retired() returns int
return retiredTriggersForTest()

function newTestListener(int which, int destroyWhich, int frameId) returns TestFrameListener
let l = new TestFrameListener(which, destroyWhich)
TestFrameListener.made[which] = l
TestFrameListener.link(l, frameId, FRAMEEVENT_CONTROL_CLICK)
return l

@Test function aFrameEventIsRegisteredOncePerTrigger()
@Test function aFrameEventIsRegisteredOnceOnTheSharedTrigger()
let first = new TestFrameListener(0, -1)
TestFrameListener.link(first, 910001, FRAMEEVENT_CONTROL_CLICK).assertTrue()
let second = new TestFrameListener(1, -1)
Expand All @@ -48,39 +45,43 @@ function newTestListener(int which, int destroyWhich, int frameId) returns TestF
destroy other
destroy otherFrame

@Test function aPairWhoseListenersWentAwayIsRegisteredAgainOnAFreshTrigger()
@Test function aPairWhoseListenersWentAwayStaysRegistered()
let first = new TestFrameListener(0, -1)
TestFrameListener.link(first, 910005, FRAMEEVENT_CONTROL_CLICK).assertTrue()
destroy first
// The old trigger is retired and destroyed, so a new frame reusing the handle id gets its event registered.
TestFrameListener.retired().assertEquals(0)
let reused = new TestFrameListener(1, -1)
TestFrameListener.link(reused, 910005, FRAMEEVENT_CONTROL_CLICK).assertTrue()
destroy reused

@Test function aTriggerRetiredDuringItsDispatchIsDestroyedAfterwards()
// WC3 has no per-event unregister: the pair is still on the shared trigger, and registering it again would run
// every handler of the frame twice per click.
let again = new TestFrameListener(1, -1)
TestFrameListener.link(again, 910005, FRAMEEVENT_CONTROL_CLICK).assertFalse()
TestFrameListener.runs = 0
// The only listener of the pair destroys itself while its trigger runs.
newTestListener(0, 0, 910006)
TestFrameListener.dispatch(910006, FRAMEEVENT_CONTROL_CLICK)
TestFrameListener.dispatch(910005, FRAMEEVENT_CONTROL_CLICK)
TestFrameListener.runs.assertEquals(1)
TestFrameListener.retired().assertEquals(1)
let next = new TestFrameListener(1, -1)
TestFrameListener.link(next, 910006, FRAMEEVENT_CONTROL_CLICK).assertTrue()
TestFrameListener.retired().assertEquals(0)
destroy next
destroy again

class OpensNextFrameListener extends FrameHandleListener
static TestFrameListener opened = null

override function onEvent()
opened = new TestFrameListener(7, -1)
TestFrameListener.link(opened, 910010, FRAMEEVENT_CONTROL_CLICK)

@Test function aRetiredTriggerIsDestroyedByTheNextFrameEventWithoutAnyListenerChange()
static function link(OpensNextFrameListener listener, int frameId, frameeventtype eventType) returns bool
return linkForTest(listener, frameId, eventType)

/** A dialog's button opening the next dialog: the new button's listener is linked while the first click is still
being dispatched, and its own click must reach it. Before the shared trigger, such a button never got its real
clicks. */
@Test function aListenerLinkedDuringADispatchGetsItsOwnFrameEvents()
TestFrameListener.runs = 0
newTestListener(0, 0, 910007)
newTestListener(1, -1, 910008)
TestFrameListener.dispatch(910007, FRAMEEVENT_CONTROL_CLICK)
TestFrameListener.retired().assertEquals(1)
// Any later frame event, with no listener added or removed, drains it.
TestFrameListener.dispatch(910008, FRAMEEVENT_CONTROL_CLICK)
TestFrameListener.retired().assertEquals(0)
TestFrameListener.runs.assertEquals(2)
destroy TestFrameListener.made[1]
let opener = new OpensNextFrameListener()
OpensNextFrameListener.link(opener, 910009, FRAMEEVENT_CONTROL_CLICK).assertTrue()
TestFrameListener.dispatch(910009, FRAMEEVENT_CONTROL_CLICK)
(OpensNextFrameListener.opened != null).assertTrue()
TestFrameListener.dispatch(910010, FRAMEEVENT_CONTROL_CLICK)
TestFrameListener.runs.assertEquals(1)
TestFrameListener.order[0].assertEquals(7)
destroy OpensNextFrameListener.opened
destroy opener

@Test function frameDispatchSurvivesAListenerDestroyingItsNextSibling()
TestFrameListener.runs = 0
Expand Down
Loading