critical problem in revscriptsys globalevent think #3674
Conversation
When we create a `globalevent` of type `think` and we forget `globalevent:interval(milliseconds)` before `globalevent:register()`, the script is registered with the interval 0. As this is not checked before, no warning is given and the server runs the script non-stop causing 100% cpu. With this check, we guarantee that the script will not be registered if it does not have a set interval.
The bug can be tested with this simple code:
```
local globalEvent = GlobalEvent('bugThink')
function globalEvent.onTime()
print('CPU says: "stop, it hurts!"')
return true
end
-- globalEvent:interval(1000)
globalEvent:register()
```
EPuncker
requested review from
DSpeichert,
EvilHero90,
nekiro,
ranisalt and
yamaken93
|
In my case only 7%~10% of the CPU, and also your code example is wrong ;), is onThink, not onTime, I do not know what others think, but if this does not block the server or make it slow, it should continue like this |
I have tested with a game dedicated server with some players online and I guarantee it's not a bad machine. If you need scripts to run at 0ms, you can set it to 1ms and good luck with your CPU percentage, but it has to be warned and not registered if the interval isn't changed in the script. edit: thanks for telling me about the code example, i replaced to onThink! |
|
Maybe a warning is not a bad idea, we still have a lot of lower priority warnings elsewhere, so why not here? even so I still think that people should know that an onThink event carries with it a manually defined interval, in the best of cases it is better to leave a default value of one second |
I consider it critical because the script is registered with interval 0 without the programmer knowing, and depending on the scope of the function, the server will explode. I say this because you saw, with a simple print you got 10% cpu, now imagine more expensive scopes and with many players online. Bad situation because the programmer will not know whats happen. (I saw it happen in the pratictice) Setting a default value sounds good, but I still prefer that the script not register and display a warning. |
|
So in XML we can declare intervals of 0, but in Revscript not why is it forbidden. I think it is as simple as changing the type of the interval number to int64_t and that the default value is -1, then the events with intervals less than 0, cannot be registered, You could also add this warning in xml, and then everything would be consistent and familiar! If this is merged, this would happen: <globalevent name="example" interval="0" script="example.lua"/> -- work perfectly <3!
globalevent:interval(0)
globalevent:register() -- Error!!! can't register :Din my opinion it seems quite silly that something works differently if they are supposed to be the same I am not rejecting this PR, I just ask you to find some way to make it work the same for XML and Revscript, the warning seems good to me, since xml also has one. The truth is, you don't have to listen to me, you can expect the opinion of someone who agrees with you and can support the push of this PR. |
I would consider using XML deprecated at this point, everyone should start moving forward to Lua. |
|
I want to set an interval of zero :( |
Then don't use an interval, just use a while true loop |
|
This is good solution of course. Software should help prevent such unexpected misbehavior where possible, with informing the developer about the missing option. Any information is better, than no information. +1 |
When we create a `globalevent` of type `think` and we forget `globalevent:interval(milliseconds)` before `globalevent:register()`, the script is registered with the interval 0. As this is not checked before, no warning is given and the server runs the script non-stop causing 100% cpu. With this check, we guarantee that the script will not be registered if it does not have a set interval.
The bug can be tested with this simple code:
```
local globalEvent = GlobalEvent('bugThink')
function globalEvent.onTime()
print('CPU says: "stop, it hurts!"')
return true
end
-- globalEvent:interval(1000)
globalEvent:register()
```
When we create a
globaleventof typethinkand we forgetglobalevent:interval(milliseconds)beforeglobalevent:register(), the script is registered with the interval 0. As this is not checked before, no warning is given and the server runs the script non-stop causing 100% cpu. With this check, we guarantee that the script will not be registered if it does not have a set interval.The bug can be tested with this simple code: