leaveRoutine executes twice #245

New issue

Open

opened 2026-02-20 10:15:40 -05:00 by deekerman · 12 comments

deekerman commented 2026-02-20 10:15:40 -05:00

Owner

Copy link

Originally created by @bjalder26 on GitHub (May 22, 2021).

Originally assigned to: @ArnoldSmith86 on GitHub.

I'm just adding this as an issue so it isn't forgotten with everything else being worked on.

leaveRoutine is activated once when a widget is picked up, and again when it leaves the area of the holder. Considering that enterRoutine is activated when the widget is dropped, it would probably be more consistent if leaveRoutine was only activated when the widget is picked up.

This room is set up to demonstrate this issue. The number increments when either leaveRoutine or enterRoutine is activated.

https://virtualtabletop.io/rgp2

Originally created by @bjalder26 on GitHub (May 22, 2021). Originally assigned to: @ArnoldSmith86 on GitHub. I'm just adding this as an issue so it isn't forgotten with everything else being worked on. leaveRoutine is activated once when a widget is picked up, and again when it leaves the area of the holder. Considering that enterRoutine is activated when the widget is dropped, it would probably be more consistent if leaveRoutine was only activated when the widget is picked up. This room is set up to demonstrate this issue. The number increments when either leaveRoutine or enterRoutine is activated. https://virtualtabletop.io/rgp2

deekerman added the

bug

pile related

routine operations

needs legacy server

widget properties

labels 2026-02-20 10:15:40 -05:00

deekerman commented 2026-02-20 10:15:41 -05:00

Author

Owner

Copy link

@bjalder26 commented on GitHub (May 26, 2021):

Maybe this will help figure out exactly where the issue is.

If I move a widget in and out using parent changes, then leaveRoutine only fires once.

If I set onLeave to apply a new parent, then leaveRoutine still fires twice.
If I set onLeave to apply a null parent, then leaveRoutine still fires twice.
When I set onLeave to apply an empty string or zero as the parent, leaveRoutine is triggered when picked up (same time as the parent change), but does not trigger again until I drop it into a holder (even the same holder). If I drop it on the tabletop, then the room freezes, and when I refresh it does not trigger a second time (so I figure some variable is cleared out when the room is refreshed).

If I use either editor to delete or change the parent property, then it will move out of the holder without triggering the leaveRoutine at all.

I barely know anything about JS, but if I were to guess what the issue is, I would guess that in widget.js the leaveRoutine is getting triggered when the parent changes.

And that in holder.js it is getting triggered a second time when the widget is removed from the area of the holder.

I kind of think that deleting lines 990 and 991 in widget.js is probably all that would need to be done to fix this bug.

@bjalder26 commented on GitHub (May 26, 2021): Maybe this will help figure out exactly where the issue is. If I move a widget in and out using parent changes, then leaveRoutine only fires once. If I set onLeave to apply a new parent, then leaveRoutine still fires twice. If I set onLeave to apply a null parent, then leaveRoutine still fires twice. When I set onLeave to apply an empty string or zero as the parent, leaveRoutine is triggered when picked up (same time as the parent change), but does not trigger again until I drop it into a holder (even the same holder). If I drop it on the tabletop, then the room freezes, and when I refresh it does not trigger a second time (so I figure some variable is cleared out when the room is refreshed). If I use either editor to delete or change the parent property, then it will move out of the holder without triggering the leaveRoutine at all. I barely know anything about JS, but if I were to guess what the issue is, I would guess that in widget.js the leaveRoutine is getting triggered when the parent changes.  And that in holder.js it is getting triggered a second time when the widget is removed from the area of the holder.  I kind of think that deleting lines 990 and 991 in widget.js is probably all that would need to be done to fix this bug.

deekerman commented 2026-02-20 10:15:41 -05:00

Author

Owner

Copy link

@robartsd commented on GitHub (May 27, 2021):

I've created a room that logs enterRoutine and leaveRoutine events:
https://virtualtabletop.io/vqib

• adding a card to a holder with a card triggers enterRoutine for the card added, leaveRoutine for the card added, then leaveRoutine for the card that was already in the holder; the pile never triggers the enterRoutine
• adding a pile to an empty holder or removing a pile from a holder triggers the routines for the pile as any other widget
• adding a card to a holder with a pile triggers enterRoutine for the card then leaveRoutine for the card
• adding a pile to a holder with a pile triggers enterRoutine for the pile, enterRoutine for the top card of the pile, leaveRoutine for the top card of the pile, then leaveRoutine for the pile
• removing the a card from a pile in a holder triggers leaveRoutine only once
• removing the second to last card from a pile in a holder triggers leaveRoutine first for the card to remain, followed by enterRoutine for the card to remain, then leaveRoutine for the pile, finally leave routine for the card being removed (only once)

@robartsd commented on GitHub (May 27, 2021): I've created a room that logs enterRoutine and leaveRoutine events: https://virtualtabletop.io/vqib - adding a card to a holder with a card triggers enterRoutine for the card added, leaveRoutine for the card added, then leaveRoutine for the card that was already in the holder; the pile never triggers the enterRoutine - adding a pile to an empty holder or removing a pile from a holder triggers the routines for the pile as any other widget - adding a card to a holder with a pile triggers enterRoutine for the card then leaveRoutine for the card - adding a pile to a holder with a pile triggers enterRoutine for the pile, enterRoutine for the top card of the pile, leaveRoutine for the top card of the pile, then leaveRoutine for the pile - removing the a card from a pile in a holder triggers leaveRoutine only once - removing the second to last card from a pile in a holder triggers leaveRoutine first for the card to remain, followed by enterRoutine for the card to remain, then leaveRoutine for the pile, finally leave routine for the card being removed (only once)

deekerman commented 2026-02-20 10:15:41 -05:00

Author

Owner

Copy link

@bjalder26 commented on GitHub (May 27, 2021):

Yes, connecting this to the issues discussed in PR #435, I think we need a clarification of when onLeave, onEnter, enterRoutine, and leaveRoutine SHOULD be triggered and what they should be applied to.

Specifically
When should those functions trigger? When a widget completely leaves a holder or when it is picked up and changes its parent?
When a pile of cards enters/exits a holder, when should each trigger, and should their effects (being added to the child collection or having a property changed) be applied to the card, pile, or both.
When a pile in a holder is split what should be triggered and what should the effects (being added to the child collection or having a property changed) be for the pile and the cards?

@bjalder26 commented on GitHub (May 27, 2021): Yes, connecting this to the issues discussed in PR #435, I think we need a clarification of when onLeave, onEnter, enterRoutine, and leaveRoutine SHOULD be triggered and what they should be applied to. Specifically When should those functions trigger? When a widget completely leaves a holder or when it is picked up and changes its parent? When a pile of cards enters/exits a holder, when should each trigger, and should their effects (being added to the child collection or having a property changed) be applied to the card, pile, or both. When a pile in a holder is split what should be triggered and what should the effects (being added to the child collection or having a property changed) be for the pile and the cards?

deekerman commented 2026-02-20 10:15:41 -05:00

Author

Owner

Copy link

@bjalder26 commented on GitHub (May 27, 2021):

• adding a card to a holder with a card triggers enterRoutine for the card added, leaveRoutine for the card added, then leaveRoutine for the card that was already in the holder; the pile never triggers the enterRoutine
  Sounds like the card enters (gains the holder as a parent*), then both cards leave (gain the pile as a parent*). I suspect the reason why the pile doesn't trigger the enter routine is because it didn't exist, thus didn't have an "old parent".

• adding a pile to an empty holder or removing a pile from a holder triggers the routines for the pile as any other widget
  So normal/expected behavior I guess.

• adding a card to a holder with a pile triggers enterRoutine for the card then leaveRoutine for the card
  Sounds like the card enters (gains the holder as a parent) then leaves (gains the pile as a parent)

• adding a pile to a holder with a pile triggers enterRoutine for the pile, enterRoutine for the top card of the pile, leaveRoutine for the top card of the pile, then leaveRoutine for the pile
  Sounds like the pile enters (gains the holder as a parent) then leaves (is destroyed). I can't say why only the top card triggers the routines though.

• removing the a card from a pile in a holder triggers leaveRoutine only once

Presumably, because it never had the holder as a parent, but triggers leaveRoutine when it moves out of the area of the holder (dispenseCard).

• removing the second to last card from a pile in a holder triggers leaveRoutine first for the card to remain, followed by enterRoutine for the card to remain, then leaveRoutine for the pile, finally leave routine for the card being removed (only once)

It is odd that the remaining card triggers a leaveRoutine first (maybe it is dispensed?). Next I figure it is gaining the holder as a parent (enter), then the pile leaves (is destroyed) and the card leaves (is dispensed, since it never had the holder as a parent).

I also noticed that recall triggers leaveRoutine for each card.

I kind of think that onLeave and leaveRoutine should probably only be triggered when they actually leave the area of the holder (which I have been assuming is the dispenseCard routine). However, it seems like exceptions will need to be put into place due to the behavior of piles (or people will need to be aware of the behavior, or the behavior of piles will need to be changed). Perhaps what should be done is that piles never trigger any of these routines, but their children ALL do.

The behavior when a pile is destroyed or formed in a holder would probably need to be dealt with too.

@bjalder26 commented on GitHub (May 27, 2021): - adding a card to a holder with a card triggers enterRoutine for the card added, leaveRoutine for the card added, then leaveRoutine for the card that was already in the holder; the pile never triggers the enterRoutine Sounds like the card enters (gains the holder as a parent*), then both cards leave (gain the pile as a parent*). I suspect the reason why the pile doesn't trigger the enter routine is because it didn't exist, thus didn't have an "old parent". - adding a pile to an empty holder or removing a pile from a holder triggers the routines for the pile as any other widget So normal/expected behavior I guess. - adding a card to a holder with a pile triggers enterRoutine for the card then leaveRoutine for the card Sounds like the card enters (gains the holder as a parent) then leaves (gains the pile as a parent) - adding a pile to a holder with a pile triggers enterRoutine for the pile, enterRoutine for the top card of the pile, leaveRoutine for the top card of the pile, then leaveRoutine for the pile Sounds like the pile enters (gains the holder as a parent) then leaves (is destroyed). I can't say why only the top card triggers the routines though. - removing the a card from a pile in a holder triggers leaveRoutine only once Presumably, because it never had the holder as a parent, but triggers leaveRoutine when it moves out of the area of the holder (dispenseCard). - removing the second to last card from a pile in a holder triggers leaveRoutine first for the card to remain, followed by enterRoutine for the card to remain, then leaveRoutine for the pile, finally leave routine for the card being removed (only once) It is odd that the remaining card triggers a leaveRoutine first (maybe it is dispensed?). Next I figure it is gaining the holder as a parent (enter), then the pile leaves (is destroyed) and the card leaves (is dispensed, since it never had the holder as a parent). I also noticed that recall triggers leaveRoutine for each card. I kind of think that onLeave and leaveRoutine should probably only be triggered when they actually leave the area of the holder (which I have been assuming is the dispenseCard routine). However, it seems like exceptions will need to be put into place due to the behavior of piles (or people will need to be aware of the behavior, or the behavior of piles will need to be changed). Perhaps what should be done is that piles never trigger any of these routines, but their children ALL do. The behavior when a pile is destroyed or formed in a holder would probably need to be dealt with too.

deekerman commented 2026-02-20 10:15:41 -05:00

Author

Owner

Copy link

@robartsd commented on GitHub (May 28, 2021):

Perhaps what should be done is that piles never trigger any of these routines, but their children ALL do.

That is a goal of #147

@robartsd commented on GitHub (May 28, 2021): > Perhaps what should be done is that piles never trigger any of these routines, but their children ALL do. That is a goal of #147

deekerman commented 2026-02-20 10:15:41 -05:00

Author

Owner

Copy link

@bjalder26 commented on GitHub (May 28, 2021):

That is a goal of #147

Yeah, personally I think your fix should probably be merged, and they should fix the double triggering of the leaveRoutine (by not having it trigger with a parent change anymore), then deal with piles and cards separately. And the best solution would probably be the goal of that PR.

@bjalder26 commented on GitHub (May 28, 2021): > That is a goal of #147 Yeah, personally I think your fix should probably be merged, and they should fix the double triggering of the leaveRoutine (by not having it trigger with a parent change anymore), then deal with piles and cards separately. And the best solution would probably be the goal of that PR.

deekerman commented 2026-02-20 10:15:42 -05:00

Author

Owner

Copy link

@ArnoldSmith86 commented on GitHub (May 28, 2021):

I've created a room that logs enterRoutine and leaveRoutine events:
https://virtualtabletop.io/vqib

Thank you. Very useful.

@ArnoldSmith86 commented on GitHub (May 28, 2021): > I've created a room that logs enterRoutine and leaveRoutine events: > https://virtualtabletop.io/vqib Thank you. Very useful.

deekerman commented 2026-02-20 10:15:42 -05:00

Author

Owner

Copy link

@robartsd commented on GitHub (May 28, 2021):

Big improvement.

We still have the issue that removing the second to last card from a pile in the holder will trigger the last card leaving then entering the holder as the pile is destroyed before the card being removed leaves the holder.

@robartsd commented on GitHub (May 28, 2021): Big improvement. We still have the issue that removing the second to last card from a pile in the holder will trigger the last card leaving then entering the holder as the pile is destroyed before the card being removed leaves the holder.

deekerman commented 2026-02-20 10:15:42 -05:00

Author

Owner

Copy link

@ArnoldSmith86 commented on GitHub (May 28, 2021):

Can you merge your test room with http://212.47.248.129:3147/pr-test so we can see when onEnter and onLeave trigger and if it is working with all kinds of automation (and with or without stackOffset)?

@ArnoldSmith86 commented on GitHub (May 28, 2021): Can you merge your test room with http://212.47.248.129:3147/pr-test so we can see when `onEnter` and `onLeave` trigger and if it is working with all kinds of automation (and with or without `stackOffset`)?

deekerman commented 2026-02-20 10:15:42 -05:00

Author

Owner

Copy link

@robartsd commented on GitHub (May 28, 2021):

Can you merge your test room with http://212.47.248.129:3147/pr-test so we can see when onEnter and onLeave trigger and if it is working with all kinds of automation (and with or without stackOffset)?

Which PR is that? I've copied my test to the pr-test room for #495.

Never mind, I see the PR-test server uses PR number plus 3000 as the port number. #147 does not have the needed routine code for this yet.

@robartsd commented on GitHub (May 28, 2021): > Can you merge your test room with http://212.47.248.129:3147/pr-test so we can see when `onEnter` and `onLeave` trigger and if it is working with all kinds of automation (and with or without `stackOffset`)? Which PR is that? I've copied my test to the pr-test room for #495. Never mind, I see the PR-test server uses PR number plus 3000 as the port number. #147 does not have the needed routine code for this yet.

deekerman commented 2026-02-20 10:15:42 -05:00

Author

Owner

Copy link

@ArnoldSmith86 commented on GitHub (May 28, 2021):

Yeah, I meant merging the other way around. I'm just saying we need a way to test all the cases and that you don't have to reinvent everything because I built that room for the pile PR.

@ArnoldSmith86 commented on GitHub (May 28, 2021): Yeah, I meant merging the other way around. I'm just saying we need a way to test all the cases and that you don't have to reinvent everything because I built that room for the pile PR.

deekerman commented 2026-02-20 10:15:42 -05:00

Author

Owner

Copy link

@bjalder26 commented on GitHub (Jun 24, 2021):

Summary list of pile-related issues and PRs.

Pile-related issues:
#27 cards being dragged can land on other cards being dragged forming a pile
#53 New menu for piles overlay
#105 Piles don't inherit movable from their children (cards)
#106 Desire to control size of pile handle and pile handle text
#151 Should dice be allowed to stack into piles
#202 Piles flip cards to the next face rather than to a defined face
#403 Problems changing pile type
#439 a card can be dropped onto a moving card
#467 Room freezing when pile formed on moving card?
#480 Pile issues with leaveRoutine
#496 SELECT max counts piles rather than cards
#596 Piles get shuffled if dropTarget is set to {}
#676 When alignChildren is set to false, RECALL sends all but the top card to (0, 0)

Pile-related PRs
pileParent pseudo property - returns parent or parent of parent pile #531
fixed that leaveRoutine would be called twice and that enterRoutine and leaveRoutine are called for a pile and not its children #495
adds css variable to hide pile handle #493
Compatibility with PCIO routines #442

@bjalder26 commented on GitHub (Jun 24, 2021): Summary list of pile-related issues and PRs. Pile-related issues: #27 cards being dragged can land on other cards being dragged forming a pile #53 New menu for piles overlay #105 Piles don't inherit movable from their children (cards) #106 Desire to control size of pile handle and pile handle text #151 Should dice be allowed to stack into piles #202 Piles flip cards to the next face rather than to a defined face #403 Problems changing pile type #439 a card can be dropped onto a moving card #467 Room freezing when pile formed on moving card? #480 Pile issues with leaveRoutine #496 SELECT max counts piles rather than cards #596 Piles get shuffled if dropTarget is set to {} #676 When alignChildren is set to false, RECALL sends all but the top card to (0, 0) Pile-related PRs pileParent pseudo property - returns parent or parent of parent pile #531 fixed that leaveRoutine would be called twice and that enterRoutine and leaveRoutine are called for a pile and not its children #495 adds css variable to hide pile handle #493 Compatibility with PCIO routines #442

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

PL-add-TooManyRoads

dependabot/npm_and_yarn/qs-6.14.2

dependabot/npm_and_yarn/multi-ca49cfd856

dependabot/npm_and_yarn/lodash-4.17.23

feature-tighterConnectionMonitoring

expand-widget-edito

fixes-in-progress-game-title

Consistent-star-icon

feature-filesPanel

feature-add-holderCopyWidget

fix/2552-editor-prefills-routine-parameters

feature-contextPopup

dependabot/npm_and_yarn/node-minify/html-minifier-10.1.1

dependabot/npm_and_yarn/node-minify/uglify-es-10.1.1

feature-assetAlias

feature-freeGames

feature-smartHolder

feature-smarterDragButtons

dependabot/npm_and_yarn/dompurify-3.3.1

feature-userAuthentication

dependabot/npm_and_yarn/body-parser-2.2.1

feature-animate-card-flips

feature-animateCardFlips

feature-PrettierGuestNames

dependabot/npm_and_yarn/js-yaml-3.14.2

feature-surfaceClass

update-card-widgets

feat-widget-tabs-221Pw

feature-ZOOMfunction

fix-addCardsButtonValues

feature-panZoomEditMode

feature-feature-fixDoubleLeaveRoutine

feature-mobileZoom

feature-desktopZoom

feature-fixTestcafeNumSort

feature-playerSpecificInputModal

feature-controlWithoutDragging

feature-fixShowingHiddenWidgetsWithoutJSON

feature-add-regexLabel

feature-add-cardLabel

legacy-compute-modes

feature-zoomy

feature-contextmenu

feature-sixteenByNine

feature-add-stackOffsetAuto

feature-routineEditor

feature-change-quickSaveLocalTime

fix-crashWhenSetToSelf

feature-lockRoom

feature-underCursor

feature-fixTurnDuplicateIndex

feature-add-timerAlertSound

bjalder26-Update-AUDIO-Tutorial

feature-updateOwnerOnRename

feature-workaroundTestcafe

bjalder26-shuffle

feature-timerSound

feature-editUIlayout

PL-update-GuanDan

bjalder26-label-text-update-JSON-editor

bugfix-recursive-CALL

fix-recursiveCALL

feature-smartClone

readmeVideoTests

codespaces

feature-reverseDropTarget

feature-fixPropertyOfArray

fix-pcioImport-moveAll

feature-cachingGet

cursor-player-name

feature-fixTTSimporter

feature-newSeatStyle

feature-fixIfComplaint

feature-feature-returnTurnTarget2

feature-canvasBrushSize

feature-recallOnlyIfNotAlreadyThere

Switch-to-Roboto-Flex

Update-Functions-Inputs

feature-emojiExtension

feature-progressiveWebApp

feature-iOSfullscreenTest

feature-iOSgestureTest

spinner-customization

feature-updateDependencies2

optimize-dom-modifications

feature-arrowKeys

combining-symbols-with-roboto-font

feature-childrenCounterRotate

feature-moveFromSeat

adjust-number-buttons-in-json-editor-to-stop-misclicks

fantasy-wars-update

robartsd/moveToHolder

robartsd/issue1605

feature-recallStuff

widget-dom

css-selector-properties

fix-scrollableLabel

feature-mtgImport

feature-betterReconnect

bjalder26-Break-Function

feature-funWithTrains

feature-splitEdit

feature-fixAssetMigration

public-library-test-PR

editable_tables

test_width

Alphabetical-mistake

feature-jsonEditorBuffer

demo-gameCategories

feature-limitRecursion

recall-optimization

JSON_fix_material

symbols-tag

JSON_move_global_commands

yet-another-game-pieces-PR

demo-stateDetailsSidebar

Holder-dropTarget-highlight

canvas-layer

fix-1116

demo-newGameListSidebar

new-game-piece-assets

demo-newGameListTutorials

feature-updateDependencies

feature-assetBatchUpload

feature-fixSafariScroll

feature-changeSortByCommand

hoverTarget-dist

ArnoldSmith86/issue1072

bjalder26-dropLimit-&-enterRoutine

feature-newGameList

feature-fixStringifiedCollection

ios-volume-fix

feature-parser

bjalder26-numericSort

feature-fixActiveFace

Text-for-holders

bjalder26-textarea-size

Major-UI-Overhaul-Project

feature-fixGameEdit

mousewax-Input-Overlay-Revamp

feature-dropTargetCSS

feature-hover

feature-dropTargetCustomization

color-to-general-widgets

Holder-customization

feature-stop

onHover-for-seats

pile-customization

fix-removedebug

bjalder26-HTML-labels

fix-label-textarea

feature-inRelation

feature-pcioRecall

feature-arrayAssignmentDeepCopy

feature-betterIDs

fonts-pr

bjalder26-JSON-editor-brackets-and-wavy-brackets

bjalder26-Test-Room

bjalder26-Click-4-Test-Room

staging-20210717

feature-MOVEcollections

feature-PCIOcimport

hide-pile-handle

New-assets-list

staging-20210710

feature-fixLeaveRoutine

bjalder26-RichText

staging-20210703

robartsd-inRelaton

legacy-compute-mode-test

stage-seat

staging-20210626

feature-absoluteCoords

staging-20210619

staging-20210612

feature-inheritFrom

random-color-to-players

feature-fixCopyPaste

adds-turn

add-in-to-computation-list

legacyVersion-2

Comments

feature-IFwithDefaultConditionVar

feature-deckManifests

legacyVersion-1

feature-sortSelect

feature-applyVariablesOperations

feature-fixGoButton

enlarge+

feature-pilesAsUI

feature-fixMoveOrder

No results found.

Labels

Clear labels   

PCIO compatibility

  

Validator

  

automated testing

  

bug

  

documentation

  

duplicate

  

editor

  

enhancement

  

enhancement

  

library

  

library

  

maintenance

  

needs legacy server

  

pile related

  

reported client crash

  

routine operations

  

user interface

  

widget properties

No labels

PCIO compatibility

Validator

automated testing

bug

documentation

duplicate

editor

enhancement

enhancement

library

library

maintenance

needs legacy server

pile related

reported client crash

routine operations

user interface

widget properties

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/virtualtabletop#245

No description provided.