-
Notifications
You must be signed in to change notification settings - Fork 213
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
Rate limit Widget use of GiveOrder* #4301
base: master
Are you sure you want to change the base?
Conversation
This adds a system which poisons Spring.GiveOrder*, storing a sliding window of command volume over time. The rate limit is provisionally set to 110c/s over a 15 second average, and can be bike shedded later. This is to reflect the normal nature of command volume being bursty, and sustained regular volume being troublesome. All functions within a Widget affect the same counter. Widgets that legitimately create large volumes of commands can be whitelisted. The whitelist is currently in cawidgets.lua, but could be moved to its own file if need be. The current whitelist is certainly incomplete. In particular, area command splitting should be whitelisted. As a side effect, each poisoned widget receives its own local copy of the Spring table, and modifications made to this table no longer affect other Spring instances. Changes made to the Spring table before widget creation, such as in cache.lua, or in Spring.Utilities, are still shared and reflected across all widgets. The expectation is that BLOCK_MODE=2 is the norm in a multiplayer setting. BLOCK_MODE defaults to 1 in the absence of any instructions from infra to permit destructive local testing, while still sending warnings about any excessive command volume when it occurs.
What is the performance like? Maybe disable the throttling for luauis that don't have local widgets enabled, to avoid impacting the bystanders if there is a cost. |
Theoretically, there are no temporary tables, no additional callouts (thanks to the use of currentGameFrame as an upvalue), and the overwhelming majority of bookkeeping is skipped with an early and cheap condition that only activates once per second. Experimentally, I could not create any observable performance decrease. This held even when calibrating with huge groups of units to avoid false positives on Widgets that can send briefly send large quantities of commands, primarily Scout Dodge. Ironically, this was a fair bit of optimization for functions that are now emphatically not meant to be called often. |
Sounds good. |
How about this:
I don't expect to bikeshed the rate or any other form of whitelist prior to pulling. |
I'm not sure that modside widgets are safe to the point that they should all be excluded from a virtually-free rate check? It's possibly worth running with BLOCK_MODE left at 1 for a while, so we can see if anything complains without needing to worry about enforcement. Is there a callin for when cheats are enabled? It would be nice to avoid callouts if possible. The limit has been given a lot more leeway (exceed 110aps over 15s) than the one originally discussed and tested with (exceed 80aps over 10s) |
Gameside widgets reside at a lower customisation level so I don't want to do anything that breaks them. If they break then they should be fixed gameside. Detection without blocking achieves this.
Not that I know of. The engine would know. You could cache it every second or so. |
This adds a system which poisons Spring.GiveOrder*, storing a sliding
window of command volume over time. The rate limit is provisionally set to
110c/s over a 15 second average, and can be bike shedded later.
This is to reflect the normal nature of command volume being bursty,
and sustained regular volume being troublesome.
All functions within a Widget affect the same counter.
Widgets that legitimately create large volumes of commands can be
whitelisted. The whitelist is currently in cawidgets.lua, but could
be moved to its own file if need be.
The current whitelist is certainly incomplete. In particular, area
command splitting should be whitelisted.
As a side effect, each poisoned widget receives its own local copy of
the Spring table, and modifications made to this table no longer
affect other Spring instances. Changes made to the Spring table
before widget creation, such as in cache.lua, or in Spring.Utilities,
are still shared and reflected across all widgets.
The expectation is that BLOCK_MODE=2 is the norm in a multiplayer setting.
BLOCK_MODE defaults to 1 in the absence of any instructions from infra to
permit destructive local testing, while still sending warnings about any
excessive command volume when it occurs.