Skip to content
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.

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

New pause command #155

Closed
bob2517 opened this issue May 29, 2021 · 36 comments
Closed

New pause command #155

bob2517 opened this issue May 29, 2021 · 36 comments
Assignees
Labels
docs done The documentation has been fully completed for this issue done on branch This issue has been committed to the latest branch, and will get merged with the next release enhancement New feature or request

Comments

@bob2517
Copy link
Member

bob2517 commented May 29, 2021

With the synchronous "await" evolution for 2.6.0, in practice, chaining regular action commands using await can get a little bit confusing.

So I'm creating a pause command, which separates the delay into an actual command.

Essentially, the test code looks easier to read with a pause command, like:

div:draw {
    background-color: blue;
    pause: 1s;
    background-color: green;
    pause: 1s;
    background-color: blue;
}

when compared to:

div:draw {
    background-color: blue;
    background-color: green after 1s await;
    background-color: blue after 1s;
}

which does the same thing, but requires more brainpower to work out what's going on, even if it is quicker to type.

This can also be written like:

div:draw {
    background-color: blue, green after 1s await, blue after 1s;
}

So the await command has power, but can arguably be a bit harder to read.

The await option therefore could make more sense for ajax-type commands. But it is handy for one-off delays, so I'm keeping it in the core.

The pause command will internally be asynchronous, like await, but from a developer viewpoint will be synchronous within a specific event declaration. Pausing shouldn't require the whole universe to stop, like in JavaScript, when it is used, even if JS uses the excuse of being single-threaded. The monitoring of time is done on a single entity or particle anyway, and so doing it this way more mirrors the way time is used in the physical universe, and so more accurately matches developer intent when they want to pause. Ie., when you want to stop moving your hand, the rest of your body doesn't have to stop. Having synchronous pauses within a specific event/element time stream therefore makes sense.

A step further on from this would be a way to asynchronously chain the same action command, so the same action command can run multiple times being chained, but allow other multiple chained commands to run at same time in the same event. Eg. color can also change at the same time something changes position, within the same event. It can be done with the existing syntax using "after", but a proper syntax for it would be cool. Maybe like a "chain" option which chains the same command if it is followed by the same command, instead of "await". Would need to work that out though. Like:

div:draw {
    background-color: blue, green after 1s chain, blue after 1s;
    color: green, blue after 1s chain, green after 1s;

In this case both commands would start at the same time, but each command is chained, something like that. Still not easy to read like that though...

I'll get this evolution wrapped up first and then ponder that last one for a future release.

@bob2517 bob2517 self-assigned this May 29, 2021
@bob2517 bob2517 added the enhancement New feature or request label May 29, 2021
@bob2517 bob2517 added this to the 2.6.0 release milestone May 29, 2021
@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

This is almost done, BTW, but I found an error when used with variables and loops, as internally the code isn't in the right place. So this weekend I'm going to jig things around a little bit to sort that out.

It would be good to have pause-transition/resume-transition and pause-animation/resume-animation commands at some point, but that's a completely different thing and nothing to do with the action of the pause command or the await option...

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

Doing this has given me an idea for restarting a specific event sequence rather than re-triggering the outer event which applies to all declaration for an event. Gonna pend this until I think of a use case. But if anyone needs it it should be easy enough to implement after doing this issue.

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

Gah. Need to change implementation method completely. There's only one sensible way to get this working with loops, and that's never to leave the loop at all, which isn't the way I've set it up.

Re-writing this now. Just need to work out how the ajax commands will work with this...

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

The only way I can do this sensibly is by using delay promises after direct action command calls, and that will only work if each event has its own event "thread" by using setTimeout. This will break stop-event-propagation and stop-immediate-event-propagation commands, so I'm going to set it up so that these new threads will only be created if await or pause commands are in the event. That way those commands can still be useful and existing configs won't suddenly do something different. It doesn't change the "after" option on action commands, as that gave a specific action command its own thread in much the same way, but at a lower level.

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

Just need to work out how to handle ajax, as that isn't going to work using this new method. The last method was fine but it didn't work with loops. I can't leave any potential loop... I think I may have to have an optional async await on the XHR call itself and change the way ajax works. I think that's the way to go. I'll get the rest done and tackle ajax commands after that.

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

Meh. Doesn't look like that's going to work for whatever reason. I'm probably getting too complicated - gonna simplify my thinking a little bit and try something more basic...

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

Double meh. Async/await isn't ES6...

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

Experimenting with using a promise in loop. Looks like it could work maybe.

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

Yeah, that's not going to work either. Looks like I'm going to have to write a manual iterator. ES7 sounds promising though in about 10 years when I can actually use it without Babel.

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

The good thing about an iterator that I have to write myself though is that it will work easily with ajax-type commands and I can resume iterations anywhere in the core that I like, so I suppose that is the plus side.

@bob2517
Copy link
Member Author

bob2517 commented May 29, 2021

I need to do some serious surgery on the core for this to work. All the for loops below the event level need to stop being for loops and become manual iterators of some kind, with manual "callbacks". That's the only way I can get asynchronicity to the level I need for everything to work correctly with loops and variables. If I didn't have to cater for loops and variables, and even ES6 (i could have used async/await) then this problem wouldn't have come up. I'm not going to add Babel at this point, as it always adds on bloat.

It's either that or get a sensible polyfill for async/await, but I'm doubtful now that a good enough solution is available for what I need.

So tomorrow, I'm going to convert all the for loops starting from the top level function above the loops, so that each can be manually iterated with callbacks, just on the core as it stands live at the moment. I'll get that working. Then add in the settimeouts pointing to those callbacks to prompt the next iteration of whatever, and that should do the job. Then the next loop iterations can be run from anywhere. I might set up a generic iterator function of some kind to simplify things a bit - maybe not though.

@bob2517
Copy link
Member Author

bob2517 commented May 30, 2021

Switching strategy again. For simplicity's sake I'm gonna use babel so I can use async/await. It's not worth rewriting everything for this. Core would get larger if I rewrote anyway. Peeps who need to support anything earlier than 2017 can use the babel version. I'm going to stick with ES6 for the rest, unless I get into a position like this again.

I think async/await should do the job. Will need to convert XHR for it and change the existing callback strategies, but will give this a go. Rewriting is a last resort. I'd rather switch to fetch and have to get a different workaround for future implementations that would have needed XHR than do a rewrite just because of XHR.

I literally had await and pause working properly until I ran into this issue with looping, and it's literally a callback issue keeping this from being simple, so anyways...

@bob2517
Copy link
Member Author

bob2517 commented May 30, 2021

Lol, async/await isn't going to cut the cheese either. I thought you'd get true synchronous and be able to mix async functions with sync functions but it's not possible to keep the event flow intact using it, which was the whole point. I should have known that the event stack would have had to have been affected in order for async to have been implemented. Going back to my original plan of rewriting the core. I'll put the core back to it all working with await and pause as they were prior to the loop fixes and rewrite it piece by from the top down, getting it all to work, probably by removing the loops. Then I'll be able to move the await and pause code to their correct places easily. At least I can get rid of babel too - it added 14kb onto the min core just by using it to support async/await. It's going to add complexity to the core, but it's also going to add flexibility.

@bob2517
Copy link
Member Author

bob2517 commented May 30, 2021

The way I'm doing this: 1) convert loops in all key event flow functions into recursive functions so the loops are totally gone. 2) From recursive functions, break up all the recursions by converting to manual function calls. 3) With manual function calls acting as callbacks, rejig the await/pause code. In theory, with manual callbacks I should retain control of the stack and have as much asynchronous stuff going on as I want. I can't see a better way of doing it and maintaining the event flow and keeping the performance benefits by using as little async code as possible. It's basically just one controlled flow. I just need to make sure I'm passing enough data around to keep everything working. I'm about half way through step 1.

@bob2517
Copy link
Member Author

bob2517 commented May 30, 2021

Step 1 is done, and everything works so far with the core loops gone. Now onto step 2. Need to draw a map of the core before starting this so I can get the flows right.

@bob2517
Copy link
Member Author

bob2517 commented May 31, 2021

Use some sort of resumption object containing start positions in iterations and the object each applies to. Then call the highest level above the loop with the original object result set. Skip ones below the steps in the resumption object and when there's a match drill down and repeat. That's probably the most sensible way the event loop structure can be restored to what it was before the await and keep in its dynamic nature. Using an event queue like before won't work as it isn't dynamic. That's probably the reason why there isn't a proper async sleep command in JavaScript as it would require event specific stacks rather than just one queue, and JS isn't primarily an event-driven language. It is, but all the events queue up in the same stack, which is no good for async sleeping per event.

@bob2517
Copy link
Member Author

bob2517 commented May 31, 2021

In fact, the changes made yesterday to remove the loops could be removed, as it isn't necessary to do this if using a resumption object as the loops must start from the beginning anyway and it doesn't matter. Maybe set up a different branch and have a play around. Remove changes if the core is now harder to read since Friday.

@bob2517
Copy link
Member Author

bob2517 commented May 31, 2021

When handling the variable looping resumption, bear in mind that there may have been changes to variables during the await or pause via a different event. The resumption needs to look out for these changes and either skip or break out if a right hand variable no longer matches, or more drastically restore to the original value before the change, or something. Work out which to do when you get to it by using a test case.

@bob2517
Copy link
Member Author

bob2517 commented May 31, 2021

Actually - leave the code split up as it is. It's probably easier for other people to get an idea of how it works, as it now has more defined sections.

Clean it up though at some point - there's room for refactoring the internal objects to make them a bit more sensible.

@bob2517
Copy link
Member Author

bob2517 commented May 31, 2021

So next steps in sequence (I can't be on this all day unfortunately as I've got other stuff going on):

  1. Create the resumption object.
  2. Check the values of the resumption object at the point of await detection, which will be in _handleFunc at the bottom of the chain.
  3. Get the code not performing any more actions once await has been started (it does this already - just check it).
  4. Make sure the variables necessary for resumption in restarting the higher-level loop function are comparable to the initial values and available after the settimeout.
  5. Call the higher-level function.

At this point, the whole event loop should be restarted from the beginning. That should work, but not how we need.

  1. Insert conditional checks at the top of each loop and check if the counter is below what we need - if so, increment and re-call the function. Use a generic comparison function for this.
  2. When it comes to checking the objects... just don't start drilling into the loop until we find the object. We can have a rule that if the paused object gets removed during a loop, the await process stops. Implement that for now. Maybe come back and check later if a different solution is discovered to be wanted. Maybe an option on await could determine what to do if the loop is allowed to continue after an object is removed. Anyway - for now keep looking for the originally paused object in order to continue the loop. This at least gives dynamic action to elements that the pause didn't occur on. It won't work on new elements that got added subsequent to the paused loop, but that's wanted behaviour (look at this - keep original selection or fresh selection? Maybe a fresh selection is better - the only thing is the element position in the selection - elements prior to the paused object will get skipped).
  3. Then handle the loop variables as in the above comment.

That should be all the steps and allow regular settimeouts to pause and resume where the event queue left off at the level high enough to respond dynamically.

@bob2517
Copy link
Member Author

bob2517 commented Jun 1, 2021

Further notes on this to avoid weird bugs coming up later. Points 1 to 5 are done above, so into the meat of the resumption next.

When resuming the event flow, the elements involved in target selector need to remain exactly the same as prior to the pause. Do this by storing each selector set (to be safe, this should be per loop sequence) and using these instead of new element queries. This is so the flow can resume even if the target selector mid pause has been removed. Otherwise the flow would just stop for any remaining target selectors, which wouldn't happen if there was no pause. The same for variables. Variables shouldn't change mid a loop as it isn't good form, but in case they do the original right-hand variable needs to be used in the resumption instead of the dynamic one. Otherwise someone might resume after a pause and get very unexpected results.

Essentially, the resumption needs to be an exact duplicate of the original event. It then doesn't matter if target selectors are no longer there, as the elements will be referenced but just no longer connected to the DOM. Once the correct point has been reached in the event flow to resume, everything can be put back to normal so it works dynamically from that point as would be expected.

@bob2517
Copy link
Member Author

bob2517 commented Jun 1, 2021

Resumption points to detect during the loop: target selector, the loop(s) reference (loopRef), unique action command ID from the config (intID). All those are required to spot the correct point paused.

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

This is pretty much done. Just need to write some more tests, etc. Might do a commit later on today. The above method works and seem to give a true synchronous effect (which is no mean feat considering that JavaScript doesn't handle separate event stacks natively). But more complex tests are definitely needed just to be on the safe side before release.

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

Example syntax - this test ran with 4 identical square spans, each working simultaneously in isolation:

span:draw {
    pause: 1s;	
    background-color: #{$RANDHEX6};
    pause: 1s;	
    border-radius: 50%;
    pause: 1s;	
    background-color: #{$RANDHEX6};
    pause: 1s;	
    border-radius: 0%;
    trigger: draw;
}

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

This also works in much the same way, which pauses after each delayed background change:

span:draw {
    background-color: #{$RANDHEX6} after 1s await, #{$RANDHEX6} after 1s await, #{$RANDHEX6} after 1s await, #{$RANDHEX6} after 1s await, #{$RANDHEX6} after 1s await;
    trigger: draw;
}

As does this, which is the more readable way to use the "await" option:

button:click {
    ajax: "/get-address-data.php" post-pars(id=2) JSON await;
    render-after-end: "<div>{title}<br>{address}</div>";
    ajax: "/get-address-data.php" post-pars(id=3) JSON await;
    render-after-end: "<div>{title}<br>{address}</div>";
}

button:afterAjaxError {
    render-replace: "<p>Error loading address.</p>";
}

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

I'm so happy with this - it's such a relief to finally crack proper non-weird synchronous coding in an asynchronous way. I'm almost tempted to skip the tests. Just kidding. Or am I?

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

This now works using the new method, which is good as it wasn't working with the old method:

span:draw {
    var: test ['bob', 'dave', 'trevor'];
    @each item in test {
        render-before-end: "{item}<br>";
        pause: 1s;
    }
}

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

This works too. Oh yeah baby...

span:draw {
    var: test2 ['hi', 'goodbye'], test ['bob', 'dave', 'trevor'];
    @each item2 in test2 {
        @each item in test {
            render-before-end: "{item2} {item}<br>";
            pause: 1s;
        }
    }
}

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

Just need to update the docs and convert these examples into proper tests.

Marking as done offline... :) :) :) :) :) :) :) :) :) :) :) :) :) :) :) :) :) :)

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

An awesome thing about this method in the core is that it is really easy to set up for any new action command. It just needs a regular settimeout and couple of function calls and that's it.

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

And it will automatically work with the future @for loop syntax and also allowing loops to be around action commands. Which is good too. All the resumption-after-pausing code is right at the bottom at the function level, and everything above it can get as weird as it likes.

@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

Having done this, it really highlights the need for being able to wrap loops around action commands. Like you can't currently do this because the first @each is around an action command - the pause command in this case is never reached internally:

span:draw {
    var: test2 ['hi', 'goodbye'], test ['bob', 'dave', 'trevor'];
    @each item2 in test2 {
        pause: 2s;
        @each item in test {
            render-before-end: "{item2} {item}<br>";
            pause: 250ms;
        }
    }
}

2.6.0 await/pause is quite an achievement in itself though, so I'm going to release 2.6.0 as it is and do the loop wrap-up for 2.7.0.

@bob2517 bob2517 added docs to do Documentation still needs doing before this can get released testing needed More testing is needed before release docs done The documentation has been fully completed for this issue and removed docs to do Documentation still needs doing before this can get released labels Jun 2, 2021
@bob2517
Copy link
Member Author

bob2517 commented Jun 2, 2021

Minor memory leak on this, which doesn't surprise me, so will have that fixed at the weekend when I'm back on this.

@bob2517
Copy link
Member Author

bob2517 commented Jun 6, 2021

Back on this. Pushing for release today!

@bob2517
Copy link
Member Author

bob2517 commented Jun 6, 2021

Been wrestling with tests today. I'm happy with this now. "Await" now has two actions. if "Await" is applied to multiple target selectors, it cycles through each target selector applying the pause as it goes, so you can get a sequenced linear effect across multiple elements. But if applied to something like a draw event, which can run multiple events, then you can get a simultaneous effect. You can also trigger off an event if you need to apply a simultaneous effect to target selectors, or just use the pause command in the original event, which does the same thing, but would allow true sequentialness in the same event if a custom event needs to have something happening after it finishes and the linear effect isn't wanted. So it's pretty flexible now. I'll make the main example clear on the docs site so it demonstrates the difference between the two techniques.

I've optimised the flow in the core today, so performance is as lean as it can be with the current setup. I've added an internal _immediateStop() function, which breaks out from all actions after the current action command, but I've not made it an official command as I can't think of a purpose for it for developers particularly. Would be two seconds to add if anyone asks for it.

I think I'm going to go with the end result I've got now on this - it's pretty good - you can do two things now with await and mix things up depending on the technique that is used, so it looks quite neat. As long as you understand how the event flow works, it all makes sense. It's not like trying to get your head around the CSS grid command though, lol. So I need to update the documentation so that the event flow is clarified (specifically how target selectors are iterated in the core) and see how far I get this evening. I may just push it live tomorrow afternoon though.

@bob2517 bob2517 added done on branch This issue has been committed to the latest branch, and will get merged with the next release and removed done offline testing needed More testing is needed before release labels Jun 7, 2021
@bob2517
Copy link
Member Author

bob2517 commented Jun 7, 2021

Closing - live on 2.6.0.

@bob2517 bob2517 closed this as completed Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs done The documentation has been fully completed for this issue done on branch This issue has been committed to the latest branch, and will get merged with the next release enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant