Repeats and Delays based on ticks

[InvisibleSoldiers]

At the moment repeats and delays are counted by real time which is a bit strange because we have to consider the time of the game itself.
For example server has host_timescale 0.5 and some delay will be executed in 5 seconds, for now the delay will be really executed in 5 seconds despite host_timescale and only if we count by tick, we will do it in combination with the game and everything will seem right (it will be executed in 10 seconds)

[L'In20Cible]

Implementing a subclass of Delay (perhaps named TickDelay) that handle delays based on frame count rather than time could be an option. Feel free to make a PR that we can base a discussion on.

[InvisibleSoldiers]

I suggest simply replace current Delay to tick based Delay. In general nothing will change and it will get better in some cases, the game has its own time and we need respect that. Even now all happens in @OnTick listener which is executed every tick_interval and delay time can be only t * tick_interval, in other words there is absolutely no point in the current implementation

[Ayuto]

Don't you think saying "there is absolutely no point in the current implementation" is a little bit exaggerated? What if someone really wants to execute something after x real seconds? Think of something that communicates with something else outside of the game. I don't think that's far-fetched.

Nevertheless, it's something I would like to discuss. I would also tend to have it tick based, but having both options (TickDelay and Delay - or simply an optional parameter in the Delay class like tick_based=True) would be great.

[InvisibleSoldiers]

It is really very rare that you need to pass host_timescale and usually it is always 1 so there is no diffrence always will be the same time (except server lags, host_timescale), that is, I think such a rare case does not deserve attention. Time in game usally has higher priority than real time.
Imagine a server has spawnprotection for 5 seconds, and the server freezes for unknown reasons for 5 seconds, with current Delay when server unfreezes, spawn protection already will be disabled. I mean Delay and Repeat frequently used for in game actions and ingame sync(preferably) and not real time dependent.
All I need when writing plugins is just tick based because it more accurate and synced with game. Ok, let's not argue. TickDelay is great.

[Ayuto]

Don't get me wrong. You brought up some valid points and I'm completly with you. Is this something you want to work on or do you want to leave this up to us?

[InvisibleSoldiers]

Don't get me wrong. You brought up some valid points and I'm completly with you. Is this something you want to work on or do you want to leave this up to us?

You mean replace current Delay to tick based only? I can do it myself, no problem.

[Ayuto]

Yes, please create a PR we can further discuss, thanks!

[InvisibleSoldiers]

https://github.com/invisiblesoldiers/timer_delays_vsp

[Ayuto]

Good idea to use the return value of the delay callback to extend/reuse the delay. I like that! However, I was asking/hoping for a pull request (PR) for Source.Python. That would save us a little bit time. To do that, just fork SP, create a branch, make your changes and commit/publish them. Then go to the SP repository and create the PR. After that we can easily review the changes.

From the sample code I see that you would also like to have this implemented in C++. Not sure if it's worth doing so keeping in mind that we would lose functionality with this implementation (AutoUnload, kwargs) and probably won't gain much speed. The Delay implementation in SP isn't really heavy.

[InvisibleSoldiers]

From the sample code I see that you would also like to have this implemented in C++.

No, I just play with VSP for now, this is an example of how to merge together current Delay and Repeat in one general Delay. One day I wanted to do this PR, but only if so, completely change the concept, remove too advanced Repeat because it can be done manually if need so (usually simple enough).
Offtop:
Also reinventing the radio menus in C++, I noticed than if user manually send menuselect command with nonexistent option index, SP does not take this into account and throws an exception and seems to close a menu.
Also you initially showed a good idea to have backroungd menus. I mean if you open new menu while another is active, old menu is added to background menus and can be reopened early, but you implemented it in weird way: when a new menu is opened it does not overlap the old, it is immediately added to background menus and only got active when initial menu will be closed.
And how cherry on the cake can be added new client command menuswap which cycle through active menus (sort of alt+Tab);
If you interested: https://github.com/invisiblesoldiers/radio_menus_vsp

[L'In20Cible]

Syntax: Select all

if (d.exec_time <= global_vars->current_time) {

That won't really work, since that time can be reset at any time (mp_restartgame, new match, etc.) which will cause all your pending delays to fire instantly.

[InvisibleSoldiers]

Syntax: Select all

if (d.exec_time <= global_vars->current_time) {

That won't really work, since that time can be reset at any time (mp_restartgame, new match, etc.) which will cause all your pending delays to fire instantly.

Well, yes and only on level change, but the solution is simple.

[Ayuto]

Also you initially showed a good idea to have backroungd menus. I mean if you open new menu while another is active, old menu is added to background menus and can be reopened early, but you implemented it in weird way: when a new menu is opened it does not overlap the old, it is immediately added to background menus and only got active when initial menu will be closed.

Yes, this is because menus are implemented based on a queue/FIFO (first in, first out). This solved annoying problems when multiple menus are sent e. g. by different plugins.
E. g. you have opened a menu and you are about to make a selection. Right before selecting an option a new menu is sent (e. g. a vote is started, map change, etc.). Then there is a chance you make a selection in the new menu you didn't want to make.
Another example could be if you have a menu that has multiple sub menus and/or allows you to navigate back and forth. In most cases you want to finish handling the menu before receiving another menu.
Though, if you really want to force your new menu to be shown immendiately, there are also ways to accomplish that. I will need to check the code for that.

Nevertheless, the menus package is far away from being perfect. It was a quick shot, because there was a high demand on it. There are multiple things I did that really bother me:

1. I paid too much attention to the compatibility of the ESC menu and radio menu classes. This comes with the loss of functionality and had an impact on the design. Today I would probably implement them differently. They still should have the same base, but the actual implementation should be different. Though, this would mean that plugin authors have to implement two menu systems if they would also like to support HL2DM.
2. The classes should be more OOP (no functions you need to pass to the menu. Instead the functions should be implemented as class methods). A year after the release of the menus package I started thinking about the design again:

There were more things that bother me, but I can't remember right now. I will add them when I remember again.

The good thing of the menus package is that everyone can implement their own menus without any conflicts if the are based on the BaseMenu class.

Oh, I also like the menuswap idea. Maybe menus should also display how many background menus are pending.

[InvisibleSoldiers]

E. g. you have opened a menu and you are about to make a selection. Right before selecting an option a new menu is sent (e. g. a vote is started, map change, etc.). Then there is a chance you make a selection in the new menu you didn't want to make.
Another example could be if you have a menu that has multiple sub menus and/or allows you to navigate back and forth. In most cases you want to finish handling the menu before receiving another menu.

I don't think it's so import but it is good to have this:
open_menu(index, menu, force_active=False) for unexpected menus (vote, questions)
and general be open_menu(index, menu, force_active=True) by default.

I would also like to not have a separate close menu callbacks and invisible zero option because it is individually, some menus may just not want to close or want to have close option on another button for convenience. (not 0 and 3 for example if it's comfortable)
I mean, it can also be programmed in the usual select_callback with None return saying that the menu is closed.

[Ayuto]

I don't think it's so import but it is good to have this:
open_menu(index, menu, force_active=False) for unexpected menus (vote, questions)
and general be open_menu(index, menu, force_active=True) by default.

I would actually do it vice-versa. The programmer should explicitly set force_active to True if he has such an exceptional menu that needs to be displayed immediately and interrupt the user's current menu.

I would also like to not have a separate close menu callbacks and invisible zero option because it is individually, some menus may just not want to close or want to have close option on another button for convenience. (not 0 and 3 for example if it's comfortable)
I mean, it can also be programmed in the usual select_callback with None return saying that the menu is closed.

It's not the menu that wants his close button at a different place, but the plugin author. However, different plugin authors have different preferences and that would result in a mess. So, a future design should still unify the look and feel, but maybe also offer possibilities to customize the menu more than currently is possible (or at least make easier). But I agree: a separate callback is not necessary. I also agree on the unclosable menus. I think they can only be implemented by returning the same menu in the select callback, currently.