added method to cancel screen waitFor #242
Conversation
| @@ -117,6 +122,12 @@ export class Screen { | |||
|
|
|||
| return new Promise<Region>(async (resolve, reject) => { | |||
| try { | |||
| const onCancel = (cancelTemplateImageFileName: string) => { | |||
| if (templateImageFilename === cancelTemplateImageFileName) { | |||
| reject(`Cancelled searching for ${cancelTemplateImageFileName}`); | |||
There was a problem hiding this comment.
@s1hofmann @svettwer Not sure, that whether we should reject or resolve it here?
There was a problem hiding this comment.
Hi @afzaalahmad 👋
I just took a look at your PR but I have to admit that it doesn't actually adress the problem you described.
Your PR only addresses Screen#find and reject early on a cancel event.
However, Screen#waitFor would just recognise this early rejection as a single failure and simply retry.
The actual thing we want to cancel is the timeout.
| @@ -131,8 +142,10 @@ export class Screen { | |||
| matchResult.location.height | |||
| ) | |||
| if (this.config.autoHighlight) { | |||
| this.eventEmitter.removeListener('cancelWaitFor', onCancel); | |||
There was a problem hiding this comment.
Listeners are not removed on error paths and thus pile up over time
| resolve(this.highlight(resultRegion)); | ||
| } else { | ||
| this.eventEmitter.removeListener('cancelWaitFor', onCancel); |
There was a problem hiding this comment.
Listeners are not removed on error paths and thus pile up over time
| /** | ||
| * @param templateImageFilename Filename of the template image to cancel waitFor of, relative to {@link Screen.config.resourceDirectory} | ||
| */ | ||
| public async cancelWaitFor( |
There was a problem hiding this comment.
To be honest, I don't think having explicit methods to cancel actions is something I want.
Having an explicit cancel method for each action with timeouts sound like too much overhead for me.
It adds a method which will use
EventEmitterclass to cancel respectivescreen.waitForon the basis of templateImageFileName.