Page MenuHomeWildfire Games

Allow to set the max selection size in user.cfg.
AbandonedPublic

Authored by Freagarach on Aug 14 2019, 7:01 PM.

Details

Reviewers
elexis
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Trac Tickets
#2609
Summary

After discussing with @elexis we came to the conclusion that a menu item for this is rather unnecessary, but this will still allow people who are not content with the maximum selection size to change it without searching through the code.

Test Plan

Verify this is the desired approach.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8847
Build 14511: Vulcan BuildJenkins
Build 14510: arc lint + arc unit

Event Timeline

Freagarach created this revision.Aug 14 2019, 7:01 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/375/display/redirect

elexis requested changes to this revision.Aug 29 2019, 8:51 PM

What I prewrote but not submit last time we spoke on IRC:

According to your testing it is not necessary to make this a user choice.

http://irclogs.wildfiregames.com/2019-08/2019-08-14-QuakeNet-%230ad-dev.log

19:10 < Freagarach> elexis: Back after crash ;) ~203k us for 1000 units (getmultipleselectionstates) vs ~25k us with 100 units.

I don't see any user that says "hey I want to limit my selection size to 50 units".
A mod might want to do that, but this patch makes it a user choice, not a mod choice.
For the user there is practically only a reason to increase it, not to decrease it.
And if the performance allows for basically infinite (as all the other bottlenecks hit sooner), then why not set this to say 3000 units and have every user satisfied without confusing people with a useless option? (its useless if everyone choses the same value)

In the ticket (#2609) there is an argument that the option would be useful for mods, but thats not really true either, because they can just change the constant in their mod, but the userconfig value cannot be changed by the mod and instead gives it into the users hands.

As mentioned most importantly it is in observermode and atlas selection. If there is an 8 player game with 200 pop, one can realistically select 1600 units.

Or imagine a gaia spam map with really many units (there is this one map with proportional attacker count depending on playercount and difficulty setting).

I had tested and successfully crashed the game by spawning too many units, that was the only effect.
I suppose I should spectate an alpha 23 game and try to select that many units when they were managed to be produced without crashing the game.

But also the time in GetEntityState should be measured. I mean the reviewer ought to repeat the experiment to compare the results to your measurement.
203k us - please dont mix units like that :P
k = kilo = 10^3
u = μ = micro = 10^-6
So thats 203ms.
Which is order of magnitude of turn length!

So in that case I would even say it would be disadvisable to grant the user to select so many units.
If some player did that in a MP game, he might, according to these numbers, additionally lag the game.

More detailed numbers would really help more say how far the selection can be pushed without hitting the performance ceiling.
I mean some stats that measure the number depending on amount of units.

This could actually be easier tested if there was a script to test that. One could then just run the test binary and get the results. But seems out of immediate reach.

So I agree with providing more freedom, but we have to ensure it doesn't add signfiicant lag. Perhaps I read the numbers wrong?

This revision now requires changes to proceed.Aug 29 2019, 8:51 PM
Freagarach abandoned this revision.May 21 2020, 10:13 AM

Valid points ^^

If the idea of the patch is to increase the nr 200 to a greater number, then the idea of the patch is correct, but it's time hasn't come yet if selecting many units is so slow that it delays the game (turn length time just for the selection).

If the idea of the patch is to increase the nr 200 to a greater number, then the idea of the patch is correct, but it's time hasn't come yet if selecting many units is so slow that it delays the game (turn length time just for the selection).

Yeah I understand :) The idea of the patch was to make it a user choice, thus the idea is wrong, as you noticed :)