Code optimization, multiple threads of waituntil and spawn

[Vandeanson 1676]

Hiho Gentlemen,

 

for the past weeks I have been looking into improving the FPS impact my Vandeanson's Apocalypse mod has on missions.

 

What does the mod do?

 

(all the below happens server side only, fyi)

 

1. []spawn "Position finder function": find well suiting locations to spawn sites of interest (small sized composition of structures, things and AI, e.g. a banditcamp) based on a random players location

 

2. []spawn "Site spawner function":mark that location (as set in #1.) and starts "waituntil {code to check for condition}" with condition that ANY player gets close to the site

 

3. create the site, enable dynamic simulation, start despawn timer

 

4.a [] "spawn hideobjectglobal function" that will hide the objects of a site once no player is near that site, but the despawn timer (item. 5 below) has not run out yet

 

4.b [] "spawn un-hideobjectglobal function" that will UNhide the objects of a site if a player is near again

 

5. wait for despawn timer to run out (minimum up-time of a site)

 

6. start "waituntil {code to check for condition}" with condition that no player is near the site

 

7. delete the site/AI

 

8. start at #1.

 

I hope its somehow recognizable, that this should allow for a dynamic environement, with least possible performance impact due to the "waituntil" gating.

Sites are only up if really needed e.g. a player is nearby to actually see the site. If the sites and AI are spawned, dynamic simulation is enabled.

 

The performance gains compared to previous versions (where assets where spawned in and FPS was only saved by dyn. simulation) are already quite considerable.

The performance saving actions happen through distance checks, waituntil, dynamic simulation, hideobjectglobal and despawn of sites that are no longer needed (e.g. noone within 4k meters proxy)

 

However, the question that is bugging me currently is about the performance impact of having multiple (possibly 100+) waituntil and spawn threads up sametime.

 

 

waituntil:

the mod spawns about 10 type of sites, each sites may be spawned up to 10 times. Each creates an own "waituntil" thread.

I seem to understand that these waituntils are continuisly eating up performance until the "condition" is met. Until then, the condition is checked every frame.

In my case, these are way to many "check" iterations than needed. A check every 5 seconds is fine as well. Hence I changed the waituntil to waituntil {sleep 5; ...code..};

 

Question: am I correct that this shold have a "considerable" positive impact on performance of waituntil usage? I got this from forum posts so it should atleast have some hand and feets.

 

 

[] spawns:

most functions need to be spawned in a separate thread, I can not use "call" as the calling script can not be halted until the function returns true and since 

suspension (waituntil) is not allowed for called functions. Where i can use "call", i do that.

Hence, in additiona to a lot of waituntils, I also have multiple spawn threads run more or less complex code sametime.

I believe that if i put in sleep timers between each bulk of complex code that should help with performance (e.g. first all bandit camps are spawned, then the next site may start to spawn in).

 

Question: this, if the sleep timers are set right, should put additional ease on cpu. Correct? Also, if I place small sleep timers throughtout spawned code, I should ease possible stutters/lags when spawning stuff.

I could also add condition checks (uff, more waituntils) between major functions spawns, that only return true once the previous site/code bulk is done.

 

 

I hope it is somehow understandable what I am trying to do and what the challenges are that I am worried about.

A playable test version of the mod can be found here FYI: (it does not include ALL of the performance saving actions yet, but it should give you a fair picture).

 

 

Why this post?

 

I would like to understand if:

- my thought process is correct

- if the issues I am looking into are actual issues worth looking into

- if more experienced people would take a similar approach or if I am missing knowledge and the issue could be taken on in better ways.

 

The changes I made already show positive results, but I am sure that there are things that I dont know yet.

The above conclusions are based on the last 2 years of coding various projects and reading up various topics online (KK's blog, forum posts, code optimisation guides...)

I have no professional coding background, hence I might lack basic knowledge.

 

End of wall of text.

 

I appreciate any feedback, input, source of information and so on that I can get;)

 

Thanks!

Cheers

VD

 

Edited May 2, 2019 by Vandeanson

[sarogahtyp 1104]

one thing. you are assuming that call is forbidden if u use loops and such things. this is wrong.

you can safely use call if u do it in a scheduled environment. initServer.sqf e.g. is running in a scheduled environment. you can check the environment with the command canSuspend or read the Scheduler wiki entry to get a clue about environments

[gc8 970]

I would only optimize code inside heavy loops. waituntil with simple condition shouldn't lag at all especially if you put sleep 5 in there

 

Had some heavy loops in my mission once with only small sleep delay on running the loops, like 2 seconds. Then I changed from the 2 secs to 7-10 seconds and immediately noticed huge performance increase. That's because the script scheduler wasn't so clogged up anymore

 

So yeah having heavy code it self isn't problem if it doesn't have to run at every frame.

 

I would also recommend using disable/enable simulations and disableAI/enableAI on units to reduce the work CPU has to make for the units not currently in important position like near player

 

 

[Vandeanson 1676]

3 minutes ago, gc8 said:

disableAI/enableAI on units

Thanks for the feedback! 

 

Good point, thanks! 

Does disableAI help if used in addition to dyn. Sim. Enabled? Or does dyn. Sim. Enabled also cover it? 

 

Cheers

VD

[Mr H. 400]

11 minutes ago, sarogahtyp said:

one thing. you are assuming that call is forbidden if u use loops and such things. this is wrong.

you can safely use call if u do it in a scheduled environment. initServer.cfg e.g. is running in a scheduled environment. you can check the environment with the command canSuspend or read the Scheduler wiki entry to get a clue about environments

also CBA has some very nice functions that will allow you to use waituntil in a non scheduled environment :
https://cbateam.github.io/CBA_A3/docs/files/common/fnc_waitAndExecute-sqf.html

and
https://cbateam.github.io/CBA_A3/docs/files/common/fnc_waitUntilAndExecute-sqf.html

and their whole PFEH system allows you to run loop codes with some delay.
ACE 3 makes use of these extensively and with little performance impact.

[gc8 970]

14 minutes ago, Vandeanson said:

Does disableAI help if used in addition to dyn. Sim. Enabled? Or does dyn. Sim. Enabled also cover it? 

 

Good question, I don't know the answer but you could test that in the editor

[Dedmen 2588]

1 hour ago, Vandeanson said:

improving the FPS impact

That's exactly my game! Let's get into it.

 

1 hour ago, Vandeanson said:

(all the below happens server side only, fyi)

Well serverside code doesn't do much for clientside fps. Do you want to improve fps on server or client?

 

1 hour ago, Vandeanson said:

and starts "waituntil {code to check for condition}"

Maybe add a sleep. If you don't sleep manually, it might check condition every frame.

 

1 hour ago, Vandeanson said:

with least possible performance impact due to the "waituntil" gating.

waitUntil isn't what prevents performance impact. Scheduled environment is.

 

1 hour ago, Vandeanson said:

However, the question that is bugging me currently is about the performance impact of having multiple (possibly 100+) waituntil and spawn threads up sametime.

Certainly considerable impact.

 

1 hour ago, Vandeanson said:

Until then, the condition is checked every frame.

At most once per frame. At minimum never or only every couple hours/days/weeks that's scheduled code for ya.

 

1 hour ago, Vandeanson said:

am I correct that this shold have a "considerable" positive impact on performance of waituntil usage?

Yes.

 

1 hour ago, Vandeanson said:

since suspension (waituntil) is not allowed for called functions

Wrong. Suspension is not allowed in unscheduled code.
If you "call" from scheduled code, it's still scheduled and you can suspend just fine.

 

1 hour ago, Vandeanson said:

I believe that if i put in sleep timers between each bulk of complex code that should help with performance

No. Scheduled scripts auto-suspend after running for 3ms, inserting manual sleeps won't make that any better.

 

1 hour ago, Vandeanson said:

this, if the sleep timers are set right, should put additional ease on cpu. Correct?

Only if all scheduled scripts do that. Such that all of them together will not execute up to the 3ms limit.

 

1 hour ago, Vandeanson said:

Also, if I place small sleep timers throughtout spawned code, I should ease possible stutters/lags when spawning stuff.

Will ease clientside stutter yes. Clients will load small things from HDD over a longer period of time and have many small lags, instead of one huge lag when loading everything at once.

 

39 minutes ago, Mr H. said:

also CBA has some very nice functions that will allow you to use waituntil in a non scheduled environment

Wouldn't recommend that to anyone who doesn't know their way around unscheduled code.

Unscheduled code will just freeze the game till it's done, scheduled runs for 3ms and then auto-suspends. So scheduled code is much safer if you don't really know what you are doing.

 

39 minutes ago, Vandeanson said:

Does disableAI help if used in addition to dyn. Sim. Enabled? Or does dyn. Sim. Enabled also cover it?

AI calculations are part of simulation. And as dynamic simulation disables all simulation, it also covers that.

 

 

General performance advice is, run less code. Instead of a waitUntil waiting for something to happen, see if there is a Eventhandler that you can use. Using a Eventhandler has 0 performance impact, until the actual event happens.

For example if you want to wait till a player picks up an item, you can use waitUntil and check every frame whether he has the item now or not, or you can use a "Take" Eventhandler and just check if he just picked the item up everytime he picked something up.

 

Scheduled code only runs for 3ms per frame. So the performance impact it can have is already capped, so it's probably not your fps problem.

Your fps problems are caused by what the code does (spawning vehicles and such) not by the code itself just running.

 

Instead of guessing around what might be a problem and how to make it better. You should profile your code/the game and see what is actually the issue, and then tackle specifically that.

No sense to optimize something that is 0.01% of the total performance impact, while you have some other function that does like 20%. (Measuring in percentage of frametime)

 

 

 

 

[LSValmont 789]

1 hour ago, Dedmen said:

Instead of guessing around what might be a problem and how to make it better. You should profile your code/the game and see what is actually the issue, and then tackle specifically that.

No sense to optimize something that is 0.01% of the total performance impact, while you have some other function that does like 20%. (Measuring in percentage of frametime)

 

I am also very interested in profiling and measuring the impact of my scripts/code.

 

I there any documentation, additional software needed or anything that can guide us though the process?

 

Also, thanks for taking your time in making all these things clear to us!

[Dedmen 2588]

36 minutes ago, LSValmont said:

I there any documentation

There... Oh... lol.

 

Oops XD Sorry biki, didn't intend on pushing you down.

 

https://community.bistudio.com/wiki/Performance_Profiling

This is what I wanted to find.

Right up gonna say that profiling specific scripts only works for unscheduled scripts. As scheduled scripts might suspend any time and are in general slower than unscheduled profiling them is hard and wouldn't yield much useful information.

On the wiki page you can read up on how to do engine profiling, what the different "scope"s mean is not documented anywhere, you basically only figure out what they mean by guessing (drw == draw, rendr == render, siFEH== script frame eventhandler and so on) or by asking a dev or me.

 

Engine profiling gives you a good overview of what generally influences your FPS. Like you can see AI processing in there, and rendering, and object simulation and so on.

The normal Arma tools can't really profile scripts though, even unscheduled ones. Unless you use my subtree trick as explained on the wiki page.

 

For profiling scripts you can use my script profiler which is basically engine profiling combined with my subtree trick together with a whole lotta steroids.

But again. Still unscheduled only so I don't think it would be much use in this case.

My profiler can also record multiple frames, multiple minutes of gameplay if you have enough RAM for it. Whereas the vanilla profiling only shows you a single frame.

So you could use it to just record while you play, and when you notice a lag spike, tab out to it and check what caused it.

 

36 minutes ago, LSValmont said:

additional software needed

Yes to that. Either Arma 3 profiling binary, or my profiler, or both for the ultimate experience (My profiler can display engine profiling information if you load profiling binary and pass a special startparameter to Arma)

 

 

Oh, should've said that first, this stuff is all for profiling your final product in it's entirety. You can ofc post script snippets into the debug console and use the profiling button there (bottom left, small button).

[Vandeanson 1676]

Will be able to properly diggest this tonight, but in the meantime, thanks for all the feedback and your time! 

 

One can always count on and learn from the bi forum hivemind

[Vandeanson 1676]

Hi all, 

 

Thanks again for all the input, I have done some reading on the scheduler and will keep it at hand going forward, some odd performance of the mod makes sense now:))

I will likely revise some recent changes I made due to some wong understanding as pointed out above. 

 

Will try to shift rubbing cose to evebbts/or eventhandlers if available for the specific event, to avoid that loops run for no reason. 

 

Currently my pc and arma seem not to like each other, my game is very unstable. I ll have to find a workarround there before futher testing of the updated mod. 

 

Cheers

Vd

[pierremgi 4738]

FPS is important, of course. It's also a good indicator for the health of your scenario. But you can also kill performance keeping acceptable FPS.

As examples:

- you can kill FPS with plenty of objects/units and few scripts, and you can read in another topic that even maps are not equal in performance;

- you can keep FPS, because GPU does its work, and drastically drop performance with multiple loops (and even bad triggers) because scheduler can't manage a huge amount of running sqf.

 

Thank you for pointing at performance profiling. I'll have a look on it.

 

What I suggest is a little bit different, easy to do for mission makers. If you noticed no significant FPS drop but AIs becoming lazy or slower popup task hints or even part of scripts which don't seem to work anymore (with no error), you probably failed to optimize your codes.

Usually, that will not occur at start, but after several minutes of game, say half an hour, in a simple SP (or MP) preview test. I hope you test your missions!

So play testing your scenario and in debug console:

- in watch lines:  count diag_ActiveSQFScripts (you don't need the Diagnostics.exe!)  

- and in console run: copyToClipboard str diag_ActiveSQFScripts

Usually, depending on your scenario, it's not abnormal to count 2 or 300 of active sqf, especially if you applied some loops on all units. (yes, some loops are useful and event handlers can't solve everything). depending on conditions you need to check, occurrence of your loops, let says a 500 sqf can stay performance friendly. Some loops like if (!isNil "aVariable") then {...} checked every 2 or 5 seconds are fine because it's not the entire code which runs but the condition "at this time". On the other hand, a framed check with hard conditions like lineIntersectSurfaces or demanding commands is more demanding.

So, without any precaution, your loops counter will grow while playing 200, 500, 1000... +2000 sqf!

But you could find now in your clipboard, if guilty loop(s) occur very often. Of course that means you wrote the code if you want to recognize it, chasing the culprit.

Of course, you will see your while {true} loop and the condition. Try to avoid them (while {alive aUnit} ) or  optimize the condition! Here, it's absolutely necessary. As example, a simple test for performance in debug console (with 10 groups, and a dozen of vehicle among them) :

{alive _x && side _x == EAST && !isNull objectParent _x} count allUnits      // poorly scripted

{side _x == EAST && !isNull objectParent _x} count allUnits                          // simplified

{side _x == EAST && {!isNull objectParent _x} } count allUnits                      // using successive condition check

{ !isNull objectParent _x && {side _x == EAST} } count allUnits                      // optimizing the successive condition check, because in this case, there are less units inside a vehicle than East ones

 

Now, your while {true} loop can be optimized like this:

waitUntil {uiSleep 5; { !isNull objectParent _x && {side _x == EAST}} count allUnits == 0 };  //example

 

Of course, you don't have to wait for a huge diag_activeSQFScripts to mind for the code optimization.

 

[Vandeanson 1676]

@pierremgi

 

35 minutes ago, pierremgi said:

kill FPS with plenty of objects/units 

This was the initial trigger for me to take a dynamic spawner approach, and start using a lot of waituntil checks and eventhandlers (where possible).

 

After reworking my scripts into a "spawn/despawn" on use (e.g. any player must be near) approach, i started to worry what such loops/waituntils would do to the mod/scenarios in the long run. e.g. Are they sustainable?

Now the further part of your post summarizes exactly what "worries" me. Thanks a lot for taking time to point these things out.

 

37 minutes ago, pierremgi said:

- in watch lines:  count diag_ActiveSQFScripts

 

great, I felt like i am not 100% sure if I properly exit all my loops, also for my basebuilding script project - this will help giving me a picture.

 

most of my spawners have two parts. first, a shorter position checker function, such as below:

 

fnc_VABC_PosChk;

Spoiler

if (!isServer) exitwith {};

waituntil {sleep 1;count ((allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") > 0};

_allplayer = (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F";
_player = selectRandom _allplayer;

private _SpawnPos = [getPosATL _player, VD_SpawnMinDist, VD_SpawnMaxDist, 0, 0, 0.4, 0, VD_Land_Blacklist_Area] call BIS_fnc_findSafePos;

_isFlatEmpty = !(_SpawnPos isFlatEmpty  [30, -1, 0.2, 30, -1, false] isEqualTo []);

if (!_isFlatEmpty) exitwith {[] spawn fnc_VABC_PosChk;};

if (count VD_BC_BLMrkrs == 0) exitwith {[_SpawnPos] spawn fnc_VABC_Spwn;};

if ({(getmarkerpos _x) distance _SpawnPos > VD_BC_DistanceCheck}foreach VD_BC_BLMrkrs) then {[_SpawnPos] spawn fnc_VABC_Spwn;}
else {[] spawn fnc_VABC_PosChk;};

 

 

this could may potentially have to run multiple times, to find suitable positions, hence i figured i separate it.

 

following that (a matching position is found), the actuall "site spawner" is spawned. In this case a premade "bandit camp" composition, random "equipment" spawns ind groundweaponholders and crates.

 

fnc_VABC_Spwn;

Spoiler

if (!isServer) exitwith {};
if (VA_DebugMrkrSites) then {  hint "BC spawn start";sleep 1;};
params ["_SpawnPos"];
private _SpawnPos = _this select 0;

private _mrkr = VD_bc_MarkerArray select 0;
VD_bc_MarkerArray = VD_bc_MarkerArray - [_mrkr];
VD_BC_BLMrkrs append  [_mrkr];
private  _DebugMrkr = createMarker [_mrkr, _SpawnPos];

if (VA_DebugMrkrSites) then {
  _DebugMrkr setMarkerShape "ICON";
  _DebugMrkr setMarkerType "hd_dot";
  _DebugMrkr setMarkerText "BC";
};

VABC_Set = VABC_Set + 1;
if (VA_DebugMrkrSites) then {  hint "BC spawn end"; sleep 1;};
waituntil {sleep 1;({_x distance _SpawnPos < 1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") || ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F")};

if ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") exitwith
                  {
                                      deleteMarker _DebugMrkr;
                                      VD_BC_BLMrkrs = VD_BC_BLMrkrs -  [_mrkr];
                                      VD_bc_MarkerArray append [_mrkr];
                                      [] spawn fnc_VABC_PosChk;
                  };


      _itemboxes = [];

      _Fireplace = "Campfire_burning_F" createVehicle _SpawnPos;
      _composition = selectrandom VD_BC_Comp_Array;
      [getpos _Fireplace, 0, call _composition] call BIS_fnc_ObjectsMapper;

                _zeds = _SpawnPos nearEntities ["zombie", 60];

                {deleteVehicle _x} foreach _zeds;


              _objects = _SpawnPos nearobjects 40;
              _units = _SpawnPos nearEntities ["zombie", 60];

              _objects = _objects - [_units];

              {_x allowDamage false;_x setVectorUp surfaceNormal position _x;} foreach _objects;

              _nearBuildings = _SpawnPos nearObjects ["CampEast", 40];
              if (count _nearbuildings >= 1) then {
              _nearBuilding1 = _nearBuildings select 0;
              _buildpositions1 = _nearBuilding1 buildingPos -1;
              _buildpos1 = _buildpositions1 select 3;
              _itemBox2 = "Box_IND_Ammo_F" createVehicle selectRandom _buildpositions1;
              _itemBox2 allowDamage false;
              _itemboxes append [_itemBox2];
              {[_x] call VD_lootCrateT1} foreach [_itembox2];};

              if (count _nearbuildings >= 2) then {
              _nearBuilding2 = _nearBuildings select 1;
              _buildpositions2 = _nearBuilding2 buildingPos -1;
              _buildpos2 = _buildpositions2 select 3;
              _itemBox3 = "Box_IND_Ammo_F" createVehicle _buildpos2;
              _itemBox3 allowDamage false;
              _itemboxes append [_itemBox3];
              {[_x] call VD_lootCrateT1} foreach [_itembox3];

              };

              [_Fireplace,_Fireplace,"GroundWeaponHolder",5,2,15,100] call fnc_VA_LootSpwn;


/////////////////////////
_zeds = _SpawnPos nearEntities ["zombie", 60];

{deleteVehicle _x} foreach _zeds;


              _FactionsArray = [east,west,independent];
              _Faction = _FactionsArray select VD_BC_Side;
              _class = selectrandom VD_BC_Units;

[_Faction, _Fireplace, 1,10,_class,2,20,90] spawn fnc_VAAI_TaskDef;

[_Fireplace,40,60,1,4] spawn fnc_VAIED_Spwn;

//[_refObj,_minC,_maxC,_minD,_maxD] spawn fnc_VAMedBx;
[_Fireplace,1,2,2,10] spawn fnc_VAMedBx;

              {_x enableDynamicSimulation true}foreach _objects;





            {_x setVariable ['grad_persistence_isExcluded',true];}foreach _itemboxes;
            {_x setVariable ['grad_persistence_isExcluded',true];}foreach _objects;
_objectsall = [];
_objectsall append _itemboxes;
_objectsall append _objects;

[_Fireplace,_objectsall] spawn fnc_VA_HidObjGl;

/////
sleep VD_UptimeFix;
sleep (random VD_UptimeRnd);
////

             _alignpos1 = getpos _Fireplace;

              waituntil {sleep 5;{_x distance _Fireplace > VD_DeletionSaveZone}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F"};

            //  {deleteVehicle _x}forEach units _bandits;
              {deleteVehicle _x}forEach _itemboxes;
              deleteVehicle _Fireplace;
              {deleteVehicle _x}foreach _objects;



                          _groundholder = _alignpos1 nearObjects ["GroundWeaponHolder", 60];
                          _Medboxes = _alignpos1 nearObjects 60;
                          sleep 2;

                          deleteMarker _DebugMrkr;

                          VD_BC_BLMrkrs = VD_BC_BLMrkrs -  [_mrkr];
                          VD_bc_MarkerArray append [_mrkr];

                          {deleteVehicle _x}foreach _groundholder;
                          {deleteVehicle _x}foreach _Medboxes;


[] spawn fnc_VABC_PosChk;

 

 

you might notice that this spawner also spawns separate functions (AI spawner, loot spawner, a hideobjectglobal "loop").

 

these spawners contain the bit bulk of code, but these are only executed at mission start (about 10 different sites, 5-15 versions of it same time).

 

I am assuming the pos finder functions have a potential to run endlessly, if no matching position is found.

and spawning multiple sites sametime, could become an issue (although one could just delay the spawns, no need to spawn all same time).

 

Then we have the "waituntil" checks that check if a player is near/not near, which I delayed by {sleep 3; waituntil...} which hopefully helps.

just some additional input/thoughts that may help to understand where i am at.

 

52 minutes ago, pierremgi said:

{alive _x && side _x == EAST && !isNull objectParent _x} count allUnits      // poorly scripted

{side _x == EAST && !isNull objectParent _x} count allUnits                          // simplified

{side _x == EAST && {!isNull objectParent _x} } count allUnits                      // using successive condition check

{ !isNull objectParent _x && {side _x == EAST} } count allUnits                      // optimizing the successive condition check, because in this case, there are less units inside a vehicle than East ones

I feel like this one (successive condition check) is gold for my needs! thank you very much!

 

oh, and i should probably replace my "sleep" with "uisleep"..=D

 

off to optimizing/testing!

 

Thanks a ton,

cheers

VD

[Dedmen 2588]

10 hours ago, pierremgi said:

// using successive condition check

It's called lazy evaluation.

"successive condition check" just means you are checking multiple conditions.

 

Also keep in mind that scheduled is always limited to 3ms, unless you use a very heavy command like nearestObjects, then it will let that command finish and then suspend the scheduled script.

Also before scheduled scripts are executed, they are first sorted to find out in which order they have to be executed, the more "diag_activeSQFScripts" you have the longer that sorting takes, but that's about it whether 5 or 5000 scripts, there is still a 3ms limit each frame.

9 hours ago, Vandeanson said:

_isFlatEmpty = !(_SpawnPos isFlatEmpty [30, -1, 0.2, 30, -1, false] isEqualTo []);

if (!_isFlatEmpty) exitwith {[] spawn fnc_VABC_PosChk;};

if (count VD_BC_BLMrkrs == 0) exitwith {[_SpawnPos] spawn fnc_VABC_Spwn;};

Here you have a nice optimization opportunity.

Order of statements is important.

For example if VD_BC_BLMrkrs is really empty, do you even need to execute the other stuff?

 

Also.. You are calling yourself here?

9 hours ago, Vandeanson said:

[] spawn fnc_VABC_PosChk;

Why? Just put a loop around BIS_fnc_findSafePos until you have found a suitable position. Maybe with a sleep in there.

 

 

9 hours ago, Vandeanson said:

waituntil {sleep 1;count ((allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") > 0};

_allplayer = (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F";

Let's say there are non dead players.

You sleep 1 second, filter the allPlayers list for nondead ones, then remove headless clients and check if >0 and exit the waitUntil.
Then you filter the allPlayers list for nondead ones, then remove headless clients.

You are filtering twice, and you have a delay in there.

Could do something like

_allplayer = (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F";

if (_allplayer isEqualTo []) then {
    waitUntil {
        sleep 1;
        _allplayer = (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F";
        count _allplayer > 0
    };
};

instead. If you already have alive players, just skip the waitUntil and sleep and re-filtering of allPlayers list completely.

 

 

Important part is also optimizing checks according to what you expect to usually happen. Usually not all players are dead. A player being dead is a rare condition.

So you should expect that there is probably a player alive and only execute your fallback code (the waitUntil) if that expectation happens to not hold true.

 

 

 

Short detour back to "VD_BC_BLMrkrs" being empty now that I understood how your script works.

So currently you are potentially executing your script a dozen times just to find a suitable position, and after the (atleast 12 seconds because of that bad waitUntil as discussed above) of work, you notice that "VD_BC_BLMrkrs" is empty and you just throw your result away.

 

9 hours ago, Vandeanson said:

{(getmarkerpos _x) distance _SpawnPos > VD_BC_DistanceCheck}foreach VD_BC_BLMrkrs

You can use findIf here.

Let's say you have 100 entries in "VD_BC_BLMrkrs"

If...... wait.. what? That code is wrong.

If you have 100 entries in "VD_BC_BLMrkrs", you check through ALL of them, and then only take the result of the last one. That is clearly a logic error.

 

Let's assume you didn't have that logic error.

 

Let's say you have 100 entries in "VD_BC_BLMrkrs"

If the 1st entry is already returning that the marker is too close to _SpawnPos and you already know you need to find a new position, you will still continue to evaluate the rest of the 99 markers even though it's completely unnecessary as you already have the result (people would usually use "count" instead of "forEach" here and then check if ==0 or >0).

If you use findIf on the other hand, it aborts checking once the first result returns true.

Meaning you could change this to being findIf and adjust your condition to return true once you hit an error.

That way in my above example you will only check the first of the 100 markers, find the error, and instantly abort and not even care about the other markers.

 

9 hours ago, Vandeanson said:

params ["_SpawnPos"];

private _SpawnPos = _this select 0;

Wtf is this? params grabs _SpawnPos out of _this. And then you do the exact same thing again right after it?

 

"params ["_SpawnPos"];" is the same as "private _SpawnPos = _this select 0;"

I guess you mixed up "params" with "private ["_SpawnPos"]" (which you btw should not use) here?

 

9 hours ago, Vandeanson said:

private _mrkr = VD_bc_MarkerArray select 0;

VD_bc_MarkerArray = VD_bc_MarkerArray - [_mrkr];

VD_BC_BLMrkrs append [_mrkr];

The second line is bad, because you copy the whole MarkerArray, then iterate over the whole array to find values that equal _mrkr and remove them.

The third line is bad because you create a temporary array.

 

private _mrkr = VD_bc_MarkerArray deleteAt 0; //Grab and then delete the first element
VD_BC_BLMrkrs pushBack _mrkr; //Append it to BLMrkrs

So in here, for one we combined deleting and selecting into one single command. Also deleteAt doesn't copy the whole array, and doesn't iterate over the whole array either.

And second we use pushBack, which doesn't need a temporary array to be passed in.

 

9 hours ago, Vandeanson said:

waituntil {sleep 1;({_x distance _SpawnPos < 1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") || ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F")};

if ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") exitwith

 

Same thing again, you waitUntil even if you might not need it. And you filter again even though you might not need it. You even filter allPlayers 3 times in here. Just do it once and store it in a variable.

Also you made the same mistake with forEach again, forEach returns the result of the last iteration, that is probably not at all what you want. And if that is what you want, it's highly inefficient.

 

This looks like a "I know I could do it better, but I'm to lazy to do it now so I'll just copy paste it".

I do such things alot too. My trick is to just add a quick

 //#TODO clean this up

at that place. And then later on I just search in all my files for "#TODO" and do whatever the todo tells me to.

 

9 hours ago, Vandeanson said:

_itemboxes = [];

I see you are using private in some places, and then not at all in other places. You should try to do it consistently. It helps your performance, and prevents very hard to find bugs.

 

9 hours ago, Vandeanson said:

{[_x] call VD_lootCrateT1} foreach [_itembox2];

Useless forEach? Or is that just a API thing to be easily extendible?

 

10 hours ago, Vandeanson said:

_objectsall = [];

_objectsall append _itemboxes;

_objectsall append _objects;

 

Can be shortened to
 

_objectsall = _itemboxes + _objects;

 

 

10 hours ago, Vandeanson said:

I am assuming the pos finder functions have a potential to run endlessly, if no matching position is found.

Yes definitely. If you use a loop as I said above, you could just use a for/to/do loop (Not the array variant, it's slow) to loop for example 10 times, but then just exitWith out of it once you found a good position. And if you didn't find a position after 10 iterations, just handle it somehow.

 

10 hours ago, Vandeanson said:

oh, and i should probably replace my "sleep" with "uisleep"..=D

Huh? why? I don't see any reason.

[Vandeanson 1676]

Hi Dedmen,

 

Thanks for your time!

Loads of new input that I am looking forward to implement into the scripts, much appreciated.

 

With regards to the position finding and distance checks to existing sites:

Awesome feedback, makes total sense indeed! I also occurred to me that I might be able to avoid checking through the blacklist arrays:

 

For each site spawned, I will create an area marker, that will be added to the "blacklistPos" parameter of "BIS_fnc_findSafePos":

 

private _SpawnPos = [getPosATL _player, VD_SpawnMinDist, VD_SpawnMaxDist, 10, 0, 0.2, 0, VD_Land_Blacklist_Area] call "BIS_fnc_findSafePos;

 

So now only positions not within the blacklisted area will be suggested by BIS_fnc_findSafePos and this removes the need to crosscheck the blacklist array completely.

 

Spoiler

private _mrkr = VD_bc_MarkerArray deleteAt 0;
VD_BC_BLMrkrs pushBack _mrkr;
private  _DebugMrkr = createMarker [_mrkr, _SpawnPos];

_DebugMrkr setMarkerShape "ELLIPSE";
_DebugMrkr setMarkerAlpha 0; //set to 1 for debugging
_DebugMrkr setMarkerSize [1000,1000];
VD_Land_Blacklist_Area pushback _DebugMrkr;

 

 

Once it is time to despawn the camp (part of the same spawner script, after some waituntil), I would like to retrieve the marker from the "blacklist marker array" (VD_BC_BLMrkrs) and put it back into the "bandit  camp marker array pool" (VD_BC_MarkerArray).

I would do this as following:

 

Spoiler

deleteMarker _DebugMrkr;

_mrkrInd = VD_BC_BLMrkrs find _mrkr;
VD_BC_BLMrkrs deleteat _mrkrInd;
VD_bc_MarkerArray pushBack _mrkr;
_mrkrInd2 = VD_Land_Blacklist_Area find _mrkr;
VD_Land_Blacklist_Area deleteat _mrkrInd2;

 

 

So whilst I couldn't get your proposal regarding the "any player alive check" right, I feel like the below update should be a bit of an improvement:

 

PosCheck:

Spoiler

waituntil {uiSleep 1;count ((allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") > 0};

while {true} do {
_allplayer = (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F";


                  private _SpawnPos = [getPosATL selectRandom _allplayer, VD_SpawnMinDist, VD_SpawnMaxDist, 10, 0, 0.1, 0, VD_Land_Blacklist_Area] call BIS_fnc_findSafePos;

                if (_SpawnPos isFlatEmpty  [15, -1, 0.2, 20, 0, false] isEqualTo []) exitwith {[_SpawnPos] spawn fnc_VABC_Spwn1};
                sleep 1;
};

 

 

and the Spawner: (just the marker placement is update, the rest is WIP) 

Spoiler

private _SpawnPos = _this select 0;

private _mrkr = VD_bc_MarkerArray deleteAt 0;
VD_BC_BLMrkrs pushBack _mrkr;
private  _DebugMrkr = createMarker [_mrkr, _SpawnPos];

_DebugMrkr setMarkerShape "ELLIPSE";
_DebugMrkr setMarkerAlpha 0; //set to 1 for debugging
_DebugMrkr setMarkerSize [1000,1000];
VD_Land_Blacklist_Area pushback _DebugMrkr;

waituntil {uiSleep 1;({_x distance _SpawnPos < 1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") || ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F")};

if ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") exitwith
                  {
                                      deleteMarker _DebugMrkr;
                                      _mrkrInd = VD_BC_BLMrkrs find _mrkr;
                                      VD_BC_BLMrkrs deleteat _mrkrInd;
                                      VD_bc_MarkerArray pushBack _mrkr;

                                      _mrkrInd2 = VD_Land_Blacklist_Area find _mrkr;
                                      VD_Land_Blacklist_Area deleteat _mrkrInd2;
                                      [] spawn fnc_VABC_PosChk;
                  };


      _itemboxes = [];

      _Fireplace = "Campfire_burning_F" createVehicle _SpawnPos;
      _composition = selectrandom VD_BC_Comp_Array;
      [getpos _Fireplace, 0, call _composition] call BIS_fnc_ObjectsMapper;

                _zeds = _SpawnPos nearEntities ["zombie", 60];

                {deleteVehicle _x} foreach _zeds;


              _objects = _SpawnPos nearobjects 40;
              _units = _SpawnPos nearEntities ["zombie", 60];

              _objects = _objects - [_units];

              {_x allowDamage false;_x setVectorUp surfaceNormal position _x;} foreach _objects;

              _nearBuildings = _SpawnPos nearObjects ["CampEast", 40];
              if (count _nearbuildings >= 1) then {
              _nearBuilding1 = _nearBuildings select 0;
              _buildpositions1 = _nearBuilding1 buildingPos -1;
              _buildpos1 = _buildpositions1 select 3;
              _itemBox2 = "Box_IND_Ammo_F" createVehicle selectRandom _buildpositions1;
              _itemBox2 allowDamage false;
              _itemboxes append [_itemBox2];
              {[_x] call VD_lootCrateT1} foreach [_itembox2];};

              if (count _nearbuildings >= 2) then {
              _nearBuilding2 = _nearBuildings select 1;
              _buildpositions2 = _nearBuilding2 buildingPos -1;
              _buildpos2 = _buildpositions2 select 3;
              _itemBox3 = "Box_IND_Ammo_F" createVehicle _buildpos2;
              _itemBox3 allowDamage false;
              _itemboxes append [_itemBox3];
              {[_x] call VD_lootCrateT1} foreach [_itembox3];

              };

              [_Fireplace,_Fireplace,"GroundWeaponHolder",5,2,15,100] call fnc_VA_LootSpwn;


/////////////////////////
_zeds = _SpawnPos nearEntities ["zombie", 60];

{deleteVehicle _x} foreach _zeds;


              _FactionsArray = [east,west,independent];
              _Faction = _FactionsArray select VD_BC_Side;
              _class = selectrandom VD_BC_Units;

[_Faction, _Fireplace, 1,10,_class,2,20,90] spawn fnc_VAAI_TaskDef;

[_Fireplace,40,60,1,4] spawn fnc_VAIED_Spwn;

//[_refObj,_minC,_maxC,_minD,_maxD] spawn fnc_VAMedBx;
[_Fireplace,1,2,2,10] spawn fnc_VAMedBx;

              {_x enableDynamicSimulation true}foreach _objects;





            {_x setVariable ['grad_persistence_isExcluded',true];}foreach _itemboxes;
            {_x setVariable ['grad_persistence_isExcluded',true];}foreach _objects;
_objectsall = [];
_objectsall append _itemboxes;
_objectsall append _objects;

[_Fireplace,_objectsall] spawn fnc_VA_HidObjGl;

/////
uiSleep VD_UptimeFix;
uiSleep (random VD_UptimeRnd);
////

             _alignpos1 = getpos _Fireplace;

              waituntil {uiSleep 5;{_x distance _Fireplace > VD_DeletionSaveZone}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F"};

            //  {deleteVehicle _x}forEach units _bandits;
              {deleteVehicle _x}forEach _itemboxes;
              deleteVehicle _Fireplace;
              {deleteVehicle _x}foreach _objects;



                          _groundholder = _alignpos1 nearObjects ["GroundWeaponHolder", 60];
                          _Medboxes = _alignpos1 nearObjects 60;
                          uiSleep 2;

                          deleteMarker _DebugMrkr;

                          _mrkrInd = VD_BC_BLMrkrs find _mrkr;
                          VD_BC_BLMrkrs deleteat _mrkrInd;
                          VD_bc_MarkerArray pushBack _mrkr;
                          _mrkrInd2 = VD_Land_Blacklist_Area find _mrkr;
                          VD_Land_Blacklist_Area deleteat _mrkrInd2;


                          {deleteVehicle _x}foreach _groundholder;
                          {deleteVehicle _x}foreach _Medboxes;


[] spawn fnc_VABC_PosChk;

 

 

 

On 5/10/2019 at 9:03 AM, Dedmen said:

On 5/9/2019 at 10:59 PM, Vandeanson said:

waituntil {sleep 1;({_x distance _SpawnPos < 1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") || ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F")};

if ({_x distance _SpawnPos > VD_SpawnMaxDist+1000}foreach (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F") exitwith

 

Same thing again, you waitUntil even if you might not need it. And you filter again even though you might not need it. You even filter allPlayers 3 times in here. Just do it once and store it in a variable.

Also you made the same mistake with forEach again, forEach returns the result of the last iteration, that is probably not at all what you want. And if that is what you want, it's highly inefficient.

 

I see your point - will look into it.

Got an idea for a workarround now tho - will provide;)

 

On 5/10/2019 at 9:03 AM, Dedmen said:

at that place. And then later on I just search in all my files for "#TODO" and do whatever the todo tells me to.

Hm yeah i should definitely start doing that! thanks!

 

On 5/10/2019 at 9:03 AM, Dedmen said:

On 5/9/2019 at 10:59 PM, Vandeanson said:

{[_x] call VD_lootCrateT1} foreach [_itembox2];

Useless forEach? Or is that just a API thing to be easily extendible?

 

partly yeah, its very old but It was also handy - will be replaced anyway,

On 5/10/2019 at 9:03 AM, Dedmen said:

On 5/9/2019 at 10:59 PM, Vandeanson said:

_itemboxes = [];

I see you are using private in some places, and then not at all in other places. You should try to do it consistently. It helps your performance, and prevents very hard to find bugs.

agreed!

 

 

dayum nearly 4 am, what a fun exercise - thanks a ton again Dedmen!

cheers

vd

 

Edited May 11, 2019 by Vandeanson

[Dedmen 2588]

11 hours ago, Vandeanson said:

So whilst I couldn't get your proposal regarding the "any player alive check" right, I feel like the below update should be a bit of an improvement:

You are still always sleeping one second, and executing the waitUntil loop, and then rebuilding the _allPlayers list. If the waitUntil condition was true from the start, there is no reason to go through the waitUntil at all.

Also why uiSleep?

 

11 hours ago, Vandeanson said:

private _SpawnPos = _this select 0;

I told you to use params, shorter and safer.

 

11 hours ago, Vandeanson said:

(allPlayers select {lifeState _x != "DEAD-RESPAWN"})

in the spawner. you are doing that 3 times in one place. Just do it once and store it in a variable.
Why did you switch all your sleeps to uiSleep? That makes no sense.

 

 

[Vandeanson 1676]

3 hours ago, Dedmen said:

If the waitUntil condition was true from the start

Would that no cause an Issue for users that make scenarios with respawn at start setting?

At least this was my thinking behind putting that waituntil, so the position finding script would not start without any player spawned on a map position.

Is that thinking wrong?

 

3 hours ago, Dedmen said:

use params, shorter and safer.

oh yeah, right! params ["_SpawnPos"];  it will be

 

3 hours ago, Dedmen said:

15 hours ago, Vandeanson said:

(allPlayers select {lifeState _x != "DEAD-RESPAWN"})

in the spawner. you are doing that 3 times in one place. Just do it once and store it in a variable.

Yep, I did not start to rework that bit of the spawner yesterday. I see why I should change it ofc. Will probably do the rest of the spawner tonight;)

 

3 hours ago, Dedmen said:

Why did you switch all your sleeps to uiSleep? That makes no sense.

good to know thanks, i saw others use UIsleep and read up about it below:

https://community.bistudio.com/wiki/sleep_vs_uiSleep

I assumed it would just generally be better.

From your feedback I take that sleep is better tho. 

Is there a clear disadvantage of/issue with uisleep?

 

cheers

vd

[Dedmen 2588]

4 hours ago, Vandeanson said:

Would that no cause an Issue for users that make scenarios with respawn at start setting?

At least this was my thinking behind putting that waituntil, so the position finding script would not start without any player spawned on a map position.

Is that thinking wrong?

You can still check without waitUntil. I already showed you how in previous post.

Just check once, and only if not successful go into the waitUntil.

 

4 hours ago, Vandeanson said:

Is there a clear disadvantage of/issue with uisleep?

uiSleep takes uiTime instead of gameTime. In singleplayer if you press Esc you pause the game and gameTime would freeze, while uiTime continues running.

 

But, on the server there is UI, thus no uiTime, so on server sleep and uiSleep are the same thing.
And in multiplayer you cannot pause the game anyway.

[Vandeanson 1676]

12 minutes ago, Dedmen said:

 

But, on the server there is UI, thus no uiTime, so on server sleep and uiSleep are the same thing.
And in multiplayer you cannot pause the game anyway.

Thanks, thats helpful. 

 

13 minutes ago, Dedmen said:

I already showed you how in previous post.

Yep a first few attempts to implement your method didnt work, but then again it was 4 am and i should probably try it with fresh brains;) will post it here once i got it right. 

 

Cheers

Vd

 

[pierremgi 4738]

16 hours ago, Dedmen said:

uiSleep takes uiTime instead of gameTime. In singleplayer if you press Esc you pause the game and gameTime would freeze, while uiTime continues running.

But, on the server there is UI, thus no uiTime, so on server sleep and uiSleep are the same thing.
And in multiplayer you cannot pause the game anyway.

 

Not a demonstration that uiSleep doesn't make sense. Especially if you write SP/MP mission.

Imho, the only advantage to use sleep instead of uiSleep is when, in SP you want to pause for what ever reason without risking to spawn (continue) many scripts at once when you're back in the game.

Without any other explanation from BI devs, uiSleep could be the norm (MP), and sleep only for specific timings you need to halt at pause (your own SP bleeding time or else).

 

[johnnyboy 3741]

Awesome thread. I just learned a lot.  Thanks to all.

[Vandeanson 1676]

tried to further slim down the PosCheck:

Spoiler

private _SpawnPos = "";
private _allplayers = "";


while {true} do {
                	_allplayers = (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F";

                      if (count _allplayers > 0) then {
                      _SpawnPos = [getPosATL selectRandom _allplayers, VD_SpawnMinDist, VD_SpawnMaxDist, 10, 0, 0.1, 0, VD_BC_BLMrkrs] call BIS_fnc_findSafePos;
                      sleep 0.5;
                      };
                      if (_SpawnPos isFlatEmpty  [15, -1, 0.2, 20, 0, false] isEqualTo []) exitwith {[_SpawnPos] spawn fnc_VABC_Spwn1};

                sleep 1;
                };

 

 

and the despawn/spawn checks in the spawner:

 

Spoiler

waituntil {

            _allplayers1 = (allPlayers select {lifeState _x != "DEAD-RESPAWN"}) - entities "HeadlessClient_F"; //only gather _allplayers once in this waituntil
             _closePl = {_x distance _SpawnPos < 1000} count _allplayers1;
            _farAwayPlayers = {_x distance _SpawnPos > VD_SpawnMaxDist+1000} count _allplayers1; //storing players that are far away from site to only gather that info once in this waituntil
            _allplayers1cnt = count _allplayers1;
           _closePl >= 1 || _farAwayPlayers == _allplayers1cnt 
          };

  if (_farAwayPlayers == _allplayers1cnt) exitwith
                    {
                                        deleteMarker _DebugMrkr;
                                        _mrkrInd = VD_BC_BLMrkrs find _mrkr;
                                        VD_BC_BLMrkrs deleteat _mrkrInd;
                                        VD_bc_MarkerArray pushBack _mrkr;

                                        [] spawn fnc_VABC_PosChk1;
                    };

 

 

and replaced the logic error with foreach - using count now. thanks for pointing that out @Dedmen, players reported a bug that I can now fix with that change. didn't realize that foreach is working that way.

[pierremgi 4738]

Why do you need a PosCheck every 1 second??

[Vandeanson 1676]

5 hours ago, pierremgi said:

Why do you need a PosCheck every 1 second??

Just for testing, it will be a longer delay in the final script. 

[pierremgi 4738]

41 minutes ago, Vandeanson said:

Just for testing, it will be a longer delay in the final script. 

So why don't you run that only when needed?