Lambda timers

[deltamolfar]

• Adds close to glua's lambda timers, which don't execute whole E2 on their execution.
• A bit more flexibility to them (allows to set reps without setting delay, allows to set callback, etc) then in original glua timers.
• Uses seconds instead of miliseconds. (unlike e2 timers)
• Have all default functionality, except for toggle, stop and start (didn't really see how it would be useful for e2 user)

Subjects to discuss/change:

• Having a delay(nf) function -> A lot of players intuitively think that it exist. Well now it does, even if they don't know about timers.
• timerPause/Resume supposed to return a value, but for example - glua's timers just refuse to return me a value that is specified on wiki, or any for that matter.
• Naming a function timerResume instead of timerUnPause. (I just think it's more intuitive).
• Currently timers don't have stop/start functions, as I think that will not be used by end user. Am I wrong? (Their naming is confusing, so I've made timerRestart for their single useful IMHO usage)
• I want to either not delete old timers, or make an constructor object, or make a destruction callback, so players dont get confused as much when their timer exists, and when it doesn't, but not sure about what would be the best way.

[unknao]

* gtimer name. I just couldn't think of better name, and lambdatimer/ltimer/luatimer didn't stick with me. G stands for garry's.

What about nutimer?

[thegrb93]

You able to review this one @Vurv78 ?

[Denneisk]

Ah, you didn't fork from Vurv's PR, or did you and just messed up the history? This appears to be a completely different implementation. Interesting.

[deltamolfar]

Yep, never forked it as I've never found it :D
Tho code is pretty simple, so didn't see a great need to take something existing and build upon

[Denneisk]

From what I know, unless you found another workaround, the E2 lambda needs to be called surrounded in an E2 context, which Vurv accomplished by extending gmod_wire_expression2.Execute in his timer PR. If you call a lambda outside of the context, it'll break some things.

My suggestion would be to copy #2843, which I was intending to do before I took a brief vacation..

[Vurv78]

Unfortunately I think the local changes I had I never ended up pushing into the PR, which are now lost since I switched off windows.

I don't really care how you implement it, but I'd prefer if the API was the same as my PR, which just adds timer overloads that match glua timer.Create.

[DerelictDrone]

From what I know, unless you found another workaround, the E2 lambda needs to be called surrounded in an E2 context, which Vurv accomplished by extending gmod_wire_expression2.Execute in his timer PR. If you call a lambda outside of the context, it'll break some things.

My suggestion would be to copy #2843, which I was intending to do before I took a brief vacation..

I believe the problems that may occur from calling a lambda outside of its context were solved by the ExtCall | UnsafeExtCall methods for the lambdas, which this PR is using.

You pass ExtCall / UnsafeExtCall the context and it'll pcall the lambda and it'll error the context / chip if an error occurs.

[Denneisk]

I believe the problems that may occur from calling a lambda outside of its context were solved by the ExtCall | UnsafeExtCall methods for the lambdas, which this PR is using.

Oh, you're correct. I didn't notice such a change was made.

[deltamolfar]

I'd prefer if the API was the same as my PR, which just adds timer overloads that match glua timer.Create.

While I would love to make timer overloads, I believe there will be misunderstandments from user for such functions as timerRepsLeft(s), timerTimeLeft, etc as work for both lambda and regular timers.

+ I wanted to use seconds, instead of miliseconds

[Denneisk]

There's nothing stopping you from using seconds for the new overloads and deprecating the old functions. Well, I'm not, anyway.

[Vurv78]

I'd prefer if the API was the same as my PR, which just adds timer overloads that match glua timer.Create.

While I would love to make timer overloads, I believe there will be misunderstandments from user for such functions as timerRepsLeft(s), timerTimeLeft, etc as work for both lambda and regular timers.

• I wanted to use seconds, instead of miliseconds

Yes, as denniesk said, you'd be deprecating all of the old timer functions and pushing them onto the new ones regardless. So there shouldn't be any confusion, unless someone is mixing the two, which absolutely shouldn't be done. And yeah the new api would use seconds as to not have another pointless conversion wasting perf in E2.

[deltamolfar]

Redid the naming, made most og timer functions depracted except for getTimers, stopAllTimers and stoptimer. Three of them now works for both timers.

E2 I've used to test it

@strict

TimerSetName = "test_timer"
TimerDelay = 0.5
TimerReps = 5

# Test 1: Create a timer with a specific delay and repetitions.
timer(TimerSetName, TimerDelay, TimerReps, function() {
    print("Timer callback executed for " + TimerSetName, timerRepsLeft(TimerSetName))
})

assert(timerExists(TimerSetName) == 1, "Test 1 Failed: Timer does not exist after creation.")

# Test 2: Verify the timer delay and repetitions.
TimerDelayCheck = timerGetDelay(TimerSetName)
TimerRepsCheck = timerGetReps(TimerSetName)
assert(TimerDelayCheck == TimerDelay, "Test 2 Failed: Timer delay mismatch. Expected " + TimerDelay + " but got " + TimerDelayCheck)
assert(TimerRepsCheck == TimerReps, "Test 2 Failed: Timer repetitions mismatch. Expected " + TimerReps + " but got " + TimerRepsCheck)
print("Test 2: Verified timer delay and repetitions.")

# Test 3: Adjust the delay and repetitions of the timer.
NewDelay = TimerDelay+0.05
NewReps = TimerReps+1
timerAdjust(TimerSetName, NewDelay, NewReps)
AdjustedDelay = timerGetDelay(TimerSetName)
AdjustedReps = timerGetReps(TimerSetName)
assert(AdjustedDelay == NewDelay, "Test 3 Failed: Adjusted delay mismatch. Expected " + NewDelay + " but got " + AdjustedDelay)
assert(AdjustedReps == NewReps, "Test 3 Failed: Adjusted repetitions mismatch. Expected " + NewReps + " but got " + AdjustedReps)
print("Test 3: Adjusted timer delay and repetitions verified. Added +1 rep")

# Test 4: Restart the timer.
timerRestart(TimerSetName)
assert(timerExists(TimerSetName) == 1, "Test 5 Failed: Timer does not exist after restart.")

RestartedDelay = timerGetDelay(TimerSetName)
RestartedReps = timerGetReps(TimerSetName)
assert(RestartedDelay == NewDelay, "Test 3 Failed: Adjusted delay mismatch. Expected " + NewDelay + " but got " + RestartedDelay)
assert(RestartedReps == NewReps, "Test 3 Failed: Adjusted repetitions mismatch. Expected " + NewReps + " but got " + RestartedReps)
print("Test 4: Timer restart verified.")

# Test 5: Test timer pause and resume.
timerPause(TimerSetName)
timerResume(TimerSetName)

print("All tests completed. Manually verify console output for timer execution.")

[deltamolfar]

Seems to be ready for review

[Vurv78]

Need to stress test it with tons of timers. That was what I was blocked on with my PR, iirc, with how I implemented it.

I was also trying to make the original timers less laggy & limiting them. Could be misremembering.

Would be nice if the integration tests could run timers, too, maybe not too hard to extend but not entirely necessary here.

[deltamolfar]

Came around to a problem.

for(I=1, 100){
    timer("test"+I, 0, 0, function(){
        I--
        I++
    })

Will result in same ops count as same code with old timers, but 0 cpu time. Don't seem to find a reason

[Vurv78]

Came around to a problem.

for(I=1, 100){
    timer("test"+I, 0, 0, function(){
        I--
        I++
    })

Will result in same ops count as same code with old timers, but 0 cpu time. Don't seem to find a reason

Hmm. I never looked into the ExtCall function that was added and figured it did more than what it actually does which is just pcalling and erroring the chip entity if it fails.. :\

It needs to run everything the chip does when executing, ie have everything in :Execute() run. I actually did have some progress in the PR for this, I made :Execute take an optional function and arguments to call specifically. So you unwrap the lambda and then pass that to :Execute().

https://github.com/Vurv78/wire/blob/0b2c88eaf43add2ca24eb2c51bc667f7b5979cdc/lua/entities/gmod_wire_expression2/init.lua#L114-L130

Idr if that was all that's needed for proper perf handling though.

[deltamolfar]

Thank you @Vurv78. Thanks to you, now it works.
Regarding a lot of timers - quota just crashes the e2, if there is too many operations going on.

Seems to be ready for testing/review.
So far I've tested:

• Timer removal on e2 removal/crash.
• Same name timer on different e2s.
• A lot of timers.
• Less then tick interval delays.
• Usual timer functionality (e2 code I've sent before)
• Expecting arguments in lambda (covered by Vurv's :Execute implementation)

[DerelictDrone]

Expecting arguments in lambda (covered by Vurv's :Execute implementation)

You could probably convert the two extcall functions to do the chip:Execute instead now since this implementation seems better.

Could also be done as a separate PR if it feels out of scope.

[deltamolfar]

Could also be done as a separate PR if it feels out of scope.

Yeah, separate PR should be made, but nice idea.

[deltamolfar]

@Vurv78 @thegrb93 PR is stale for a week. Could we set it going onward?

(Want to rewrite everything I have with lambda timers, and am kinda stuck in limbo rn, as don't see any point making anything to having to rewrite it afterwards :D)

[Vurv78]

@Vurv78 @thegrb93 PR is stale for a week. Could we set it going onward?

(Want to rewrite everything I have with lambda timers, and am kinda stuck in limbo rn, as don't see any point making anything to having to rewrite it afterwards :D)

If you were feeling really motivated you could write this api in pure E2. I did that some time ago albeit with string calls (probably posted in the discord somewhere). Just have a library you include at the top that uses the traditional timer() underneath, calling the lambdas, and then exit() so the rest of the code doesn't run.

[deltamolfar]

Yeah, I thought about that. Didn't think of the exit() part tho, so I've scrapped it. Oh well lol

[thegrb93]

Will wait for Vurv to review

[Vurv78]

not a real review cause I can't test the actual code, but api is fine to me

[Vurv78]

no c comments

also this is good but i'd prefer emmylua annotations